kern/122977: System hang on removal of USB serial device (tty)

[Arthur Hartwig]

>Number:         122977
>Category:       kern
>Synopsis:       System hang on removal of USB serial device (tty)
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Apr 22 05:40:03 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator:     Arthur Hartwig
>Release:        6.3
>Organization:
Nokia
>Environment:
>Description:
Custom USB serial port driver in system which interfaces to ucom (USB tty) driver.

User logs in as root on ttyU0. syslogd is configured to send system messages to root. USB device is removed. System hang results.

On console terminal <break>ddb is used to enter ddb. Analysis shows kernel is looping in devfs_allocv() with vget() repeated called and returning 2 (ENOENT). Examination of the struct vnode pointer passed to vget() shows the VI_DOOMED bit is set. ddb shows syslogd as current process and syslogd attempting to open /dev/ttyU0, presumably to output the message which says that ucom0 (the 'port' for /dev/ttyU0) has been removed.


>How-To-Repeat:
See above description. 

All my tests have been done on a UP system. I haven't tested this theory but its possible that different behaviour might be observed on a MP system: on a MP system other threads may get to run and cause some other change of state such that after a sufficient time, in devfs_allocv() not only does vget() return an error code but  devfs_allocv_drop_refs() also returns an error code resulting in a break out of the loop in devfs_allocv().
>Fix:
In sys/fs/devfs/devfs_vnops.c, function devfs_allocv() change

 loop:
	DEVFS_DE_HOLD(de);
	DEVFS_DMP_HOLD(dmp);
	mtx_lock(&devfs_de_interlock);
	vp = de->de_vnode;
	if (vp != NULL) {
		VI_LOCK(vp);
		mtx_unlock(&devfs_de_interlock);
		sx_xunlock(&dmp->dm_lock);
		error = vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, td);
		sx_xlock(&dmp->dm_lock);
		if (devfs_allocv_drop_refs(0, dmp, de)) {
			if (error == 0)
				vput(vp);
			return (ENOENT);
		}
		else if (error)
			goto loop;
		sx_xunlock(&dmp->dm_lock);
		*vpp = vp;
		return (0);
	}

to

 loop:
	DEVFS_DE_HOLD(de);
	DEVFS_DMP_HOLD(dmp);
	mtx_lock(&devfs_de_interlock);
	vp = de->de_vnode;
	if (vp != NULL) {
		VI_LOCK(vp);
		mtx_unlock(&devfs_de_interlock);
		sx_xunlock(&dmp->dm_lock);
		error = vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, td);
		sx_xlock(&dmp->dm_lock);
		if (devfs_allocv_drop_refs(0, dmp, de)) {
			if (error == 0)
				vput(vp);
			return (ENOENT);
		}
                else if (error == ENOENT) {
                        /* Don't loop if vget() returned ENOENT */
                        sx_xunlock(&dmp->dm_lock);
                        return error;
                }
		else if (error)
			goto loop;
		sx_xunlock(&dmp->dm_lock);
		*vpp = vp;
		return (0);
	}


>Release-Note:
>Audit-Trail:
>Unformatted: