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.
+{
+ 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;
+
+ 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:
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;
--
Kees Cook