On Tue, 13 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.
>
> Signed-off-by: Mateusz Berezecki <[email protected]>
> ---
...
> /* 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;
>
> - atomic_add(amount, &usbfs_memory_usage);
> - if (atomic_read(&usbfs_memory_usage) <= lim)
> - return 0;
> - atomic_sub(amount, &usbfs_memory_usage);
> - return -ENOMEM;
> + atomic64_add(amount, &usbfs_memory_usage);
> +
> + if (lim > 0) {
> + if (atomic64_read(&usbfs_memory_usage) <= lim)
> + return 0;
> + atomic64_sub(amount, &usbfs_memory_usage);
> + return -ENOMEM;
> + }
> +
> + return 0;
I would have written:
if (lim > 0 && atomic64_read(&usbfs_memory_usage) > lim) {
atomic64_sub(amount, &usbfs_memory_usage);
return -ENOMEM;
}
return 0;
But that's merely a matter of personal style and taste.
> @@ -1458,6 +1453,7 @@ static int proc_do_submiturb(struct usb_dev_state *ps,
> struct usbdevfs_urb *uurb
> int number_of_packets = 0;
> unsigned int stream_id = 0;
> void *buf;
> + u32 overhead;
>
> if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
> USBDEVFS_URB_SHORT_NOT_OK |
> @@ -1584,7 +1580,11 @@ static int proc_do_submiturb(struct usb_dev_state *ps,
> struct usbdevfs_urb *uurb
> return -EINVAL;
> }
>
> - if (uurb->buffer_length >= USBFS_XFER_MAX) {
> + /* check for overflow */
> + overhead = u + sizeof(struct async) + sizeof(struct urb)
> + + num_sgs * sizeof(struct scatterlist);
> +
> + if (uurb->buffer_length + overhead < uurb->buffer_length) {
> ret = -EINVAL;
> goto error;
> }
I just realized that this part isn't necessary after all. u is
unsigned, but uurb->buffer_length is a signed integer. Since num_sgs
is limited to 128, the computation cannot overflow in any case.
Sorry for the confusion.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
