| Message ID | 20200904154547.3836-4-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: mockup: support dynamically created and removed chips | expand |
On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Doesn't mm/util.c provides us something like this? > > strndup_user()? > > > > Yes, there's both strndup_user() as well as strncpy_from_user(). The > problem is that they rely on the strings being NULL-terminated. This > is not guaranteed for debugfs file_operations write callbacks. We need > some helper that takes the minimum of bytes provided by userspace and > the buffer size and figure out how many bytes to actually copy IMO. Wouldn't this [1] approach work? [1]: https://elixir.bootlin.com/linux/v5.9-rc3/source/arch/x86/kernel/cpu/mtrr/if.c#L93
On Mon, Sep 7, 2020 at 12:19 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Sep 7, 2020 at 1:05 PM Bartosz Golaszewski > <bgolaszewski@baylibre.com> wrote: > > On Fri, Sep 4, 2020 at 6:35 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Fri, Sep 04, 2020 at 05:45:27PM +0200, Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > Doesn't mm/util.c provides us something like this? > > > strndup_user()? > > > > > > > Yes, there's both strndup_user() as well as strncpy_from_user(). The > > problem is that they rely on the strings being NULL-terminated. This > > is not guaranteed for debugfs file_operations write callbacks. We need > > some helper that takes the minimum of bytes provided by userspace and > > the buffer size and figure out how many bytes to actually copy IMO. > > Wouldn't this [1] approach work? > > [1]: https://elixir.bootlin.com/linux/v5.9-rc3/source/arch/x86/kernel/cpu/mtrr/if.c#L93 > Sure, but this is pretty much what I do in getline_from_user(). If anything we should port mtrr_write() to using getline_from_user() once it's available upstream, no? Bart
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index 94b285411659..5aedd8ac5c31 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -333,6 +333,9 @@ long strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr, long count); long strnlen_user_nofault(const void __user *unsafe_addr, long count); +ssize_t getline_from_user(char *dst, size_t dst_size, + const char __user *src, size_t src_size); + /** * get_kernel_nofault(): safely attempt to read from a location * @val: read into this variable diff --git a/lib/usercopy.c b/lib/usercopy.c index b26509f112f9..55aaaf93d847 100644 --- a/lib/usercopy.c +++ b/lib/usercopy.c @@ -87,3 +87,40 @@ int check_zeroed_user(const void __user *from, size_t size) return -EFAULT; } EXPORT_SYMBOL(check_zeroed_user); + +/** + * getline_from_user - Copy a single line from user + * @dst: Where to copy the line to + * @dst_size: Size of the destination buffer + * @src: Where to copy the line from + * @src_size: Size of the source user buffer + * + * Copies a number of characters from given user buffer into the dst buffer. + * The number of bytes is limited to the lesser of the sizes of both buffers. + * If the copied string contains a newline, its first occurrence is replaced + * by a NULL byte in the destination buffer. Otherwise the function ensures + * the copied string is NULL-terminated. + * + * Returns the number of copied bytes or a negative error number on failure. + */ + +ssize_t getline_from_user(char *dst, size_t dst_size, + const char __user *src, size_t src_size) +{ + size_t size = min_t(size_t, dst_size, src_size); + char *c; + int ret; + + ret = copy_from_user(dst, src, size); + if (ret) + return -EFAULT; + + dst[size - 1] = '\0'; + + c = strchrnul(dst, '\n'); + if (*c) + *c = '\0'; + + return c - dst; +} +EXPORT_SYMBOL(getline_from_user);