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();