segfaults of processes while being killed after commit "mm: make the page fault mmap locking killable"

[Fiona Ebner <f.ebner@proxmox.com>]

Hi,

we are seeing segfaults from processes while being killed starting with
kernels which include commit eda0047296a16d65a7f2bc60a408f70d178b2014
("mm: make the page fault mmap locking killable") all the way up to
v6.5-rc3 which is the kernel I based this report on.

I don't have a simple reproducer unfortunately, the one I have is big
and quite racy. My working theory for what happens is (see also the
bpftrace script and output [0]):

Since get_mmap_lock_carefully() now uses mmap_read_lock_killable(), if
rwsem_down_write_slowpath() is taken and there is a fatal signal
pending, rwsem_down_write_slowpath() will return -EINTR and this is
propagated up until get_mmap_lock_carefully() will return its boolean
negation with !mmap_read_lock_killable(mm), i.e. 0.

Then lock_mm_and_find_vma() returns NULL

>     if (!get_mmap_lock_carefully(mm, regs))
>         return NULL;
and so do_user_addr_fault()

>     vma = lock_mm_and_find_vma(mm, address, regs);
>     if (unlikely(!vma)) {
>         bad_area_nosemaphore(regs, error_code, address);
>         return;
>     }

will end up without a vma and cause/log the segfault. Of course the
process is already being killed, but I'd argue it is very confusing to
users when apparent segfaults from such processes are being logged by
the kernel.

Happy to provide other traces or information if required!

Best Regards,
Fiona

[0]:

I ended up with the following bpftrace script

> #include <linux/signal.h>
> #include <linux/sched.h>
> 
> kprobe:down_read_killable {
>     printf("%s %d %d\n", func, pid, tid);
> }
> 
> kprobe:rwsem_down_read_slowpath {
>     printf("%s %d %d\n", func, pid, tid);
> }
> 
> kretprobe:rwsem_down_read_slowpath {
>     printf("%s %d %d retval 0x%x\n", func, pid, tid, retval);
>     printf("%s\n", kstack());
> }
> 
> kprobe:bad_area_nosemaphore {
>     printf("%s %d %d %s pending signal: %d\n", func, pid, tid, comm,
>     curtask->pending.signal.sig[0]
>     );
>     if (curtask->pending.signal.sig[0]) {
> 	printf("%s\n", kstack());
>     }
> }

and here is a capture of a process running into the segfault. AFAIU, the
pending signal translates to SIGKILL and the return value from
down_read_killable() is -EINTR.

> down_read_killable 987299 987299
> rwsem_down_read_slowpath 987299 987299
> down_read_killable 987299 987299 retval 0xfffffffc
> 
>         down_read_killable+72
>         lock_mm_and_find_vma+167
>         do_user_addr_fault+477
>         exc_page_fault+131
>         asm_exc_page_fault+39
> 
> bad_area_nosemaphore 987299 987299 pverados pending signal: 256
> 
>         bad_area_nosemaphore+1
>         exc_page_fault+131
>         asm_exc_page_fault+39
> 
> bad_area_nosemaphore 987299 987299 pverados pending signal: 256
> 
>         bad_area_nosemaphore+1
>         exc_page_fault+131
>         asm_exc_page_fault+39
>         rep_movs_alternative+96
>         show_opcodes+118
>         __bad_area_nosemaphore+640
>         bad_area_nosemaphore+22
>         do_user_addr_fault+708
>         exc_page_fault+131
>         asm_exc_page_fault+39
> 
> bad_area_nosemaphore 987299 987299 pverados pending signal: 256
> 
>         bad_area_nosemaphore+1
>         exc_page_fault+131
>         asm_exc_page_fault+39
>         rep_movs_alternative+15
>         show_opcodes+118
>         __bad_area_nosemaphore+640
>         bad_area_nosemaphore+22
>         do_user_addr_fault+708
>         exc_page_fault+131
>         asm_exc_page_fault+39
>