Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 15 Mar 2009 01:07:43 GMT
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 159225 for review
Message-ID:  <200903150107.n2F17hNe012582@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=159225

Change 159225 by hselasky@hselasky_laptop001 on 2009/03/15 01:07:17

	
	USB CORE: Fix regression issue in the USB file system interface.
	- Use cdev_privdata pointer as indicator of correct file handle.
	- Remove redundant FIFO opened flags.
	
	Reported by: Alexander Best

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb/usb_dev.c#8 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_dev.h#5 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_device.h#5 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb/usb_dev.c#8 (text+ko) ====

@@ -72,7 +72,8 @@
 
 /* prototypes */
 
-static int	usb2_fifo_open(struct usb2_fifo *, int);
+static int	usb2_fifo_open(struct usb2_cdev_privdata *, 
+		    struct usb2_fifo *, int);
 static void	usb2_fifo_close(struct usb2_fifo *, int);
 static void	usb2_dev_init(void *);
 static void	usb2_dev_init_post(void *);
@@ -200,6 +201,8 @@
 			cpd->is_write = 1;	/* ref */
 			if (f == NULL || f->refcount == USB_FIFO_REF_MAX)
 				goto error;
+			if (f->curr_cpd != cpd)
+				goto error;
 			/* check if USB-FS is active */
 			if (f->fs_ep_max != 0) {
 				cpd->is_usbfs = 1;
@@ -222,6 +225,8 @@
 			cpd->is_read = 1;	/* ref */
 			if (f == NULL || f->refcount == USB_FIFO_REF_MAX)
 				goto error;
+			if (f->curr_cpd != cpd)
+				goto error;
 			/* check if USB-FS is active */
 			if (f->fs_ep_max != 0) {
 				cpd->is_usbfs = 1;
@@ -434,7 +439,7 @@
 					/* wrong endpoint index */
 					continue;
 				}
-				if (f->opened) {
+				if (f->curr_cpd != NULL) {
 					/* FIFO is opened */
 					is_busy = 1;
 					continue;
@@ -451,7 +456,7 @@
 					/* wrong endpoint index */
 					continue;
 				}
-				if (f->opened) {
+				if (f->curr_cpd != NULL) {
 					/* FIFO is opened */
 					is_busy = 1;
 					continue;
@@ -470,6 +475,16 @@
 			return (is_busy ? EBUSY : EINVAL);
 		}
 	}
+
+	if ((ep != 0) && is_busy) {
+		/*
+		 * Only the default control endpoint is allowed to be
+		 * opened multiple times!
+		 */
+		DPRINTFN(5, "busy\n");
+		return (EBUSY);
+	}
+
 	/* Check TX FIFO */
 	if (is_tx &&
 	    (udev->fifo[n + USB_FIFO_TX] == NULL)) {
@@ -639,7 +654,8 @@
  * Else: Failure
  *------------------------------------------------------------------------*/
 static int
-usb2_fifo_open(struct usb2_fifo *f, int fflags)
+usb2_fifo_open(struct usb2_cdev_privdata *cpd, 
+    struct usb2_fifo *f, int fflags)
 {
 	int err;
 
@@ -660,7 +676,7 @@
 
 	/* check if we are already opened */
 	/* we don't need any locks when checking this variable */
-	if (f->opened) {
+	if (f->curr_cpd != NULL) {
 		err = EBUSY;
 		goto done;
 	}
@@ -690,9 +706,9 @@
 	/* reset ASYNC proc flag */
 	f->async_p = NULL;
 
+	mtx_lock(&usb2_ref_lock);
 	/* flag the fifo as opened to prevent others */
-	mtx_lock(&usb2_ref_lock);
-	f->opened = 1;
+	f->curr_cpd = cpd;
 	mtx_unlock(&usb2_ref_lock);
 
 	/* reset queue */
@@ -733,14 +749,14 @@
 	int err;
 
 	/* check if we are not opened */
-	if (!f->opened) {
+	if (f->curr_cpd == NULL) {
 		/* nothing to do - already closed */
 		return;
 	}
 	mtx_lock(f->priv_mtx);
 
-	/* clear current file flag */
-	f->opened = 0;
+	/* clear current cdev private data pointer */
+	f->curr_cpd = NULL;
 
 	/* check if we are selected */
 	if (f->flag_isselect) {
@@ -834,21 +850,6 @@
 	}
 	cpd->fflags = fflags;	/* access mode for open lifetime */
 
-	/* Check if the endpoint is already open, we always allow EP0 */
-	if (ep > 0) {
-		if ((fflags & FREAD && cpd->udev->ep_rd_opened & (1 << ep)) ||
-		    (fflags & FWRITE && cpd->udev->ep_wr_opened & (1 << ep))) {
-			DPRINTFN(2, "endpoint already open\n");
-			usb2_unref_device(cpd);
-			free(cpd, M_USBDEV);
-			return (EBUSY);
-		}
-		if (fflags & FREAD)
-			cpd->udev->ep_rd_opened |= (1 << ep);
-		if (fflags & FWRITE)
-			cpd->udev->ep_wr_opened |= (1 << ep);
-	}
-
 	/* create FIFOs, if any */
 	err = usb2_fifo_create(cpd);
 	/* check for error */
@@ -859,7 +860,7 @@
 		return (err);
 	}
 	if (fflags & FREAD) {
-		err = usb2_fifo_open(cpd->rxfifo, fflags);
+		err = usb2_fifo_open(cpd, cpd->rxfifo, fflags);
 		if (err) {
 			DPRINTFN(2, "read open failed\n");
 			usb2_unref_device(cpd);
@@ -868,7 +869,7 @@
 		}
 	}
 	if (fflags & FWRITE) {
-		err = usb2_fifo_open(cpd->txfifo, fflags);
+		err = usb2_fifo_open(cpd, cpd->txfifo, fflags);
 		if (err) {
 			DPRINTFN(2, "write open failed\n");
 			if (fflags & FREAD) {
@@ -906,13 +907,9 @@
 	udev = cpd->udev;
 	if (cpd->fflags & FREAD) {
 		usb2_fifo_close(cpd->rxfifo, cpd->fflags);
-		/* clear read bitmask */
-		udev->ep_rd_opened &= ~(1 << cpd->ep_addr);
 	}
 	if (cpd->fflags & FWRITE) {
 		usb2_fifo_close(cpd->txfifo, cpd->fflags);
-		/* clear write bitmask */
-		udev->ep_wr_opened &= ~(1 << cpd->ep_addr);
 	}
 
 	usb2_unref_device(cpd);
@@ -1762,7 +1759,9 @@
 	f_sc->fp[USB_FIFO_RX] = NULL;
 
 	if (f_sc->dev != NULL) {
-		destroy_dev_sched_cb(f_sc->dev, usb2_fifo_cleanup, f_sc->dev->si_drv1);
+		destroy_dev_sched_cb(f_sc->dev, 
+		    usb2_fifo_cleanup, f_sc->dev->si_drv1);
+		f_sc->dev = NULL;
 	}
 
 	DPRINTFN(2, "detached %p\n", f_sc);

==== //depot/projects/usb/src/sys/dev/usb/usb_dev.h#5 (text+ko) ====

@@ -93,12 +93,12 @@
 	int			bus_index;	/* bus index */
 	int			dev_index;	/* device index */
 	int			ep_addr;	/* endpoint address */
+	int			fflags;
 	uint8_t			fifo_index;	/* FIFO index */
 	uint8_t			is_read;	/* location has read access */
 	uint8_t			is_write;	/* location has write access */
 	uint8_t			is_uref;	/* USB refcount decr. needed */
 	uint8_t			is_usbfs;	/* USB-FS is active */
-	int			fflags;
 };
 
 struct usb2_fs_privdata {
@@ -130,7 +130,8 @@
 	struct usb2_xfer *xfer[2];
 	struct usb2_xfer **fs_xfer;
 	struct mtx *priv_mtx;		/* client data */
-	int    opened;			/* set if FIFO is opened by a FILE */
+	/* set if FIFO is opened by a FILE: */
+	struct usb2_cdev_privdata *curr_cpd;
 	void   *priv_sc0;		/* client data */
 	void   *priv_sc1;		/* client data */
 	void   *queue_data;

==== //depot/projects/usb/src/sys/dev/usb/usb_device.h#5 (text+ko) ====

@@ -127,8 +127,6 @@
 
 	uint32_t plugtime;		/* copy of "ticks" */
 
-	uint16_t ep_rd_opened;		/* bitmask of endpoints opened */
-	uint16_t ep_wr_opened;		/*  from the device nodes. */
 	uint16_t refcount;
 #define	USB_DEV_REF_MAX 0xffff
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200903150107.n2F17hNe012582>