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

Work around a race condition in devfs by changing the way closes

Browse Source 
are handled in most CAM peripheral drivers that are not handled by
GEOM's disk class.

The usual character driver open and close semantics are that the
driver gets N open calls, but only one close, when the last caller
closes the device.

CAM peripheral drivers expect that behavior to be honored to the
letter, and the CAM peripheral driver code (specifically
cam_periph_release_locked_busses()) panics if it is done incorrectly.

Since devfs has to drop its locks while it calls a driver's close
routine, and it does not have a way to delay or prevent open calls
while it is calling the close routine, there is a race.

The sequence of events, simplified a bit, is:

- devfs acquires a lock
- devfs checks the reference count, and if it is 1, continues to close.
- devfs releases the lock

- 2nd process open call on the device happens here

- devfs calls the driver's close routine

- devfs acquires a lock
- devfs decrements the reference count
- devfs releases the lock

- 2nd process close call on the device happens here

At the second close, we get a panic in
cam_periph_release_locked_busses(), complaining that peripheral
has been released when the reference count is already 0.  This is
because we have gotten two closes in a row, which should not
happen.

The fix is to add the D_TRACKCLOSE flag to the driver's cdevsw, so
that we get a close() call for each open().  That does happen
reliably, so we can make sure that our reference counts are
correct.

Note that the sa(4) and pt(4) drivers only allow one context
through the open routine.  So these drivers aren't exposed to the
same race condition.

scsi_ch.c,
scsi_enc.c,
scsi_enc_internal.h,
scsi_pass.c,
scsi_sg.c:
		For these drivers, change the open() routine to
		increment the reference count for every open, and
		just decrement the reference count in the close.

		Call cam_periph_release_locked() in some scenarios
		to avoid additional lock and unlock calls.

scsi_pt.c:	Call cam_periph_release_locked() in some scenarios
		to avoid additional lock and unlock calls.

MFC after:	3 days
This commit is contained in:
Kenneth D. Merry 2012-05-27 06:11:09 +00:00
parent c5ec8d1ddd
commit c552ebe12d
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=236138
5 changed files with 22 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
sys/cam/scsi/scsi_ch.c
View File

