Subject: kern/31126: bad suser checks in serial drivers
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: None <dholland@eecs.harvard.edu>
List: netbsd-bugs
Date: 09/03/2005 07:17:00
>Number:         31126
>Category:       kern
>Synopsis:       serial device drivers check cr_uid instead of calling suser
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Sep 03 07:17:00 +0000 2005
>Originator:     David A. Holland <dholland@eecs.harvard.edu>
>Release:        NetBSD 3.99.7 (-20050718)
>Organization:
   Harvard EECS
>Environment:
System: NetBSD tanaqui 3.99.7 NetBSD 3.99.7 (TANAQUI) #1: Mon Jul 18 21:43:19 EDT 2005 dholland@tanaqui:/usr/src/sys/arch/i386/compile/TANAQUI i386
Architecture: i386
Machine: i386
>Description:
	Tonight I was compiling the cyclades driver for a research OS
	I happened to notice a glitch: where it checks TS_XCLUDE it
	does an explicit check of p->p_ucred->cr_uid instead of making
	a call to suser().

	Some grep work shows that, unsurprisingly, the problem seems
	to be shared among nearly all serial/tty drivers, of which
	there are a lot.

	This is not a crisis, but it is incorrect, as the ASU flag in
	p_acflag won't get set... and any future auditing or logging
	logic that someone might add to suser() won't be reached.

	The following patches apply to sys/dev/ic/cy.c, where I first
	found it, and sys/dev/ic/com.c, which is most likely to be the
	cut and paste source for any new drivers that appear. I don't
	have time to hunt and kill all the other instances tonight,
	but maybe soon. :-/

	(I haven't actually tested these patches, but they hardly
	demand it. I hope I haven't stuffed up in some stupid way.)

>How-To-Repeat:
	n/a

>Fix:

Index: cy.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/cy.c,v
retrieving revision 1.37
diff -u -r1.37 cy.c
--- cy.c	27 Feb 2005 00:27:01 -0000	1.37
+++ cy.c	3 Sep 2005 06:35:51 -0000
@@ -369,7 +369,8 @@
 			SET(tp->t_state, TS_CARR_ON);
 		else
 			CLR(tp->t_state, TS_CARR_ON);
-	} else if (ISSET(tp->t_state, TS_XCLUDE) && p->p_ucred->cr_uid != 0) {
+	} else if (ISSET(tp->t_state, TS_XCLUDE) &&
+	    suser(p->p_ucred, &p->p_acflag) != 0) {
 		return EBUSY;
 	} else {
 		s = spltty();
Index: com.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/com.c,v
retrieving revision 1.234
diff -u -r1.234 com.c
--- com.c	21 Jun 2005 14:01:11 -0000	1.234
+++ com.c	3 Sep 2005 07:03:31 -0000
@@ -838,7 +838,7 @@
 
 	if (ISSET(tp->t_state, TS_ISOPEN) &&
 	    ISSET(tp->t_state, TS_XCLUDE) &&
-		p->p_ucred->cr_uid != 0)
+	    suser(p->p_ucred, &p->p_acflag) != 0)
 		return (EBUSY);
 
 	s = spltty();