Re: git: 95f773e59482 - main - i386 copyout_fast: improve detection of a fault on accessing userspace

On Tue, 6 Sep 2022 23:17:45 +0200 Tijl Coosemans <tijl@FreeBSD.org> wrote:
> On Tue, 6 Sep 2022 18:30:01 +0300 Konstantin Belousov
> <kostikbel@gmail.com> wrote:
>> I suspect you see that leftover panics, which I am working on right now.  
> 
> Yes, it looks like this:
> 
> panic: vm_fault_lookup: fault on nofault entry, addr: 0
> 
> GNU gdb (GDB) 12.1 [GDB v12.1 for FreeBSD]
> Copyright (C) 2022 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> Type "show copying" and "show warranty" for details.
> This GDB was configured as "i386-portbld-freebsd14.0".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <https://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
>     <http://www.gnu.org/software/gdb/documentation/>.
> 
> For help, type "help".
> Type "apropos word" to search for commands related to "word"...
> Reading symbols from /boot/kernel/kernel...
> Reading symbols from /usr/lib/debug//boot/kernel/kernel.debug...
> 
> Unread portion of the kernel message buffer:
> panic: vm_fault_lookup: fault on nofault entry, addr: 0
> time = 1662487582
> KDB: stack backtrace:
> db_trace_self_wrapper(5,ffffffff,0,0,0,...) at db_trace_self_wrapper+0x28/frame 0xffc09cf4
> kdb_backtrace(1a716ae0,100,e340e0,ffc09db4,ffc09e00,...) at kdb_backtrace+0x2b/frame 0xffc09d4c
> vpanic(bef316,ffc09d88,ffc09d88,ffc09dac,b3181a,...) at vpanic+0xe9/frame 0xffc09d68
> panic(bef316,be68d1,0,ffc09dc4,480f308,...) at panic+0x14/frame 0xffc09d7c
> vm_fault_lookup(0,0,1a710701,0,0,...) at vm_fault_lookup+0x13a/frame 0xffc09dac
> vm_fault(e340e0,0,1,0,0) at vm_fault+0x79/frame 0xffc09e30
> vm_fault_trap(e340e0,3b,1,0,0,0) at vm_fault_trap+0x44/frame 0xffc09e58
> trap_pfault(3b,0,0) at trap_pfault+0x119/frame 0xffc09ea0
> trap(ffc09f6c,8,28,28,e247000,...) at trap+0x2d4/frame 0xffc09f60
> calltrap() at 0xffc031ff/frame 0xffc09f60
> --- trap 0xc, eip = 0x3b, esp = 0xffc09fac, ebp = 0xffc033dc ---
> (null)() at 0x3b/frame 0xffc033dc
> KDB: enter: panic
> 
> 0x009a1dfd in dump_savectx ()
>     at /home/tijl/freebsd/base/main/sys/kern/kern_shutdown.c:404
> (kgdb) #0  0x009a1dfd in dump_savectx ()
>     at /home/tijl/freebsd/base/main/sys/kern/kern_shutdown.c:404
> Backtrace stopped: Cannot access memory at address 0xffc09af4

The eip = 0x3b here looks like the size of a copy so I added a test on
top of your patch to see if copyout_fast or copyin_fast was called a
second time (see attached patch) because that would push the size where
eip is in the trapframe.  This triggered after running for a while with
this backtrace:

Unread portion of the kernel message buffer:
panic: 
time = 1662560141
KDB: stack backtrace:
db_trace_self_wrapper(5,ffffffff,0,0,0,...) at db_trace_self_wrapper+0x28/frame 0x1b0bca0c
kdb_backtrace(1c1f93a0,100,1b0bcb1a,ffc06ff0,141e000,...) at kdb_backtrace+0x2b/frame 0x1b0bca64
vpanic(b8da7d,1b0bcaa0,1b0bcaa0,1b0bcaac,ffc042af,...) at vpanic+0xe9/frame 0x1b0bca80
panic(b8da7d,fb028f96,2,0,1b0bcadc,...) at panic+0x14/frame 0x1b0bca94
__stop_set_sysinit_set(1b0bcb1a,fb028f96,2,141e000) at 0xffc042af/frame 0x1b0bcaac
copyout(1b0bcb1a,fb028f96,2) at copyout+0x58/frame 0x1b0bcadc
pollout(fb028f90,2) at pollout+0x3c/frame 0x1b0bcb04
kern_poll(1c1f93a0,fb028f90,2,1b0bcc3c,0) at kern_poll+0x83/frame 0x1b0bcc20
sys_poll(1c1f93a0,1c1f9644) at sys_poll+0x54/frame 0x1b0bcc48
syscallenter(1,0,1c1f93a0,1b0bcd30,480f318,...) at syscallenter+0xc0/frame 0x1b0bcc78
syscall(1b0bcce8,3b,3b,3b,3dfb1300,...) at syscall+0x28/frame 0x1b0bccdc
Xint0x80_syscall() at 0xffc03419/frame 0x1b0bccdc
--- syscall (4, FreeBSD ELF32, sys_write), eip = 0x2058c67b, esp = 0xffbfc524, ebp = 0x1 ---
KDB: enter: panic


So interrupts must have been reenabled somehow, probably by the page
fault handler, and this allows context switches and then another process
can call copyout or copyin and corrupt the trampoline stack and
copyout_buf.