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

[Li Wang]

On Tue, Nov 19, 2024 at 9:32 PM Jan Stancek <jstancek@redhat.com> wrote:

> 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
>

That is a simple method to check that the buffer can be executed correctly.


> 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.
>

Yes, this could work as well, thanks for improving it.

P.s I am on traveling this week, so I wouldn't find a machine to test the
patch.


-- 
Regards,
Li Wang