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

From: Tijl Coosemans <tijl_at_FreeBSD.org>
Date: Thu, 08 Sep 2022 16:36:26 UTC
On Thu, 8 Sep 2022 14:49:09 +0200 Tijl Coosemans <tijl@FreeBSD.org> wrote:
> My patch adds a trylock after cli and an unlock before sti.  If the
> trylock fails panic("") is called.  I put the lock at the base of the
> trampoline stack so it's per cpu.  The code between cli and sti is
> supposed to be uninterruptible so the trylock should never fail, but it
> does!  Somehow there's a context switch to another thread that calls
> copyout or copyin while the first thread was still busy copying when it
> was switched out.  This corrupts the trampoline stack and/or copyout_buf
> of the first thread.

I managed to find such a pair of threads.

This is the first thread calling copyin, handling a page fault and
handling an interrupt on top of the page fault:

(kgdb) thread 202
[Switching to thread 202 (Thread 101397)]
(kgdb) bt
#0  0x009c55b6 in sched_switch (td=0x1a7fb000, flags=1538)
    at /home/tijl/freebsd/base/main/sys/kern/sched_4bsd.c:1099
#1  0x009abada in mi_switch (flags=1538)
    at /home/tijl/freebsd/base/main/sys/kern/kern_synch.c:545
#2  0x009a9d60 in critical_exit_preempt ()
    at /home/tijl/freebsd/base/main/sys/kern/kern_switch.c:245
#3  0x0097507e in critical_exit ()
    at /home/tijl/freebsd/base/main/sys/sys/systm.h:196
#4  0x00974fde in intr_event_handle (ie=0x4858e00, frame=0x1b076844)
    at /home/tijl/freebsd/base/main/sys/kern/kern_intr.c:1470
#5  0x00b7a86e in intr_execute_handlers (isrc=0xc8270c <atintrs+224>, 
    frame=0x1b076844)
    at /home/tijl/freebsd/base/main/sys/x86/x86/intr_machdep.c:357
#6  0x00bad840 in atpic_handle_intr (vector=4, frame=0x1b076844)
    at /home/tijl/freebsd/base/main/sys/x86/isa/atpic.c:553
#7  0xffc03845 in ?? ()
#8  0x00000004 in ?? ()
#9  0x1b076844 in ?? ()
(kgdb) info sym 0xffc03845-setidt_disp 
Xatpic_intr4_pti + 197 in section .text of /boot/kernel/kernel

Further backtrace reconstructed with "frame view":

#0  sse2_pagezero ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/support.s:56
56		movnti	%ebx,(%ecx)
#0  0x00b9aef5 in pagezero ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/pmap.c:4497
4497				sse2_pagezero(page);
#0  pmap_nopae_zero_page ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/pmap.c:4524
4524		*cmap_pte2 = 0;
#0  0x00b94bb1 in pmap_zero_page ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/pmap_base.c:668
668		pmap_methods_ptr->pm_zero_page(m);
#0  0x00b31f02 in vm_fault_zerofill ()
    at /home/tijl/freebsd/base/main/sys/vm/vm_fault.c:1114
1114			pmap_zero_page(fs->m);
#0  0x00b31453 in vm_fault ()
    at /home/tijl/freebsd/base/main/sys/vm/vm_fault.c:1602
1602			vm_fault_zerofill(&fs);
#0  0x00b311c4 in vm_fault_trap ()
    at /home/tijl/freebsd/base/main/sys/vm/vm_fault.c:683
