Re: [PATCH 1/5] string.h: add array-wrappers for (v)memdup_user()

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Wed, Aug 30, 2023 at 4:46 PM Philipp Stanner <pstanner@redhat.com> wrote:
>
> Currently, user array duplications are sometimes done without an
> overflow check. Sometimes the checks are done manually; sometimes the
> array size is calculated with array_size() and sometimes by calculating
> n * size directly in code.
>
> Introduce wrappers for arrays for memdup_user() and vmemdup_user() to
> provide a standardized and safe way for duplicating user arrays.
>
> This is both for new code as well as replacing usage of (v)memdup_user()
> in existing code that uses, e.g., n * size to calculate array sizes.

...

> --- a/include/linux/string.h
> +++ b/include/linux/string.h

I'm wondering if this has no side-effects as string.h/string.c IIRC is
used also for early stages where some of the APIs are not available.

> @@ -6,6 +6,8 @@
>  #include <linux/types.h>       /* for size_t */
>  #include <linux/stddef.h>      /* for NULL */
>  #include <linux/errno.h>       /* for E2BIG */
> +#include <linux/overflow.h>    /* for check_mul_overflow() */
> +#include <linux/err.h>         /* for ERR_PTR() */

Can we preserve order (to some extent)?

>  #include <linux/stdarg.h>
>  #include <uapi/linux/string.h>

...

> +/**
> + * memdup_array_user - duplicate array from user space

> + *

Do we need this blank line?

> + * @src: source address in user space
> + * @n: number of array members to copy
> + * @size: size of one array member
> + *
> + * Return: an ERR_PTR() on failure.  Result is physically
> + * contiguous, to be freed by kfree().
> + */

...

> +/**
> + * vmemdup_array_user - duplicate array from user space

> + *

Redundant?

> + * @src: source address in user space
> + * @n: number of array members to copy
> + * @size: size of one array member
> + *
> + * Return: an ERR_PTR() on failure.  Result may be not
> + * physically contiguous.  Use kvfree() to free.
> + */

-- 
With Best Regards,
Andy Shevchenko