Re: [PATCH] lsm: check size of writes

Tetsuo Handa
2024-12-16T10:53:15Z

On 2024/12/16 12:02, Leo Stone wrote:
> syzbot attempts to write a buffer with a large size to a sysfs entry
> with writes handled by safesetid_gid_file_write(), 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>

Since handle_policy_update() has two callers, you should check
inside handle_policy_update().

By the way, since syzbot found this pattern in multiple LSM modules,
do we want to ask Andrew Morton to send all patches in one pull request
(instead of sending trivial patch from multiple LSM modules) ?

> ---
> security/safesetid/securityfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
> index 25310468bcdd..5eba4c7f8d9e 100644
> --- a/security/safesetid/securityfs.c
> +++ b/security/safesetid/securityfs.c
> @@ -254,7 +254,7 @@ static ssize_t safesetid_gid_file_write(struct file *file,
> if (!file_ns_capable(file, &init_user_ns, CAP_MAC_ADMIN))
> return -EPERM;
>
> - if (*ppos != 0)
> + if (*ppos != 0 || len >= KMALLOC_MAX_SIZE)
> return -EINVAL;
>
> return handle_policy_update(file, buf, len, GID);