Re: [PATCH v4 02/19] liveupdate: Pass user argument to file preserve and retrieve

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] liveupdate: pass NULL instead of 0 for arg in
  liveupdate_get_file_incoming()
- [Low] liveupdate: initialize __free(kfree) karg at kmalloc() in
  luo_copy_user_arg()
--

commit ce30d6f92617722b3b0681f0c0fa88542091865b
Author: Pasha Tatashin <pasha.tatashin@soleen.com>

liveupdate: Pass user argument to file preserve and retrieve

This commit extends liveupdate file preservation and retrieval ioctls to
accept a handler-specific user argument structure, validating and copying it
into struct liveupdate_file_op_args for the handler callbacks.

> diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
> index a42b60356d523..1437630d435e6 100644
> --- a/kernel/liveupdate/luo_file.c
> +++ b/kernel/liveupdate/luo_file.c
[ ... ]
> @@ -202,11 +203,43 @@ static bool luo_token_is_used(struct luo_file_set *file_set, u64 token)
>  	return false;
>  }
>  
> +static int luo_copy_user_arg(const void __user *uptr, const void **argp,
> +			     const struct liveupdate_arg_spec *spec)
> +{
> +	void *karg __free(kfree) = NULL;
> +	u32 user_size;
> +	int err;
> +
> +	if (!uptr)
> +		return 0;
> +	if (!spec->size)
> +		return -EOPNOTSUPP;
> +
> +	if (get_user(user_size, (const u32 __user *)uptr))
> +		return -EFAULT;
> +	if (user_size < spec->min_size)
> +		return -EINVAL;
> +	if (user_size > PAGE_SIZE)
> +		return -E2BIG;
> +
> +	karg = kmalloc(spec->size, GFP_KERNEL);
> +	if (!karg)
> +		return -ENOMEM;

[Severity: Low]
Could karg be defined and initialized at the kmalloc() call instead of
being initialized to NULL at the top of luo_copy_user_arg(), following the
Cleanup API convention for __free() variables?

[ ... ]

> @@ -995,6 +1052,6 @@ int liveupdate_get_file_incoming(struct liveupdate_session *s, u64 token,
>  				 struct file **filep)
>  {
>  	return luo_retrieve_file(luo_file_set_from_session_locked(s),
> -				 token, filep);
> +				 token, 0, filep);
>  }
>  EXPORT_SYMBOL_GPL(liveupdate_get_file_incoming);

[Severity: Low]
Should this pass NULL instead of 0 for the const void __user *arg
parameter to avoid a Sparse -Wnon-pointer-null warning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111002358.1842967-1-dmatlack@google.com?part=2