683		result = vm_fault(map, trunc_page(vaddr), fault_type, fault_flags,
#0  0x00ba7829 in trap_pfault ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/trap.c:849
849		rv = vm_fault_trap(map, eva, ftype, VM_FAULT_NORMAL, signo, ucode);
#0  0x00ba6ec4 in trap ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/trap.c:474
474				(void)trap_pfault(frame, false, eva, NULL, NULL);
(kgdb) x/3x 0x1b076a84
0x1b076a84:	0x1b076a90	0xffc031ff	0x1b076a90
(kgdb) info sym 0xffc031ff-setidt_disp 
calltrap + 8 in section .text of /boot/kernel/kernel
(kgdb) p/x *(struct trapframe *)0x1b076a90
$47 = {tf_fs = 0x8, tf_es = 0x28, tf_ds = 0x28, tf_edi = 0xe25c000, 
  tf_esi = 0xffc04f80, tf_ebp = 0x1b076adc, tf_isp = 0x1b076abc, 
  tf_ebx = 0x141e000, tf_edx = 0xb8da80, tf_ecx = 0x1, tf_eax = 0x1b076ad0, 
  tf_trapno = 0xc, tf_err = 0x2, tf_eip = 0xffc04362, tf_cs = 0x20, 
  tf_eflags = 0x210017, tf_esp = 0xffbfc5a0, tf_ss = 0x1}
(kgdb) info sym 0xffc04362-setidt_disp
copyin_fast + 106 in section .text of /boot/kernel/kernel
(kgdb) frame view 0x1b076adc 0xffc04362-setidt_disp
#0  0x00b8da62 in copyin_fast ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/copyout_fast.s:133
133		rep; movsb
(kgdb) disas 0xffc04362-setidt_disp
Dump of assembler code for function copyin_fast:
   0x00b8d9f8 <+0>:	push   %ebp
   0x00b8d9f9 <+1>:	mov    %esp,%ebp
   0x00b8d9fb <+3>:	push   %esi
   0x00b8d9fc <+4>:	push   %edi
   0x00b8d9fd <+5>:	push   %ebx
   0x00b8d9fe <+6>:	mov    0x14(%ebp),%ebx
   0x00b8da01 <+9>:	mov    %fs:0x10,%eax
   0x00b8da07 <+15>:	mov    0x44(%eax),%edx
   0x00b8da0a <+18>:	mov    0x10(%ebp),%ecx
   0x00b8da0d <+21>:	mov    0x8(%ebp),%esi
   0x00b8da10 <+24>:	cli    
   0x00b8da11 <+25>:	mov    %fs:0x120,%edi
   0x00b8da18 <+32>:	mov    $0xdeadc0de,%eax
   0x00b8da1d <+37>:	xchg   %eax,-0xff0(%edi)
   0x00b8da23 <+43>:	cmp    $0xdeadc0de,%eax
   0x00b8da28 <+48>:	jne    0xb8da36 <copyin_fast+62>
   0x00b8da2a <+50>:	push   $0xb8da7d
   0x00b8da2f <+55>:	mov    $0x9a20b0,%eax
   0x00b8da34 <+60>:	call   *%eax
   0x00b8da36 <+62>:	mov    %fs:0x194,%edi
   0x00b8da3d <+69>:	mov    %esp,%eax
   0x00b8da3f <+71>:	mov    %fs:0x120,%esp
   0x00b8da46 <+78>:	mov    %edx,%cr3
   0x00b8da49 <+81>:	mov    $0xb8da80,%edx
   0x00b8da4e <+86>:	rep movsb %ds:(%esi),%es:(%edi)
   0x00b8da50 <+88>:	mov    %ebx,%cr3
   0x00b8da53 <+91>:	mov    %eax,%esp
   0x00b8da55 <+93>:	mov    0x10(%ebp),%ecx
   0x00b8da58 <+96>:	mov    0xc(%ebp),%edi
   0x00b8da5b <+99>:	mov    %fs:0x194,%esi
   0x00b8da62 <+106>:	rep movsb %ds:(%esi),%es:(%edi)
   0x00b8da64 <+108>:	mov    %fs:0x120,%edi
   0x00b8da6b <+115>:	movl   $0x0,-0xff0(%edi)
   0x00b8da75 <+125>:	sti    
   0x00b8da76 <+126>:	xor    %eax,%eax
   0x00b8da78 <+128>:	pop    %ebx
   0x00b8da79 <+129>:	pop    %edi
   0x00b8da7a <+130>:	pop    %esi
   0x00b8da7b <+131>:	leave  
   0x00b8da7c <+132>:	ret    
   0x00b8da7d <+133>:	add    %ah,-0x70(%esi)
End of assembler dump.
#0  0x00b89208 in copyin ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/copyout.c:215
215		    copyin_fast_tramp(udaddr, kaddr, len, pmap_get_kcr3()) == 0))
#0  0x009ea24e in uiomove_faultflag ()
    at /home/tijl/freebsd/base/main/sys/kern/subr_uio.c:255
