[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAFULd4YgiPA3+2zu67WrhASzad5W74MNbbE6sZyZrSFX8kA8Qw@mail.gmail.com>
Date: Wed, 2 Apr 2025 09:15:27 +0200
From: Uros Bizjak <ubizjak@...il.com>
To: Ingo Molnar <mingo@...nel.org>
Cc: Mateusz Guzik <mjguzik@...il.com>, mingo@...hat.com, x86@...nel.org,
linux-kernel@...r.kernel.org, Linus Torvalds <torvalds@...ux-foundation.org>,
Peter Zijlstra <peterz@...radead.org>, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] x86: predict __access_ok() returning true
On Tue, Apr 1, 2025 at 10:35 PM Ingo Molnar <mingo@...nel.org> wrote:
>
>
> * Mateusz Guzik <mjguzik@...il.com> wrote:
>
> > This works around what seems to be an optimization bug in gcc (at least
> > 13.3.0), where it predicts access_ok() to fail despite the hint to the
> > contrary.
> >
> > _copy_to_user contains:
> > if (access_ok(to, n)) {
> > instrument_copy_to_user(to, from, n);
> > n = raw_copy_to_user(to, from, n);
> > }
> >
> > Where access_ok is likely(__access_ok(addr, size)), yet the compiler
> > emits conditional jumps forward for the case where it succeeds:
> >
> > <+0>: endbr64
> > <+4>: mov %rdx,%rcx
> > <+7>: mov %rdx,%rax
> > <+10>: xor %edx,%edx
> > <+12>: add %rdi,%rcx
> > <+15>: setb %dl
> > <+18>: movabs $0x123456789abcdef,%r8
> > <+28>: test %rdx,%rdx
> > <+31>: jne 0xffffffff81b3b7c6 <_copy_to_user+38>
> > <+33>: cmp %rcx,%r8
> > <+36>: jae 0xffffffff81b3b7cb <_copy_to_user+43>
> > <+38>: jmp 0xffffffff822673e0 <__x86_return_thunk>
> > <+43>: nop
> > <+44>: nop
> > <+45>: nop
> > <+46>: mov %rax,%rcx
> > <+49>: rep movsb %ds:(%rsi),%es:(%rdi)
> > <+51>: nop
> > <+52>: nop
> > <+53>: nop
> > <+54>: mov %rcx,%rax
> > <+57>: nop
> > <+58>: nop
> > <+59>: nop
> > <+60>: jmp 0xffffffff822673e0 <__x86_return_thunk>
> >
> > Patching _copy_to_user() to likely() around the access_ok() use does
> > not change the asm.
> >
> > However, spelling out the prediction *within* __access_ok() does the
> > trick:
> > <+0>: endbr64
> > <+4>: xor %eax,%eax
> > <+6>: mov %rdx,%rcx
> > <+9>: add %rdi,%rdx
> > <+12>: setb %al
> > <+15>: movabs $0x123456789abcdef,%r8
> > <+25>: test %rax,%rax
> > <+28>: jne 0xffffffff81b315e6 <_copy_to_user+54>
> > <+30>: cmp %rdx,%r8
> > <+33>: jb 0xffffffff81b315e6 <_copy_to_user+54>
> > <+35>: nop
> > <+36>: nop
> > <+37>: nop
> > <+38>: rep movsb %ds:(%rsi),%es:(%rdi)
> > <+40>: nop
> > <+41>: nop
> > <+42>: nop
> > <+43>: nop
> > <+44>: nop
> > <+45>: nop
> > <+46>: mov %rcx,%rax
> > <+49>: jmp 0xffffffff82255ba0 <__x86_return_thunk>
> > <+54>: mov %rcx,%rax
> > <+57>: jmp 0xffffffff82255ba0 <__x86_return_thunk>
Mateusz, can you please file a bug in GCC bug tracker [1], following
the instructions in [2]?
[1] https://gcc.gnu.org/bugzilla/
[2] https://gcc.gnu.org/bugs/
Thanks,
Uros.
Powered by blists - more mailing lists