linux-kernel - Re: [PATCH 5/5] selinux: drop unnecessary NULL check

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] [day] [month] [year] [list]

Message-ID: <CAHC9VhQ=tDdbTeugwiZtWkZxupDkdt7QBDr8a4txtSg0D+Q46w@mail.gmail.com>
Date:   Tue, 7 Jun 2022 17:35:05 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Christian Göttsche <cgzones@...glemail.com>,
        selinux@...r.kernel.org,
        Stephen Smalley <stephen.smalley.work@...il.com>,
        Eric Paris <eparis@...isplace.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Ondrej Mosnacek <omosnace@...hat.com>,
        Serge Hallyn <serge@...lyn.com>,
        Austin Kim <austin.kim@....com>,
        Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Yang Li <yang.lee@...ux.alibaba.com>,
        linux-kernel@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH 5/5] selinux: drop unnecessary NULL check

On Tue, Jun 7, 2022 at 5:26 PM Nick Desaulniers <ndesaulniers@...gle.com> wrote:
>
> On Tue, Jun 7, 2022 at 2:22 PM Paul Moore <paul@...l-moore.com> wrote:
> >
> > On Thu, Feb 17, 2022 at 9:22 AM Christian Göttsche
> > <cgzones@...glemail.com> wrote:
> > >
> > > Commit e3489f8974e1 ("selinux: kill selinux_sb_get_mnt_opts()")
> > > introduced a NULL check on the context after a successful call to
> > > security_sid_to_context().  This is on the one hand redundant after
> > > checking for success and on the other hand insufficient on an actual
> > > NULL pointer, since the context is passed to seq_escape() leading to a
> > > call of strlen() on it.
> > >
> > > Reported by Clang analyzer:
> > >
> > >     In file included from security/selinux/hooks.c:28:
> > >     In file included from ./include/linux/tracehook.h:50:
> > >     In file included from ./include/linux/memcontrol.h:13:
> > >     In file included from ./include/linux/cgroup.h:18:
> > >     ./include/linux/seq_file.h:136:25: warning: Null pointer passed as 1st argument to string length function [unix.cstring.NullArg]
> > >             seq_escape_mem(m, src, strlen(src), flags, esc);
> > >                                    ^~~~~~~~~~~
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@...glemail.com>
> > > ---
> > >  security/selinux/hooks.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > I was waiting for Nick to reply, but he never did, and this looks good
> > to me so I just merged it into selinux/next.  Thanks for your patience
> > Christian.
>
> LGTM; you can ping me on irc #ndesaulniers on most kernel channels if
> you're waiting on me. ;)

Thanks, but I generally don't have the spare cycles to keep track of
everyone's prefered method of interaction, that's why we've got the
mailing list (warts and all) :)

For what it's worth, I was waiting on you because you asked about the
additional trace info and without any context I thought you might be
looking for something else (?).  In the end, I think everyone agreed
that the patch was good so I merged it.  I think as a general rule
it's a good practice to follow-up with a reply when people provide
additional information that you've requested.  Not only is it the
polite thing to do, it helps clarify things with everyone else that
there is no hidden "gotcha!" in the patch.

Regardless, thanks for checking back on this :)

-- 
paul-moore.com