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

Paul Moore
2024-12-18T21:51:42Z

On Tue, Dec 17, 2024 at 1:27 PM Leo Stone <leocstone@gmail.com> 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=4eb7a741b3216020043a
> Signed-off-by: Leo Stone <leocstone@gmail.com>
> ---
> 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@gmail.com/
> ---
> 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@paul-moore.com>

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