Skip to content

Commit

Permalink
USB: usb gadgets avoid le{16,32}_to_cpup()
Browse files Browse the repository at this point in the history
It turns out that le16_to_cpup() and le32_to_cpup() aren't always safe
to call with pointers into packed structures, since those are inlined
functions and GCC may lose the "packed" attribute.  So those references
can become unaligned kernel accesses, which are evil on some hardware.

This patch updates uses of those routines in the gadget stack.  The
references into packed structures can just use leXX_to_cpu(*x), which
in most cases is more natural.  Some other uses in RNDIS, mostly in
debug code, were wrong in the first place; those use get_unaligned().

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
David Brownell authored and Greg Kroah-Hartman committed Jun 8, 2007
1 parent 97cb95d commit 01ee7d7
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 24 deletions.
2 changes: 1 addition & 1 deletion drivers/usb/gadget/epautoconf.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ ep_matches (
* where it's an output parameter representing the full speed limit.
* the usb spec fixes high speed bulk maxpacket at 512 bytes.
*/
max = 0x7ff & le16_to_cpup (&desc->wMaxPacketSize);
max = 0x7ff & le16_to_cpu(desc->wMaxPacketSize);
switch (type) {
case USB_ENDPOINT_XFER_INT:
/* INT: limit 64 bytes full speed, 1024 high speed */
Expand Down
8 changes: 4 additions & 4 deletions drivers/usb/gadget/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1369,12 +1369,12 @@ config_buf (struct dev_data *dev, u8 type, unsigned index)
hs = !hs;
if (hs) {
dev->req->buf = dev->hs_config;
len = le16_to_cpup (&dev->hs_config->wTotalLength);
len = le16_to_cpu(dev->hs_config->wTotalLength);
} else
#endif
{
dev->req->buf = dev->config;
len = le16_to_cpup (&dev->config->wTotalLength);
len = le16_to_cpu(dev->config->wTotalLength);
}
((u8 *)dev->req->buf) [1] = type;
return len;
Expand Down Expand Up @@ -1885,7 +1885,7 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)

/* full or low speed config */
dev->config = (void *) kbuf;
total = le16_to_cpup (&dev->config->wTotalLength);
total = le16_to_cpu(dev->config->wTotalLength);
if (!is_valid_config (dev->config) || total >= length)
goto fail;
kbuf += total;
Expand All @@ -1894,7 +1894,7 @@ dev_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
/* optional high speed config */
if (kbuf [1] == USB_DT_CONFIG) {
dev->hs_config = (void *) kbuf;
total = le16_to_cpup (&dev->hs_config->wTotalLength);
total = le16_to_cpu(dev->hs_config->wTotalLength);
if (!is_valid_config (dev->hs_config) || total >= length)
goto fail;
kbuf += total;
Expand Down
6 changes: 3 additions & 3 deletions drivers/usb/gadget/net2280.c
Original file line number Diff line number Diff line change
Expand Up @@ -2440,9 +2440,9 @@ static void handle_stat0_irqs (struct net2280 *dev, u32 stat)

tmp = 0;

#define w_value le16_to_cpup (&u.r.wValue)
#define w_index le16_to_cpup (&u.r.wIndex)
#define w_length le16_to_cpup (&u.r.wLength)
#define w_value le16_to_cpu(u.r.wValue)
#define w_index le16_to_cpu(u.r.wIndex)
#define w_length le16_to_cpu(u.r.wLength)

/* ack the irq */
writel (1 << SETUP_PACKET_INTERRUPT, &dev->regs->irqstat0);
Expand Down
6 changes: 3 additions & 3 deletions drivers/usb/gadget/omap_udc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1651,9 +1651,9 @@ static void ep0_irq(struct omap_udc *udc, u16 irq_src)
UDC_EP_NUM_REG = 0;
} while (UDC_IRQ_SRC_REG & UDC_SETUP);

#define w_value le16_to_cpup (&u.r.wValue)
#define w_index le16_to_cpup (&u.r.wIndex)
#define w_length le16_to_cpup (&u.r.wLength)
#define w_value le16_to_cpu(u.r.wValue)
#define w_index le16_to_cpu(u.r.wIndex)
#define w_length le16_to_cpu(u.r.wLength)

/* Delegate almost all control requests to the gadget driver,
* except for a handful of ch9 status/feature requests that
Expand Down
35 changes: 22 additions & 13 deletions drivers/usb/gadget/rndis.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,14 @@ gen_ndis_query_resp (int configNr, u32 OID, u8 *buf, unsigned buf_len,
DEBUG("query OID %08x value, len %d:\n", OID, buf_len);
for (i = 0; i < buf_len; i += 16) {
DEBUG ("%03d: %08x %08x %08x %08x\n", i,
le32_to_cpup((__le32 *)&buf[i]),
le32_to_cpup((__le32 *)&buf[i + 4]),
le32_to_cpup((__le32 *)&buf[i + 8]),
le32_to_cpup((__le32 *)&buf[i + 12]));
le32_to_cpu(get_unaligned((__le32 *)
&buf[i])),
le32_to_cpu(get_unaligned((__le32 *)
&buf[i + 4])),
le32_to_cpu(get_unaligned((__le32 *)
&buf[i + 8])),
le32_to_cpu(get_unaligned((__le32 *)
&buf[i + 12])));
}
}

Expand Down Expand Up @@ -665,7 +669,7 @@ gen_ndis_query_resp (int configNr, u32 OID, u8 *buf, unsigned buf_len,
break;
case OID_PNP_QUERY_POWER:
DEBUG("%s: OID_PNP_QUERY_POWER D%d\n", __FUNCTION__,
le32_to_cpup((__le32 *) buf) - 1);
le32_to_cpu(get_unaligned((__le32 *)buf)) - 1);
/* only suspend is a real power state, and
* it can't be entered by OID_PNP_SET_POWER...
*/
Expand Down Expand Up @@ -704,10 +708,14 @@ static int gen_ndis_set_resp (u8 configNr, u32 OID, u8 *buf, u32 buf_len,
DEBUG("set OID %08x value, len %d:\n", OID, buf_len);
for (i = 0; i < buf_len; i += 16) {
DEBUG ("%03d: %08x %08x %08x %08x\n", i,
le32_to_cpup((__le32 *)&buf[i]),
le32_to_cpup((__le32 *)&buf[i + 4]),
le32_to_cpup((__le32 *)&buf[i + 8]),
le32_to_cpup((__le32 *)&buf[i + 12]));
le32_to_cpu(get_unaligned((__le32 *)
&buf[i])),
le32_to_cpu(get_unaligned((__le32 *)
&buf[i + 4])),
le32_to_cpu(get_unaligned((__le32 *)
&buf[i + 8])),
le32_to_cpu(get_unaligned((__le32 *)
&buf[i + 12])));
}
}

