Re: [PATCH v38 39/39] LSM: Create lsm_module_list system call
On 10/12/2022 3:04 PM, Kees Cook wrote:
On Tue, Sep 27, 2022 at 01:31:55PM -0700, Casey Schaufler wrote:
> +SYSCALL_DEFINE3(lsm_module_list,
> + unsigned int __user *, ids,
> + size_t __user *, size,
> + int, flags)
Please make this unsigned int.
Sure.
> +{
> + unsigned int *interum;
> + size_t total_size = lsm_id * sizeof(*interum);
> + size_t usize;
> + int rc;
> + int i;
Please test that flags == 0 so it can be used in the future:
if (flags)
return -EINVAL;
Yup.
> +
> + if (get_user(usize, size))
> + return -EFAULT;
> +
> + if (usize < total_size) {
> + if (put_user(total_size, size) != 0)
> + return -EFAULT;
> + return -E2BIG;
> + }
> +
> + interum = kzalloc(total_size, GFP_KERNEL);
> + if (interum == NULL)
> + return -ENOMEM;
> +
> + for (i = 0; i < lsm_id; i++)
> + interum[i] = lsm_idlist[i]->id;
> +
> + if (copy_to_user(ids, interum, total_size) != 0 ||
> + put_user(total_size, size) != 0)
> + rc = -EFAULT;
No need to repeat this, if it is written first.
> + else
> + rc = lsm_id;
> +
> + kfree(interum);
> + return rc;
No need for the alloc/free. Here's what I would imagine for the whole
thing:
A better approach. Thank you.
if (flags)
return -EINVAL;
if (get_user(usize, size))
return -EFAULT;
if (put_user(total_size, size) != 0)
return -EFAULT;
if (usize < total_size)
return -E2BIG;
for (i = 0; i < lsm_id; i++)
if (put_user(lsm_idlist[i]->id, id++))
return -EFAULT;
return lsm_id;