Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 13 Sep 2006 19:55:09 -0500
From:      "Christian S.J. Peron" <csjp@FreeBSD.org>
To:        Martin Blapp <mb@imp.ch>
Cc:        src-committers@FreeBSD.org, jhb@FreeBSD.org, cvs-src@FreeBSD.org, cvs-all@FreeBSD.org, phk@FreeBSD.org, bde@FreeBSD.org
Subject:   Re: cvs commit: src/sys/kern kern_exit.c (DEVFS bug) ?
Message-ID:  <4508A86D.5030406@FreeBSD.org>
In-Reply-To: <20060913234613.S1494@godot.imp.ch>
References:  <200609131547.k8DFlrmv012940@repoman.freebsd.org> <20060913234613.S1494@godot.imp.ch>

next in thread | previous in thread | raw e-mail | index | archive | help
Aye

This is one of the panics which plagued me. If I remember correctly, 
this particular race rears it's ugly head in situations where you are 
opening and revoking/closing TTY devices. I seem to recall that the the 
devfs<->tty interactions were suspect here. I've also seen this problem 
this RELENG_5.

As I mentioned, I think my recent commit to kern_exit.c (1.290) should 
conceptually take care of some races, but certainly will not fix all the 
problems associated with the TTY/DEVFS code.

Although, my knowledge around TTYs and the internal inner workings of 
DEVFS is quite limited in scope.

Martin Blapp wrote:
>
> Hi all,
>
> The kernel I've made with 'mtx_assert(&Giant, MA_OWNED);' in all tty 
> functions
> works fine. I'm not able to crash it yet, even under 24 hour load.
>
>> But I have also seen what appears to be strange interactions or races 
>> between devfs and the TTY code before the Giant push down here, which 
>> was causing me some problems. After some discussions with John and 
>> Bruce, it looks like the manipulation of t_session should be 
>> protected by Giant.
>
> I Agree. This backout doesn't solve this panic here. I got this panic 
> on FreeBSD 5.3 RELEASE and  5.4 RELEASE too. Exactly the same trace. 
> And revision 1.272 never made it into RELENG_5.
>
> #1  0xc066355e in boot (howto=260) at 
> /usr/src/sys/kern/kern_shutdown.c:409
> #2  0xc06638b5 in panic (fmt=0xc0891732 "%s") at 
> /usr/src/sys/kern/kern_shutdown.c:565
> #3  0xc085c6b6 in trap_fatal (frame=0xed6e4ab8, eva=4) at 
> /usr/src/sys/i386/i386/trap.c:836
> #4  0xc085c3bf in trap_pfault (frame=0xed6e4ab8, usermode=0, eva=4) at 
> /usr/src/sys/i386/i386/trap.c:744
> #5  0xc085bfb5 in trap (frame=
>       {tf_fs = 8, tf_es = 40, tf_ds = -1063714776, tf_edi = 
> -1064042304, tf_esi = 0, tf_ebp = -311538944, tf_isp = -311538972, tf_ebx
> = -967615488, tf_edx = -1063651212, tf_ecx = -941099136, tf_eax = 0, 
> tf_trapno = 12, tf_err = 0, tf_eip = -1066845359, tf_cs = 32,
> tf_eflags = 66194, tf_esp = -967615488, tf_ss = 0})
>     at /usr/src/sys/i386/i386/trap.c:434
> #6  0xc0848bea in calltrap () at /usr/src/sys/i386/i386/exception.s:139
> #7  0xc0693b51 in ttymodem (tp=0xc6535c00, flag=-1063651212) at 
> /usr/src/sys/kern/tty.c:1659
> #8  0xc0698362 in ptcclose (dev=0x0, flags=3, fmt=8192, td=0xc7e7f780) 
> at linedisc.h:136
> #9  0xc0638a6f in giant_close (dev=0xcb3c1100, fflag=3, devtype=8192, 
> td=0xc7e7f780) at /usr/src/sys/kern/kern_conf.c:266
> #10 0xc06162bf in devfs_close (ap=0xed6e4b7c) at 
> /usr/src/sys/fs/devfs/devfs_vnops.c:287
> #11 0xc086dc1c in VOP_CLOSE_APV (vop=0x0, a=0xc099f874) at vnode_if.c:426
> #12 0xc06c87e2 in vn_close (vp=0xc9cdf660, flags=3, file_cred=0x0, 
> td=0xc7e7f780) at vnode_if.h:227
> #13 0xc06c974a in vn_closefile (fp=0xc6fc5438, td=0xc7e7f780) at 
> /usr/src/sys/kern/vfs_vnops.c:865
> #14 0xc06162e7 in devfs_close_f (fp=0xc6fc5438, td=0xc7e7f780) at 
> /usr/src/sys/fs/devfs/devfs_vnops.c:297
> #15 0xc0642cdc in fdrop_locked (fp=0xc6fc5438, td=0xc7e7f780) at 
> file.h:295
> #16 0xc0642c29 in fdrop (fp=0xc6fc5438, td=0xc7e7f780) at 
> /usr/src/sys/kern/kern_descrip.c:2122
> #17 0xc06411c7 in closef (fp=0xc6fc5438, td=0xc7e7f780) at 
> /usr/src/sys/kern/kern_descrip.c:1942
> #18 0xc063e329 in close (td=0xc7e7f780, uap=0x0) at 
> /usr/src/sys/kern/kern_descrip.c:1007
>
>>  Back out one of the Giant removals from revision 1.272. Giant was 
>> not here to
>>  protect the vnode, it was present to synchronize access to TTY session
>>  information between exit(2) and the TTY code. While we are here, 
>> note that
>>  Giant is required for TTY protection.
>>
>>  Clue from:      bde
>>  Discussed with: jhb
>>  MFC after:      1 week
>>
>>  Revision  Changes    Path
>>  1.290     +2 -2      src/sys/kern/kern_exit.c
>>
>
>


-- 
Christian S.J. Peron
csjp@FreeBSD.ORG
FreeBSD Committer
FreeBSD Security Team




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4508A86D.5030406>