Re: [PATCH v2] selinux: various sparse fixes

[Paul Moore <paul@xxxxxxxxxxxxxx>]

On Thu, Jan 27, 2022 at 2:03 PM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
>
> On Thu, Jan 27, 2022 at 7:22 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > When running the SELinux code through sparse, there are a handful of
> > warnings.  This patch resolves some of these warnings caused by
> > "__rcu" mismatches.
> >
> >  % make W=1 C=1 security/selinux/
> >
> > Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> > ---
> >  security/selinux/hooks.c   |    6 +++---
> >  security/selinux/ibpkey.c  |    2 +-
> >  security/selinux/netnode.c |    5 +++--
> >  security/selinux/netport.c |    2 +-
> >  4 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 221e642025f5..0e857f86f5a7 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2534,7 +2534,7 @@ static void selinux_bprm_committed_creds(struct linux_binprm *bprm)
> >         if (rc) {
> >                 clear_itimer();
> >
> > -               spin_lock_irq(&current->sighand->siglock);
> > +               spin_lock_irq(unrcu_pointer(&current->sighand->siglock));
> >                 if (!fatal_signal_pending(current)) {
> >                         flush_sigqueue(&current->pending);
> >                         flush_sigqueue(&current->signal->shared_pending);
> > @@ -2542,13 +2542,13 @@ static void selinux_bprm_committed_creds(struct linux_binprm *bprm)
> >                         sigemptyset(&current->blocked);
> >                         recalc_sigpending();
> >                 }
> > -               spin_unlock_irq(&current->sighand->siglock);
> > +               spin_unlock_irq(unrcu_pointer(&current->sighand->siglock));
>
> Shouldn't this be:
>
> spin_[un]lock_irq(&unrcu_pointer(current->sighand)->siglock);

Maybe.

The __rcu space annotation is definitely on task_struct::sighand, but
my (quick) look at unrcu_pointer() was that the the de-rcu'ification
applies to all of the dereferencing that is passed as the macro
argument.  Because of that I decided to pass the entire dereferencing
chain to the unrcu_pointer() macro just in case.  If that way of
thinking is incorrect please let me know, otherwise I would rather
just leave it as it is in v2.

-- 
paul-moore.com