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

[Paul Moore]

On Tue, Jun 7, 2022 at 5:26 PM Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote:
>
> On Tue, Jun 7, 2022 at 2:22 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> >
> > On Thu, Feb 17, 2022 at 9:22 AM Christian Göttsche
> > <cgzones@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > > ---
> > >  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