d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

sAdd a comment indicating why there continues to be a race condition in

Browse Source 
the tap driver, even with Giant over the cdev operation vector, due to
a non-atomic test-and-set of the si_drv1 field in the dev_t.  This bug
exists with Giant under high memory pressure, as malloc() may sleep
in tapcreate(), but is less likely to occur.  The resolution will
probably be to cover si_drv1 using the global tapmtx since no softc is
available, but I need to think about this problem more generally
across a range of drivers using si_drv1 in combination with SI_CHEAPCLONE
to defer expensive allocation to open().

Correct what appears to be a bug in the original if_tap implementation,
in which tapopen() will panic if a tap device instance is opened more
than once due to an incorrect assertion -- only triggered if INVARIANTS
is compiled in (i.e., when built into a kernel).  Return EBUSY instead.

Expand mtx_lock() coverage using tp->tap_mtx to include tp->ether_addr.
This commit is contained in:
Robert Watson 2004-03-18 09:55:11 +00:00
parent c11857b462
commit b4f5ef7eac
1 changed files with 14 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
sys/net/if_tap.c
View File

@ -346,19 +346,24 @@ tapopen(dev, flag, mode, td)
if ((dev2unit(dev) & CLONE_UNITMASK) > TAPMAXUNIT)
return (ENXIO);
/*
* XXXRW: Non-atomic test-and-set of si_drv1. Currently protected
* by Giant, but the race actually exists under memory pressure as
* well even when running with Giant, as malloc() may sleep.
*/
tp = dev->si_drv1;
if (tp == NULL) {
tapcreate(dev);
tp = dev->si_drv1;
}
/* Unlocked read. */
KASSERT(!(tp->tap_flags & TAP_OPEN),
("%s flags is out of sync", tp->tap_if.if_xname));
mtx_lock(&tp->tap_mtx);
if (tp->tap_flags & TAP_OPEN) {
mtx_unlock(&tp->tap_mtx);
return (EBUSY);
}
bcopy(tp->arpcom.ac_enaddr, tp->ether_addr, sizeof(tp->ether_addr));
mtx_lock(&tp->tap_mtx);
tp->tap_pid = td->td_proc->p_pid;
tp->tap_flags |= TAP_OPEN;
mtx_unlock(&tp->tap_mtx);
@ -679,11 +684,15 @@ tapioctl(dev, cmd, data, flag, td)
case OSIOCGIFADDR: /* get MAC address of the remote side */
case SIOCGIFADDR:
mtx_lock(&tp->tap_mtx);
bcopy(tp->ether_addr, data, sizeof(tp->ether_addr));
mtx_unlock(&tp->tap_mtx);
break;
case SIOCSIFADDR: /* set MAC address of the remote side */
mtx_lock(&tp->tap_mtx);
bcopy(data, tp->ether_addr, sizeof(tp->ether_addr));
mtx_unlock(&tp->tap_mtx);
break;
default: