linux-kernel - Re: [GIT PULL] objtool/urgent for v6.5-rc2

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: <d11eea99-6e93-6af4-87aa-2e7f1c6ae83e@oracle.com>
Date:   Tue, 18 Jul 2023 13:01:33 +0200
From:   Vegard Nossum <vegard.nossum@...cle.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Borislav Petkov <bp@...en8.de>
Cc:     x86-ml <x86@...nel.org>, lkml <linux-kernel@...r.kernel.org>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Jens Axboe <axboe@...nel.dk>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [GIT PULL] objtool/urgent for v6.5-rc2

On 7/16/23 22:39, Linus Torvalds wrote:
> On Sun, 16 Jul 2023 at 11:42, Borislav Petkov <bp@...en8.de> wrote:
>>
>> - Mark copy_iovec_from_user() __noclone in order to  prevent gcc from
>>    doing an inter-procedural optimization and confuse objtool
> 
> This is painful.
> 
> Isn't there some way to mark the user_access_begin() itself so that
> the compiler doesn't move it?
> 
> I've pulled this thing, but it does seem like we're chasing the
> symptoms, not the deeper cause..

The deeper cause is that the [code generated by the] user_access_begin()
and user_write_access_end()/unsafe_get_user() calls end up in different
functions and objtool doesn't like that.

If you are willing to add another helper function that also takes the
label argument, you could do something like this:

#define user_access_begin_label(a,b, err_label) \
         ({ \
                 asm_volatile_goto("" :::: err_label); \
                 user_access_begin(a,b); \
         })

The asm_volatile_goto() isn't a real goto (it emits no instructions),
but it makes GCC believe there's a potential jump there and prevents the
compiler from splitting up the function across the label and its usage.

unsafe_get_user() already takes this same label as an argument and the
user_access_end() call is where the label is defined.

I tried first to avoid changing the uaccess API by defining a fixed
label inside user_write_access_end() and always doing "fake jumps" to
this fixed label from both user_write_access_begin() and
unsafe_get_user(). However, you run into trouble with functions like
sys_waitid() in kernel/exit.c that have multiple user_write_access_end()
calls.

You might think that you could switch them around -- define the label in
_begin() and fake-jump to it from _end(), but that doesn't work either
since _begin() needs to be an expression and labels defined inside a
statement expression are not accessible outside that expression...

Anyway, attached a proof of concept patch that fixes the objtool
warning, but I didn't even do a full build test.

Vegard
View attachment "uaccess.patch" of type "text/x-patch" (1049 bytes)