linux-kernel - Re: objtool clac/stac handling change..

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: <5F1767D0-416A-4BA4-9DFF-E82D1EA3F5EE@amacapital.net>
Date:   Wed, 1 Jul 2020 13:36:22 -0700
From:   Andy Lutomirski <luto@...capital.net>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andy Lutomirski <luto@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: objtool clac/stac handling change..



> On Jul 1, 2020, at 12:35 PM, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> On Wed, Jul 1, 2020 at 11:29 AM Andy Lutomirski <luto@...nel.org> wrote:
>> 
>> Do we really want the exception handling to do the CLAC?  Having
>> unsafe_get_user() do CLAC seems surprising to me, and it will break
>> use cases like:
>> 
>> if (!user_access_begin(...)
>>  goto out;
>> 
>> ret = unsafe_get_user(...);
>> 
>> user_access_end();
>> 
>> check ret;
> 
> That's not how unsafe_get_user() works.
> 
> unsafe_get_user() always jumps to the error label, it never returns a
> value. So the code is actually now what you claim above, but
> 
>    if (!user_access_begin(...)
>       goto out;
> 
>    unsafe_get_user(..., out_fault);
>    user_access_end();
>   .. this is good, use the value we got..
> 
> out_fault:
>    user_access_end();
> out:
>    return -EFAULT;

Ugh, right. But maybe, with the asm goto magic, we can’t get rid of this. I’ve always disliked the pattern where we enable user access, do a bunch of accesses that branch on error, and finish up. We ought to be able to do it the way I described and get decent code generation too.

If we do this extable change, we end up with a different mess: some exception handlers will clear AC and some won’t.  I’m sure objtool can deal with this with some effort, but I’m not convinced it’s worth it.