Date: Wed, 7 Sep 2022 18:38:04 +0200 From: =?UTF-8?B?VMSzbA==?= Coosemans <tijl@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: pho@freebsd.org, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 95f773e59482 - main - i386 copyout_fast: improve detection of a fault on accessing userspace Message-ID: <20220907183804.29829b14@FreeBSD.org> In-Reply-To: <20220906231745.1a0f3c15@FreeBSD.org> References: <202208241925.27OJP9Fh069091@gitrepo.freebsd.org> <20220906171826.1629cfcf@FreeBSD.org> <YxdneZXqyaAPmVL0@kib.kiev.ua> <20220906231745.1a0f3c15@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--MP_/nCzBi7cNE=+_XY7edmAIGcT Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Tue, 6 Sep 2022 23:17:45 +0200 T=C4=B3l Coosemans <tijl@FreeBSD.org> wro= te: > 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.= =20 >=20 > Yes, it looks like this: >=20 > panic: vm_fault_lookup: fault on nofault entry, addr: 0 >=20 > 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.h= tml> > 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/>. >=20 > 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... >=20 > Unread portion of the kernel message buffer: > panic: vm_fault_lookup: fault on nofault entry, addr: 0 > time =3D 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 0xff= c09dac > 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 =3D 0x3b, esp =3D 0xffc09fac, ebp =3D 0xffc033dc --- > (null)() at 0x3b/frame 0xffc033dc > KDB: enter: panic >=20 > 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 =3D 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:=20 time =3D 1662560141 KDB: stack backtrace: db_trace_self_wrapper(5,ffffffff,0,0,0,...) at db_trace_self_wrapper+0x28/f= rame 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 0x1= b0bcaac 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 =3D 0x2058c67b, esp =3D 0xff= bfc524, ebp =3D 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. --MP_/nCzBi7cNE=+_XY7edmAIGcT Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=i386-copyout_fast-reent.patch --- sys/i386/i386/copyout_fast.s.kib 2022-09-06 20:58:04.682001000 +0200 +++ sys/i386/i386/copyout_fast.s 2022-09-07 18:29:56.219998000 +0200 @@ -53,6 +53,15 @@ ENTRY(copyout_fast) movl 16(%ebp),%ecx movl 8(%ebp),%esi cli + movl PCPU(TRAMPSTK),%edi + movl $0xdeadc0de,%eax + xchgl %eax,(VM86_STACK_SPACE-4096)(%edi) + cmpl $0xdeadc0de,%eax + jne 1f + pushl $2f + movl $panic,%eax + call *%eax +1: movl PCPU(COPYOUT_BUF),%edi rep; movsb @@ -71,6 +80,8 @@ pf_x1: rep; movsb movl %ebx,%cr3 movl %eax,%esp + movl PCPU(TRAMPSTK),%edi + movl $0,(VM86_STACK_SPACE-4096)(%edi) sti xorl %eax,%eax @@ -94,6 +105,15 @@ ENTRY(copyin_fast) movl 16(%ebp),%ecx /* len */ movl 8(%ebp),%esi /* udaddr */ cli + movl PCPU(TRAMPSTK),%edi + movl $0xdeadc0de,%eax + xchgl %eax,(VM86_STACK_SPACE-4096)(%edi) + cmpl $0xdeadc0de,%eax + jne 1f + pushl $2f + movl $panic,%eax + call *%eax +1: movl PCPU(COPYOUT_BUF),%edi /* kaddr */ movl %esp,%eax @@ -112,6 +132,8 @@ pf_x2: rep; movsb movl PCPU(COPYOUT_BUF),%esi rep; movsb + movl PCPU(TRAMPSTK),%edi + movl $0,(VM86_STACK_SPACE-4096)(%edi) sti xorl %eax,%eax @@ -120,11 +142,14 @@ pf_x2: rep; movsb popl %esi leave ret +2: .zero 1 END(copyin_fast) ALIGN_TEXT copyout_fault: movl %eax,%esp + movl PCPU(TRAMPSTK),%edi + movl $0,(VM86_STACK_SPACE-4096)(%edi) sti movl $EFAULT,%eax popl %ebx --MP_/nCzBi7cNE=+_XY7edmAIGcT--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20220907183804.29829b14>