Re: [PATCH v2] lsm: check size of writes

[Paul Moore]

On Tue, Dec 17, 2024 at 1:27 PM Leo Stone <leocstone@xxxxxxxxx> wrote:
>
> syzbot attempts to write a buffer with a large size to a sysfs entry
> with writes handled by handle_policy_update(), triggering a warning
> in kmalloc.
>
> Check the size specified for write buffers before allocating.
>
> Reported-by: syzbot+4eb7a741b3216020043a@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=4eb7a741b3216020043a
> Signed-off-by: Leo Stone <leocstone@xxxxxxxxx>
> ---
> v2: Make the check in handle_policy_update() to also cover
> safesetid_uid_file_write(). Thanks for your feedback.
> v1: https://lore.kernel.org/all/20241216030213.246804-2-leocstone@xxxxxxxxx/
> ---
>  security/safesetid/securityfs.c | 3 +++
>  1 file changed, 3 insertions(+)

Looks okay to me.  Micah, are you planning to merge this patch, or
would you like me to take it via the LSM tree?

Reviewed-by: Paul Moore <paul@xxxxxxxxxxxxxx>

I'm going to tag this to come back to it in a week or so in case we
don't hear from Micah, but if you don't see any further replies Leo,
feel free to send a gentle nudge ;)

> diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
> index 25310468bcdd..8e1ffd70b18a 100644
> --- a/security/safesetid/securityfs.c
> +++ b/security/safesetid/securityfs.c
> @@ -143,6 +143,9 @@ static ssize_t handle_policy_update(struct file *file,
>         char *buf, *p, *end;
>         int err;
>
> +       if (len >= KMALLOC_MAX_SIZE)
> +               return -EINVAL;
> +
>         pol = kmalloc(sizeof(struct setid_ruleset), GFP_KERNEL);
>         if (!pol)
>                 return -ENOMEM;
> --
> 2.43.0

--
paul-moore.com