Re: [PATCH v38 39/39] LSM: Create lsm_module_list system call

[Kees Cook]

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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit