Re: [syzbot] general protection fault in PageHeadHuge

[Peter Xu <peterx@redhat.com>]

On Tue, Sep 27, 2022 at 09:24:37AM -0700, Mike Kravetz wrote:
> This should guarantee a read fault independent of what pthread_mutex_lock
> does.  However, it still results in the occasional "ERROR: unexpected write
> fault".  So, something else if happening.  I will continue to experiment
> and think about this.

Thanks for verifying this, Mike.  I didn't yet reply but I did have some
update on my side too.  I plan to look deeper and wanted to reply until
that, because I do think there's something special on hugetlb and I still
don't know. I just keep getting distracted by other things.. but maybe I
should still share out what I have already.

I think I already know what had guaranteed the read faults - the NPTL
pthread lib implemented mutex in different types, and the 1st instruction
of lock() is to fetch the mutex type (at offset 0x10) then we know how we
should move on:

(gdb) disas pthread_mutex_lock
Dump of assembler code for function ___pthread_mutex_lock:
   0x00007ffff7e3b0d0 <+0>:     endbr64 
   0x00007ffff7e3b0d4 <+4>:     mov    0x10(%rdi),%eax       <---- read 0x10 of &mutex
   0x00007ffff7e3b0d7 <+7>:     mov    %eax,%edx
   0x00007ffff7e3b0d9 <+9>:     and    $0x17f,%edx
(gdb) ptype pthread_mutex_t
type = union {
    struct __pthread_mutex_s __data;
    char __size[40];
    long __align;
}
(gdb) ptype struct __pthread_mutex_s
type = struct __pthread_mutex_s {
    int __lock;
    unsigned int __count;
    int __owner;
    unsigned int __nusers;
    int __kind;                                              <--- 0x10 offset here
    short __spins;
    short __elision;
    __pthread_list_t __list;
}

I looked directly at asm dumped from the binary I tested to make sure it's
accurate.  So it means with current uffd selftest logically there should
never be a write missing fault (I still don't think it's good to have the
write check though.. but that's separate question to ask).

It also means hugetlb does something special here.  It smells really like
for some reason the hugetlb pgtable got evicted after UFFDIO_COPY during
locking_thread running, then any further lock() (e.g. cmpxchg) or modifying
the counter could trigger a write fault.

OTOH this also explained why futex code was never tested on userfaultfd
selftest, simply because futex() will always to be after that "read the
type of mutex" thing.. which is something I want to rework a bit, so as to
have uffd selftest to cover gup as you used to rightfully suggest.  But
that'll be nothing urgent, and be something after we know what's special
with hugetlb new code.

I'll also keep update if I figured something more out of it.

-- 
Peter Xu