linux-kernel - Re: [PATCH] lsm: check size of writes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhRDrmNBF7ta_AHNbztDZ90zCJ81+m+4inGu_Jv2m2dChA@mail.gmail.com>
Date: Mon, 16 Dec 2024 17:59:40 -0500
From: Paul Moore <paul@...l-moore.com>
To: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc: Leo Stone <leocstone@...il.com>, 
	syzbot+4eb7a741b3216020043a@...kaller.appspotmail.com, jmorris@...ei.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org, 
	mortonm@...omium.org, serge@...lyn.com, syzkaller-bugs@...glegroups.com, 
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] lsm: check size of writes

On Mon, Dec 16, 2024 at 4:59 AM Tetsuo Handa
<penguin-kernel@...ove.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@...kaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=4eb7a741b3216020043a
> > Signed-off-by: Leo Stone <leocstone@...il.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