[PATCH] lsm: check size of writes

[Paul Moore]

On Mon, Dec 16, 2024 at 4:59 AM Tetsuo Handa
<penguin-kernel at i-love.sakura.ne.jp> wrote:
>
> 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 at syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=4eb7a741b3216020043a
> > Signed-off-by: Leo Stone <leocstone at 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) ?

I think you mean Micah Morton (mortonm) and not Andrew Morton (akpm),
yes?  Micah is the SafeSetID maintainer while Andrew maintains the
memory subsystem, and likely a few others.

In order to help prevent merge conflicts across the different LSM
trees I think it would be best for each LSM maintainer to merge their
respective patches.  If one of the maintainers is not responding after
a period of time, I can pick up the patch via the LSM tree.  We have
had some issues with SafeSetID in the past, but I'm hopeful we
resolved that with Micah and we just need to give him some time to
review and pickup this patch.

> > ---
> >  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);

-- 
paul-moore.com