Expand All @@ -721,7 +729,8 @@ static int gen_ndis_set_resp (u8 configNr, u32 OID, u8 *buf, u32 buf_len,
* PROMISCUOUS, DIRECTED,
* MULTICAST, ALL_MULTICAST, BROADCAST
*/
*params->filter = (u16) le32_to_cpup((__le32 *)buf);
*params->filter = (u16) le32_to_cpu(get_unaligned(
(__le32 *)buf));
DEBUG("%s: OID_GEN_CURRENT_PACKET_FILTER %08x\n",
__FUNCTION__, *params->filter);

Expand Down Expand Up @@ -771,7 +780,7 @@ static int gen_ndis_set_resp (u8 configNr, u32 OID, u8 *buf, u32 buf_len,
* resuming, Windows forces a reset, and then SET_POWER D0.
* FIXME ... then things go batty; Windows wedges itself.
*/
i = le32_to_cpup((__force __le32 *)buf);
i = le32_to_cpu(get_unaligned((__le32 *)buf));
DEBUG("%s: OID_PNP_SET_POWER D%d\n", __FUNCTION__, i - 1);
switch (i) {
case NdisDeviceStateD0:
Expand Down Expand Up @@ -1058,8 +1067,8 @@ int rndis_msg_parser (u8 configNr, u8 *buf)
return -ENOMEM;

tmp = (__le32 *) buf;
MsgType = le32_to_cpup(tmp++);
MsgLength = le32_to_cpup(tmp++);
MsgType = le32_to_cpu(get_unaligned(tmp++));
MsgLength = le32_to_cpu(get_unaligned(tmp++));

if (configNr >= RNDIS_MAX_CONFIGS)
return -ENOTSUPP;
Expand Down

0 comments on commit 01ee7d7

Please sign in to comment.