> Roel Kluin wrote:
>
>> After this the test below does no longer make sense.
>> - /* the I/O buffer must be mapped/unmapped, except when length=0 */
>> - if (urb->transfer_buffer_length < 0)
>> - return -EMSGSIZE;
> I agree, this test is now unnecessary.
>
> There are a couple of other small things which should be changed as
> well:
>
> In hcd.c, rh_call_control() and rh_queue_status() contain
> a "len" variable which should now be unsigned rather than
> int. The same is true for the "len" parameter of
> rh_string().
>
> In devio.c there's a dev_info() statement in which
> transfer_buffer_length is printed with a %d specification.
> It should now be %u.
>
> All these changes could be wrapped up into a single patch.
>
> Alan Stern
I think this is what you wanted? It compiles with defconfig, checkpatch doesn't
complain, nor sparse anymore, but please review anyways.
urb->dev->maxchild is an int, shuld it be converted to unsigned as well?
------------------------------>8-------------8<---------------------------------
transfer_buffer_length and actual_length have become unsigned, therefore some
additional conversion of local variables, function arguments and print
specifications is desired. A test for a negative urb->transfer_buffer_length has
become obsolete as well.
rh_string() does no longer return -EPIPE in the case of an unsupported ID.
Instead its only caller, rh_call_control() does the check.
usb_internal_control_msg() returns both error values and a length. To ensure
that it does not return an unsigned length that is converted into a negative,
the returned length is limited to INT_MAX.
Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx>
---
drivers/usb/core/devio.c | 6 +++---
drivers/usb/core/hcd.c | 31 ++++++++++++-------------------
drivers/usb/core/message.c | 12 ++++++------
drivers/usb/core/urb.c | 4 ----
include/linux/usb.h | 4 ++--
5 files changed, 23 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 7513bb0..bcd013d 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -311,9 +311,9 @@ static void snoop_urb(struct urb *urb, void __user *userurb)
dev_info(&urb->dev->dev, "direction=%s\n",
usb_urb_dir_in(urb) ? "IN" : "OUT");
dev_info(&urb->dev->dev, "userurb=%p\n", userurb);
- dev_info(&urb->dev->dev, "transfer_buffer_length=%d\n",
+ dev_info(&urb->dev->dev, "transfer_buffer_length=%u\n",
urb->transfer_buffer_length);
- dev_info(&urb->dev->dev, "actual_length=%d\n", urb->actual_length);
+ dev_info(&urb->dev->dev, "actual_length=%u\n", urb->actual_length);
dev_info(&urb->dev->dev, "data: ");
for (j = 0; j < urb->transfer_buffer_length; ++j)
printk("%02x ", data[j]);
@@ -754,7 +754,7 @@ static int proc_bulk(struct dev_state *ps, void __user *arg)
struct usb_device *dev = ps->dev;
struct usbdevfs_bulktransfer bulk;
unsigned int tmo, len1, pipe;
- int len2;
+ u32 len2;
unsigned char *tbuf;
int i, j, ret;
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 3c711db..8cec7cc 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -279,9 +279,9 @@ static const u8 hs_rh_config_descriptor [] = {
* helper routine for returning string descriptors in UTF-16LE
* input can actually be ISO-8859-1; ASCII is its 7-bit subset
*/
-static int ascii2utf (char *s, u8 *utf, int utfmax)
+static u32 ascii2utf(char *s, u8 *utf, int utfmax)
{
- int retval;
+ u32 retval;
for (retval = 0; *s && utfmax > 1; utfmax -= 2, retval += 2) {
*utf++ = *s++;
@@ -304,19 +304,15 @@ static int ascii2utf (char *s, u8 *utf, int utfmax)
* Produces either a manufacturer, product or serial number string for the
* virtual root hub device.
*/
-static int rh_string (
- int id,
- struct usb_hcd *hcd,
- u8 *data,
- int len
-) {
+static u32 rh_string(int id, struct usb_hcd *hcd, u8 *data, u32 len)
+{
char buf [100];
// language ids
if (id == 0) {
buf[0] = 4; buf[1] = 3; /* 4 bytes string data */
buf[2] = 0x09; buf[3] = 0x04; /* MSFT-speak for "en-us" */
- len = min (len, 4);
+ len = min_t(u32, len, 4);
memcpy (data, buf, len);
return len;
@@ -332,10 +328,7 @@ static int rh_string (
} else if (id == 3) {
snprintf (buf, sizeof buf, "%s %s %s", init_utsname()->sysname,
init_utsname()->release, hcd->driver->description);
-
- // unsupported IDs --> "protocol stall"
- } else
- return -EPIPE;
+ }
switch (len) { /* All cases fall through */
default:
@@ -360,9 +353,8 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
u8 tbuf [sizeof (struct usb_hub_descriptor)]
__attribute__((aligned(4)));
const u8 *bufp = tbuf;
- int len = 0;
+ u32 len = 0;
int status;
- int n;
u8 patch_wakeup = 0;
u8 patch_protocol = 0;
@@ -456,10 +448,11 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
patch_wakeup = 1;
break;
case USB_DT_STRING << 8:
- n = rh_string (wValue & 0xff, hcd, ubuf, wLength);
- if (n < 0)
+ if ((wValue & 0xff) < 4)
+ urb->actual_length = rh_string(wValue & 0xff,
+ hcd, ubuf, wLength);
+ else /* unsupported IDs --> "protocol stall" */
goto error;
- urb->actual_length = n;
break;
default:
goto error;
@@ -629,7 +622,7 @@ static int rh_queue_status (struct usb_hcd *hcd, struct urb *urb)
{
int retval;
unsigned long flags;
- int len = 1 + (urb->dev->maxchild / 8);
+ u32 len = 1 + (urb->dev->maxchild / 8);
spin_lock_irqsave (&hcd_root_hub_lock, flags);
if (hcd->status_urb || urb->transfer_buffer_length < len) {
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 49e7f56..50c03ca 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -40,7 +40,7 @@ static void usb_api_blocking_completion(struct urb *urb)
* interruptible and therefore these drivers should implement their
* own interruptible routines.
*/
-static int usb_start_wait_urb(struct urb *urb, int timeout, int *actual_length)
+static int usb_start_wait_urb(struct urb *urb, int timeout, u32 *actual_length)
{
struct api_context ctx;
unsigned long expire;
@@ -59,7 +59,7 @@ static int usb_start_wait_urb(struct urb *urb, int timeout, int *actual_length)
retval = (ctx.status == -ENOENT ? -ETIMEDOUT : ctx.status);
dev_dbg(&urb->dev->dev,
- "%s timed out on ep%d%s len=%d/%d\n",
+ "%s timed out on ep%d%s len=%u/%u\n",
current->comm,
usb_endpoint_num(&urb->ep->desc),
usb_urb_dir_in(urb) ? "in" : "out",
@@ -84,7 +84,7 @@ static int usb_internal_control_msg(struct usb_device *usb_dev,
{
struct urb *urb;
int retv;
- int length;
+ u32 length;
urb = usb_alloc_urb(0, GFP_NOIO);
if (!urb)
@@ -97,7 +97,7 @@ static int usb_internal_control_msg(struct usb_device *usb_dev,
if (retv < 0)
return retv;
else
- return length;
+ return min_t(u32, length, INT_MAX);
}
/**
@@ -182,7 +182,7 @@ EXPORT_SYMBOL_GPL(usb_control_msg);
* the request.
*/
int usb_interrupt_msg(struct usb_device *usb_dev, unsigned int pipe,
- void *data, int len, int *actual_length, int timeout)
+ void *data, int len, u32 *actual_length, int timeout)
{
return usb_bulk_msg(usb_dev, pipe, data, len, actual_length, timeout);
}
@@ -220,7 +220,7 @@ EXPORT_SYMBOL_GPL(usb_interrupt_msg);
* (with the default interval) if the target is an interrupt endpoint.
*/
int usb_bulk_msg(struct usb_device *usb_dev, unsigned int pipe,
- void *data, int len, int *actual_length, int timeout)
+ void *data, int len, u32 *actual_length, int timeout)
{
struct urb *urb;
struct usb_host_endpoint *ep;
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 58bc5e3..e03cad1 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -369,10 +369,6 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
}
}
- /* the I/O buffer must be mapped/unmapped, except when length=0 */
- if (urb->transfer_buffer_length < 0)
- return -EMSGSIZE;
-
#ifdef DEBUG
/* stuff that drivers shouldn't do, but which shouldn't
* cause problems in HCDs if they get it wrong.
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 88079fd..ea258e3 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1550,9 +1550,9 @@ extern int usb_control_msg(struct usb_device *dev, unsigned int pipe,
__u8 request, __u8 requesttype, __u16 value, __u16 index,
void *data, __u16 size, int timeout);
extern int usb_interrupt_msg(struct usb_device *usb_dev, unsigned int pipe,
- void *data, int len, int *actual_length, int timeout);
+ void *data, int len, u32 *actual_length, int timeout);
extern int usb_bulk_msg(struct usb_device *usb_dev, unsigned int pipe,
- void *data, int len, int *actual_length,
+ void *data, int len, u32 *actual_length,
int timeout);
/* wrappers around usb_control_msg() for the most common standard requests */
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
