[LTP] [PATCH] Revert "pkey01: Adding test for PKEY_DISABLE_EXECUTE"

[Jan Stancek]

On Tue, Nov 19, 2024 at 2:11 PM Jan Stancek <jstancek@redhat.com> wrote:
>
> On Fri, Nov 15, 2024 at 7:58 PM Petr Vorel <pvorel@suse.cz> wrote:
> >
> > Hi Li, Martin,
> >
> > > Sorry, I forgot to CC Li in the patch.
> >
> > > On 15. 11. 24 16:44, Martin Doucha wrote:
> > > > This reverts commit d2b8a476c29d52c484b387454082bbc906b0b4f8.
> >
> > > > Remove the PKEY_DISABLE_EXECUTE subtest. The function_size() code
> > > > is broken in a way that I cannot easily fix. The function tries
> > > > to calculate the size of a function by finding the first RET
> > > > instruction. However, in 32bit LTP builds, the code gets compiled
> > > > to this:
> >
> > > > 0804b690 <function_size>:
> > > >   804b690:       8b 4c 24 04             mov    0x4(%esp),%ecx
> > > >   804b694:       0f b6 01                movzbl (%ecx),%eax
> > > >   804b697:       83 c0 3e                add    $0x3e,%eax
> > > >   804b69a:       3c 01                   cmp    $0x1,%al
> > > >   804b69c:       76 1a                   jbe    804b6b8 <function_size+0x28>
> > > >   804b69e:       89 c8                   mov    %ecx,%eax
> > > >   804b6a0:       83 c0 01                add    $0x1,%eax
> > > >   804b6a3:       0f b6 10                movzbl (%eax),%edx
> > > >   804b6a6:       83 c2 3e                add    $0x3e,%edx
> > > >   804b6a9:       80 fa 01                cmp    $0x1,%dl
> > > >   804b6ac:       77 f2                   ja     804b6a0 <function_size+0x10>
> > > >   804b6ae:       29 c8                   sub    %ecx,%eax
> > > >   804b6b0:       83 c0 10                add    $0x10,%eax
> > > >   804b6b3:       c3                      ret
> > > >   804b6b4:       8d 74 26 00             lea    0x0(%esi,%eiz,1),%esi
> > > >   804b6b8:       b8 10 00 00 00          mov    $0x10,%eax
> > > >   804b6bd:       c3                      ret
> > > >   804b6be:       66 90                   xchg   %ax,%ax
> >
> > > > If you look closely enough, you'll notice a C2 byte in add $0x3e,%edx
> > > > instruction on address 804b6a6. The function will assume this byte is
> > > > a RET instruction, return a size that's 22 bytes too short and then
> > > > the code execution inside the executable buffer will run past the end
> > > > of buffer, resulting in a segfault.
> >
> > Martin, thanks a lot for debugging!
> >
> > Acked-by: Petr Vorel <pvorel@suse.cz>
> >
> > Other option would be to disable this test only for 32bit (keep it for 64bit).
> > Li, any change you could have look into it?
>
> I vaguely recall we already dealt with something similar for a
> different test, which
> copied function, then changed protection to EXEC and finally ran it.
> I'll see if I can find it.

Do we need function_size() to return accurate number? In mprotect04
we ended up using CFLAGS += -falign-functions=64 and copying everything
until end of page. Function was dummy, so it always fit and it's not an
issue if we copied more.