@ -107,8 +107,7 @@ static const u_int32_t CH_TIMEOUT_SEND_VOLTAG = 10000;
static const u_int32_t CH_TIMEOUT_INITIALIZE_ELEMENT_STATUS = 500000;
typedef enum {
CH_FLAG_INVALID = 0x001,
CH_FLAG_OPEN = 0x002
CH_FLAG_INVALID = 0x001
} ch_flags;
typedef enum {
@ -211,7 +210,7 @@ PERIPHDRIVER_DECLARE(ch, chdriver);
static struct cdevsw ch_cdevsw = {
.d_version = D_VERSION,
.d_flags = 0,
.d_flags = D_TRACKCLOSE,
.d_open = chopen,
.d_close = chclose,
.d_ioctl = chioctl,
@ -404,16 +403,11 @@ chopen(struct cdev *dev, int flags, int fmt, struct thread *td)
cam_periph_lock(periph);
if (softc->flags & CH_FLAG_INVALID) {
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return(ENXIO);
}
if ((softc->flags & CH_FLAG_OPEN) == 0)
softc->flags |= CH_FLAG_OPEN;
else
cam_periph_release(periph);
if ((error = cam_periph_hold(periph, PRIBIO | PCATCH)) != 0) {
cam_periph_unlock(periph);
cam_periph_release(periph);
@ -424,9 +418,8 @@ chopen(struct cdev *dev, int flags, int fmt, struct thread *td)
* Load information about this changer device into the softc.
*/
if ((error = chgetparams(periph)) != 0) {
softc->flags &= ~CH_FLAG_OPEN;
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return(error);
}
@ -451,11 +444,6 @@ chclose(struct cdev *dev, int flag, int fmt, struct thread *td)
softc = (struct ch_softc *)periph->softc;
cam_periph_lock(periph);
softc->flags &= ~CH_FLAG_OPEN;
cam_periph_unlock(periph);
cam_periph_release(periph);
return(0);

16
sys/cam/scsi/scsi_enc.c
View File

@ -93,7 +93,7 @@ static struct cdevsw enc_cdevsw = {
.d_close = enc_close,
.d_ioctl = enc_ioctl,
.d_name = "ses",
.d_flags = 0,
.d_flags = D_TRACKCLOSE,
};
static void
@ -254,12 +254,12 @@ enc_open(struct cdev *dev, int flags, int fmt, struct thread *td)
error = ENXIO;
goto out;
}
out:
if (error != 0)
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
if (error) {
cam_periph_release(periph);
}
return (error);
}
@ -267,17 +267,11 @@ static int
enc_close(struct cdev *dev, int flag, int fmt, struct thread *td)
{
struct cam_periph *periph;
struct enc_softc *softc;
periph = (struct cam_periph *)dev->si_drv1;
if (periph == NULL)
return (ENXIO);
cam_periph_lock(periph);
softc = (struct enc_softc *)periph->softc;
cam_periph_unlock(periph);
cam_periph_release(periph);
return (0);

26
sys/cam/scsi/scsi_pass.c
View File

@ -111,7 +111,7 @@ PERIPHDRIVER_DECLARE(pass, passdriver);
static struct cdevsw pass_cdevsw = {
.d_version = D_VERSION,
.d_flags = 0,
.d_flags = D_TRACKCLOSE,
.d_open = passopen,
.d_close = passclose,
.d_ioctl = passioctl,
@ -377,8 +377,8 @@ passopen(struct cdev *dev, int flags, int fmt, struct thread *td)
softc = (struct pass_softc *)periph->softc;
if (softc->flags & PASS_FLAG_INVALID) {
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return(ENXIO);
}
@ -387,8 +387,8 @@ passopen(struct cdev *dev, int flags, int fmt, struct thread *td)
*/
error = securelevel_gt(td->td_ucred, 1);
if (error) {
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return(error);
}
@ -396,8 +396,8 @@ passopen(struct cdev *dev, int flags, int fmt, struct thread *td)
* Only allow read-write access.
*/
if (((flags & FWRITE) == 0) || ((flags & FREAD) == 0)) {
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return(EPERM);
}
@ -406,19 +406,12 @@ passopen(struct cdev *dev, int flags, int fmt, struct thread *td)
*/
if ((flags & O_NONBLOCK) != 0) {
xpt_print(periph->path, "can't do nonblocking access\n");
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return(EINVAL);
}
if ((softc->flags & PASS_FLAG_OPEN) == 0) {
softc->flags |= PASS_FLAG_OPEN;
cam_periph_unlock(periph);
} else {
/* Device closes aren't symmertical, so fix up the refcount */
cam_periph_unlock(periph);
cam_periph_release(periph);
}
cam_periph_unlock(periph);
return (error);
}
@ -427,18 +420,11 @@ static int
passclose(struct cdev *dev, int flag, int fmt, struct thread *td)
{
struct cam_periph *periph;
struct pass_softc *softc;
periph = (struct cam_periph *)dev->si_drv1;
if (periph == NULL)
return (ENXIO);
cam_periph_lock(periph);
softc = (struct pass_softc *)periph->softc;
softc->flags &= ~PASS_FLAG_OPEN;
cam_periph_unlock(periph);
cam_periph_release(periph);
return (0);

4
sys/cam/scsi/scsi_pt.c
View File

@ -148,8 +148,8 @@ ptopen(struct cdev *dev, int flags, int fmt, struct thread *td)
cam_periph_lock(periph);
if (softc->flags & PT_FLAG_DEVICE_INVALID) {
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return(ENXIO);
}
@ -182,8 +182,8 @@ ptclose(struct cdev *dev, int flag, int fmt, struct thread *td)
cam_periph_lock(periph);
softc->flags &= ~PT_FLAG_OPEN;
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return (0);
}

25
sys/cam/scsi/scsi_sg.c
View File

@ -61,9 +61,8 @@ __FBSDID("$FreeBSD$");
#include <compat/linux/linux_ioctl.h>
typedef enum {
SG_FLAG_OPEN = 0x01,
SG_FLAG_LOCKED = 0x02,
SG_FLAG_INVALID = 0x04
SG_FLAG_LOCKED = 0x01,
SG_FLAG_INVALID = 0x02
} sg_flags;
typedef enum {
@ -141,7 +140,7 @@ PERIPHDRIVER_DECLARE(sg, sgdriver);
static struct cdevsw sg_cdevsw = {
.d_version = D_VERSION,
.d_flags = D_NEEDGIANT,
.d_flags = D_NEEDGIANT | D_TRACKCLOSE,
.d_open = sgopen,
.d_close = sgclose,
.d_ioctl = sgioctl,
@ -415,19 +414,12 @@ sgopen(struct cdev *dev, int flags, int fmt, struct thread *td)
softc = (struct sg_softc *)periph->softc;
if (softc->flags & SG_FLAG_INVALID) {
cam_periph_release_locked(periph);
cam_periph_unlock(periph);
cam_periph_release(periph);
return (ENXIO);
}
if ((softc->flags & SG_FLAG_OPEN) == 0) {
softc->flags |= SG_FLAG_OPEN;
cam_periph_unlock(periph);
} else {
/* Device closes aren't symmetrical, fix up the refcount. */
cam_periph_unlock(periph);
cam_periph_release(periph);
}
cam_periph_unlock(periph);
return (error);
}
@ -436,18 +428,11 @@ static int
sgclose(struct cdev *dev, int flag, int fmt, struct thread *td)
{
struct cam_periph *periph;
struct sg_softc *softc;
periph = (struct cam_periph *)dev->si_drv1;
if (periph == NULL)
return (ENXIO);
cam_periph_lock(periph);
softc = (struct sg_softc *)periph->softc;
softc->flags &= ~SG_FLAG_OPEN;
cam_periph_unlock(periph);
cam_periph_release(periph);
return (0);