On Mon, 12 Dec 2016, Mateusz Berezecki wrote: > Promote a variable keeping track of USB transfer memory usage to a > wider data type and allow for higher bandwidth transfers from a large > number of USB devices connected to a single host. > --- > drivers/usb/core/devio.c | 34 ++++++++++++---------------------- > 1 file changed, 12 insertions(+), 22 deletions(-) > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > index 4016dae..bead975 100644 > --- a/drivers/usb/core/devio.c > +++ b/drivers/usb/core/devio.c > @@ -134,42 +134,32 @@ enum snoop_when { > #define USB_DEVICE_DEV MKDEV(USB_DEVICE_MAJOR, 0) > > /* Limit on the total amount of memory we can allocate for transfers */ > -static unsigned usbfs_memory_mb = 16; > +static u32 usbfs_memory_mb = 16; > module_param(usbfs_memory_mb, uint, 0644); > MODULE_PARM_DESC(usbfs_memory_mb, > "maximum MB allowed for usbfs buffers (0 = no limit)"); > > -/* Hard limit, necessary to avoid arithmetic overflow */ > -#define USBFS_XFER_MAX (UINT_MAX / 2 - 1000000) > - > -static atomic_t usbfs_memory_usage; /* Total memory currently allocated */ > +static atomic64_t usbfs_memory_usage; /* Total memory currently > allocated */ > > /* Check whether it's okay to allocate more memory for a transfer */ > -static int usbfs_increase_memory_usage(unsigned amount) > +static int usbfs_increase_memory_usage(u64 amount) > { > - unsigned lim; > + u64 lim; > > - /* > - * Convert usbfs_memory_mb to bytes, avoiding overflows. > - * 0 means use the hard limit (effectively unlimited). > - */ > lim = ACCESS_ONCE(usbfs_memory_mb); > - if (lim == 0 || lim > (USBFS_XFER_MAX >> 20)) > - lim = USBFS_XFER_MAX; > - else > - lim <<= 20; > + lim <<= 20;
You need to handle the case where lim == 0 (see the MODULE_PARM_DESC text). > > - atomic_add(amount, &usbfs_memory_usage); > - if (atomic_read(&usbfs_memory_usage) <= lim) > + atomic64_add(amount, &usbfs_memory_usage); > + if (atomic64_read(&usbfs_memory_usage) <= lim) > return 0; > - atomic_sub(amount, &usbfs_memory_usage); > + atomic64_sub(amount, &usbfs_memory_usage); > return -ENOMEM; > } > > /* Memory for a transfer is being deallocated */ > -static void usbfs_decrease_memory_usage(unsigned amount) > +static void usbfs_decrease_memory_usage(u64 amount) > { > - atomic_sub(amount, &usbfs_memory_usage); > + atomic64_sub(amount, &usbfs_memory_usage); > } > > static int connected(struct usb_dev_state *ps) > @@ -1191,7 +1181,7 @@ static int proc_bulk(struct usb_dev_state *ps, void > __user *arg) > if (!usb_maxpacket(dev, pipe, !(bulk.ep & USB_DIR_IN))) > return -EINVAL; > len1 = bulk.len; > - if (len1 >= USBFS_XFER_MAX) > + if (len1 >= (INT_MAX - sizeof(struct urb))) > return -EINVAL; > ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb)); > if (ret) > @@ -1584,7 +1574,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps, > struct usbdevfs_urb *uurb > return -EINVAL; > } > > - if (uurb->buffer_length >= USBFS_XFER_MAX) { > + if (uurb->buffer_length >= (INT_MAX - sizeof(struct urb))) { > ret = -EINVAL; > goto error; > } This isn't right. See how the u variable gets used in this routine. You have to guarantee that the computation of u won't overflow. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html