255					error = copyin(iov->iov_base, cp, cnt);
#0  0x009ea1a3 in uiomove ()
    at /home/tijl/freebsd/base/main/sys/kern/subr_uio.c:198
198		return (uiomove_faultflag(cp, n, uio, 0));
#0  0x009f2ae0 in pipe_write ()
    at /home/tijl/freebsd/base/main/sys/kern/sys_pipe.c:1252
1252				error = uiomove(&wpipe->pipe_buffer.buffer[wpipe->pipe_buffer.in],
#0  0x009f1dc4 in fo_write ()
    at /home/tijl/freebsd/base/main/sys/sys/file.h:349
349		return ((*fp->f_ops->fo_write)(fp, uio, active_cred, flags, td));
#0  0x009efac9 in dofilewrite ()
    at /home/tijl/freebsd/base/main/sys/kern/sys_generic.c:565
565		if ((error = fo_write(fp, auio, td->td_ucred, flags, td))) {
#0  0x009ef8cb in kern_writev ()
    at /home/tijl/freebsd/base/main/sys/kern/sys_generic.c:492
492		error = dofilewrite(td, fd, fp, auio, (off_t)-1, 0);
#0  0x009ef880 in sys_write ()
    at /home/tijl/freebsd/base/main/sys/kern/sys_generic.c:407
407		error = kern_writev(td, uap->fd, &auio);
#0  0x00ba7c00 in syscallenter ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/../../kern/subr_syscall.c:189
189			error = (se->sy_call)(td, sa->args);
#0  syscall ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/trap.c:1137
1137		if ((orig_tf_eflags & PSL_T) && !(orig_tf_eflags & PSL_VM)) {



This is the second thread calling copyout:

(kgdb) thread 192
[Switching to thread 192 (Thread 101428)]
(kgdb) bt
#0  0x009a1dfd in dump_savectx ()
    at /home/tijl/freebsd/base/main/sys/kern/kern_shutdown.c:404
#1  0x009a1e77 in doadump (textdump=0)
    at /home/tijl/freebsd/base/main/sys/kern/kern_shutdown.c:434
#2  0x008d1194 in db_dump (dummy=10315780, dummy2=false, dummy3=-1, 
    dummy4=0x1b0bc850 "")
    at /home/tijl/freebsd/base/main/sys/ddb/db_command.c:593
#3  0x008d0f41 in db_command (last_cmdp=<optimized out>, 
    cmd_table=<optimized out>, dopager=true)
    at /home/tijl/freebsd/base/main/sys/ddb/db_command.c:506
#4  0x008d0ddc in db_command_loop ()
    at /home/tijl/freebsd/base/main/sys/ddb/db_command.c:553
#5  0x008d3b00 in db_trap (type=3, code=0)
    at /home/tijl/freebsd/base/main/sys/ddb/db_main.c:270
#6  0x009d6b89 in kdb_trap (type=3, code=0, tf=0x1b0bca14)
    at /home/tijl/freebsd/base/main/sys/kern/subr_kdb.c:745
#7  0x00ba6e9e in trap (frame=0x1b0bca14)
    at /home/tijl/freebsd/base/main/sys/i386/i386/trap.c:667
#8  0xffc031ff in ?? ()
#9  0x1b0bca14 in ?? ()
(kgdb) info sym 0xffc031ff-setidt_disp 
calltrap + 8 in section .text of /boot/kernel/kernel

Reconstructed backtrace:

#0  breakpoint ()
    at /home/tijl/freebsd/base/main/sys/i386/include/cpufunc.h:57
57	}
#0  kdb_enter ()
    at /home/tijl/freebsd/base/main/sys/kern/subr_kdb.c:509
509			kdb_why = KDB_WHY_UNSET;
#0  0x009a21e6 in vpanic ()
    at /home/tijl/freebsd/base/main/sys/kern/kern_shutdown.c:966
966			kdb_enter(KDB_WHY_PANIC, "panic");
#0  0x009a20c4 in panic ()
    at /home/tijl/freebsd/base/main/sys/kern/kern_shutdown.c:903
