Hi Linus,
please pull two urgent objtool fixes for 6.5.
Thx.
---
The following changes since commit 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5:
Linux 6.5-rc1 (2023-07-09 13:53:13 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git tags/objtool_urgent_for_v6.5_rc2
for you to fetch changes up to 719a937b7003933de1298ffa4b881dd6a234e244:
iov_iter: Mark copy_iovec_from_user() noclone (2023-07-10 09:52:28 +0200)
----------------------------------------------------------------
- Mark copy_iovec_from_user() __noclone in order to prevent gcc from
doing an inter-procedural optimization and confuse objtool
- Initialize struct elf fully to avoid build failures
----------------------------------------------------------------
Michal Kubecek (1):
objtool: initialize all of struct elf
Peter Zijlstra (1):
iov_iter: Mark copy_iovec_from_user() noclone
lib/iov_iter.c | 2 +-
tools/objtool/elf.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Sun, 16 Jul 2023 at 11:42, Borislav Petkov <[email protected]> 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..
Linus
The pull request you sent on Sun, 16 Jul 2023 20:42:04 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git tags/objtool_urgent_for_v6.5_rc2
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/8a3e4a64849eb9da0e8c7e693978499562581631
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
On 7/16/23 22:39, Linus Torvalds wrote:
> On Sun, 16 Jul 2023 at 11:42, Borislav Petkov <[email protected]> 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
On Tue, 18 Jul 2023 at 04:02, Vegard Nossum <[email protected]> wrote:
>
> If you are willing to add another helper function that also takes the
> label argument, you could do something like this:
Ugh. That's a bit uglier than I was hoping for.
I was hoping more like "use this special attribute to mark this asm as
being something that cannot be moved across function boundaries".
Now a "we have to make up horrible games and pass in labels that are
not used and use a completely different API".
Oh well.
I guess in the end we can just continue to work around this.
After all, in many ways this isn't about the code - the AC setting and
clearing is fine - it's about objtool not doing the analysis across
function boundaries.
I guess we could have just marked these iovec copy functions
"always_inline" too, and avoided it that way.
Linus