On Thu, Nov 19, 2020 at 5:10 AM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
>
> Peter Collingbourne <pcc@xxxxxxxxxx> writes:
>
> > diff --git a/kernel/signal.c b/kernel/signal.c
> > index 8f34819e80de..678cdeb235ae 100644
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -2524,6 +2524,26 @@ static int ptrace_signal(int signr, kernel_siginfo_t *info)
> > return signr;
> > }
> >
> > +static void hide_si_addr_tag_bits(struct ksignal *ksig)
> > +{
> > + switch (siginfo_layout(ksig->sig, ksig->info.si_code)) {
> > + case SIL_FAULT:
> > + case SIL_FAULT_MCEERR:
> > + case SIL_FAULT_BNDERR:
> > + case SIL_FAULT_PKUERR:
> > + ksig->info.si_addr = arch_untagged_si_addr(
> > + ksig->info.si_addr, ksig->sig, ksig->info.si_code);
> > + break;
> > + case SIL_KILL:
> > + case SIL_TIMER:
> > + case SIL_POLL:
> > + case SIL_CHLD:
> > + case SIL_RT:
> > + case SIL_SYS:
> > + break;
> > + }
> > +}
> > +
> > bool get_signal(struct ksignal *ksig)
> > {
> > struct sighand_struct *sighand = current->sighand;
> > @@ -2761,6 +2781,10 @@ bool get_signal(struct ksignal *ksig)
> > spin_unlock_irq(&sighand->siglock);
> >
> > ksig->sig = signr;
> > +
> > + if (!(sighand->action[signr - 1].sa.sa_flags & SA_EXPOSE_TAGBITS))
> > + hide_si_addr_tag_bits(ksig);
> > +
> > return ksig->sig > 0;
> > }
>
> Ok. Seeing that this code compiles out I don't have any concerns about
> it's impact on other architectures. And I like having it always
> present as that makes all of the concerns the code has to deal with
> easier to discover.
Ack.
> There is one small issue. The test should be:
> if (!(ksig->ka.sa.sa_flags & SA_EXPOSE_TAGBITS))
> hide_si_addr_tag_bits(ksig);
>
> Outside of sighand->siglock it is racy to access sighand->action and we
> already have an atomic snapshot of the values for exactly this reason.
>
> Is there some reason you didn't use ksig->ka? Am I missing something?
No, I missed that we were copying the sigaction for this reason. I've
changed the code to use the copy as suggested in v20.
> I agree that our consumption of SA bits is slow enough that my other
> concerns are a non-issue.
Ack.
Peter