903		vpanic(fmt, ap);
#0  0x00b8d9af in copyout_fast ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/copyout_fast.s:65
65		movl	PCPU(COPYOUT_BUF),%edi
Dump of assembler code for function copyout_fast:
   0x00b8d970 <+0>:	push   %ebp
   0x00b8d971 <+1>:	mov    %esp,%ebp
   0x00b8d973 <+3>:	push   %esi
   0x00b8d974 <+4>:	push   %edi
   0x00b8d975 <+5>:	push   %ebx
   0x00b8d976 <+6>:	mov    0x14(%ebp),%ebx
   0x00b8d979 <+9>:	mov    %fs:0x10,%edx
   0x00b8d980 <+16>:	mov    0x44(%edx),%edx
   0x00b8d983 <+19>:	mov    0x10(%ebp),%ecx
   0x00b8d986 <+22>:	mov    0x8(%ebp),%esi
   0x00b8d989 <+25>:	cli    
   0x00b8d98a <+26>:	mov    %fs:0x120,%edi
   0x00b8d991 <+33>:	mov    $0xdeadc0de,%eax
   0x00b8d996 <+38>:	xchg   %eax,-0xff0(%edi)
   0x00b8d99c <+44>:	cmp    $0xdeadc0de,%eax
   0x00b8d9a1 <+49>:	jne    0xb8d9af <copyout_fast+63>
   0x00b8d9a3 <+51>:	push   $0xb8da7d
   0x00b8d9a8 <+56>:	mov    $0x9a20b0,%eax
   0x00b8d9ad <+61>:	call   *%eax
   0x00b8d9af <+63>:	mov    %fs:0x194,%edi
   0x00b8d9b6 <+70>:	rep movsb %ds:(%esi),%es:(%edi)
   0x00b8d9b8 <+72>:	mov    0x10(%ebp),%ecx
   0x00b8d9bb <+75>:	mov    %fs:0x194,%esi
   0x00b8d9c2 <+82>:	mov    0xc(%ebp),%edi
   0x00b8d9c5 <+85>:	mov    %esp,%eax
   0x00b8d9c7 <+87>:	mov    %fs:0x120,%esp
   0x00b8d9ce <+94>:	mov    %edx,%cr3
   0x00b8d9d1 <+97>:	mov    $0xb8da80,%edx
   0x00b8d9d6 <+102>:	rep movsb %ds:(%esi),%es:(%edi)
   0x00b8d9d8 <+104>:	mov    %ebx,%cr3
   0x00b8d9db <+107>:	mov    %eax,%esp
   0x00b8d9dd <+109>:	mov    %fs:0x120,%edi
   0x00b8d9e4 <+116>:	movl   $0x0,-0xff0(%edi)
   0x00b8d9ee <+126>:	sti    
   0x00b8d9ef <+127>:	xor    %eax,%eax
   0x00b8d9f1 <+129>:	pop    %ebx
   0x00b8d9f2 <+130>:	pop    %edi
   0x00b8d9f3 <+131>:	pop    %esi
   0x00b8d9f4 <+132>:	leave  
   0x00b8d9f5 <+133>:	ret    
End of assembler dump.
#0  0x00b892f8 in copyout ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/copyout.c:250
250		    copyout_fast_tramp(kaddr, udaddr, len, pmap_get_kcr3()) == 0))
#0  0x009f177c in pollout ()
    at /home/tijl/freebsd/base/main/sys/kern/sys_generic.c:1687
1687			error = copyout(&fds->revents, &ufds->revents,
#0  0x009f1233 in kern_poll ()
    at /home/tijl/freebsd/base/main/sys/kern/sys_generic.c:1607
1607			error = pollout(td, kfds, ufds, nfds);
#0  0x009f1194 in sys_poll ()
    at /home/tijl/freebsd/base/main/sys/kern/sys_generic.c:1485
1485		return (kern_poll(td, uap->fds, uap->nfds, tsp, NULL));
#0  0x00ba7c00 in syscallenter ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/../../kern/subr_syscall.c:189
189			error = (se->sy_call)(td, sa->args);
#0  syscall ()
    at /home/tijl/freebsd/base/main/sys/i386/i386/trap.c:1137
1137		if ((orig_tf_eflags & PSL_T) && !(orig_tf_eflags & PSL_VM)) {