linux-kernel - Re: objtool warning "uses BP as a scratch register" with clang-9

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: <CAK8P3a0ddxbGVj974XS+PM_mSJDu=aGfTGarjmqMCuLKn81mRg@mail.gmail.com>
Date:   Fri, 30 Aug 2019 12:44:24 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Nick Desaulniers <ndesaulniers@...gle.com>,
        Ilie Halip <ilie.halip@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>
Subject: Re: objtool warning "uses BP as a scratch register" with clang-9

On Fri, Aug 30, 2019 at 1:24 AM Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> On Wed, Aug 28, 2019 at 05:40:01PM +0200, Arnd Bergmann wrote:
> > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> > index 8eb7193e158d..fd49d28abbc5 100644
> > --- a/arch/x86/kernel/signal.c
> > +++ b/arch/x86/kernel/signal.c
> > @@ -414,6 +414,9 @@ static int __setup_rt_frame(int sig, struct ksignal *ksig,
> >                  */
> >                 put_user_ex(*((u64 *)&rt_retcode), (u64 *)frame->retcode);
> >         } put_user_catch(err);
> > +
> > +       if (current->sas_ss_flags & SS_AUTODISARM)
> > +               sas_ss_reset(current);
> >
> >         err |= copy_siginfo_to_user(&frame->info, &ksig->info);
> >         err |= setup_sigcontext(&frame->uc.uc_mcontext, fpstate,

> > diff --git a/include/linux/signal.h b/include/linux/signal.h
> > index 67ceb6d7c869..9056239787f7 100644
> > --- a/include/linux/signal.h
> > +++ b/include/linux/signal.h
> > @@ -435,8 +435,6 @@ int __save_altstack(stack_t __user *, unsigned long);
> >         put_user_ex((void __user *)t->sas_ss_sp, &__uss->ss_sp); \
> >         put_user_ex(t->sas_ss_flags, &__uss->ss_flags); \
> >         put_user_ex(t->sas_ss_size, &__uss->ss_size); \
> > -       if (t->sas_ss_flags & SS_AUTODISARM) \
> > -               sas_ss_reset(t); \
> >  } while (0);
> >
> >  #ifdef CONFIG_PROC_FS
>
> Reviewed-by: Josh Poimboeuf <jpoimboe@...hat.com>

Thanks! Before I submit this version for inclusion, let's make sure this
is the best variant. I noticed later that save_altstack_ex() is meant to
behave the same as __save_altstack(), but my patch breaks that
assumption.

Two other alternatives I can think of are

- completely open-code save_altstack_ex() in its only call site on x86,
  in addition to the change above

- explicitly mark memset() as an exception in objtool in
  uaccess_safe_builtin[], assuming that is actually safe.

      Arnd