| Message ID | alpine.LNX.2.00.1101132229260.11347@swampdragon.chaosbits.net |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Jesper Juhl <jj@chaosbits.net> Date: Thu, 13 Jan 2011 22:40:11 +0100 (CET) > skb_clone() dynamically allocates memory and may fail. If it does it > returns NULL. This means we'll dereference a NULL pointer in > drivers/net/usb/cdc_ncm.c::cdc_ncm_rx_fixup(). > As far as I can tell, the proper way to deal with this is simply to goto > the error label. > > Furthermore gcc complains that 'skb' may be used uninitialized: > drivers/net/usb/cdc_ncm.c: In function ‘cdc_ncm_rx_fixup’: > drivers/net/usb/cdc_ncm.c:922:18: warning: ‘skb’ may be used uninitialized in this function > and I believe it is right. On the line where we > pr_debug("invalid frame detected (ignored)" ... > we are using the local variable 'skb' but nothing has ever been assigned > to that variable yet. I believe the correct fix for that is to use > 'skb_in' instead. > > Signed-off-by: Jesper Juhl <jj@chaosbits.net> Applied.
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 593c104..d776c4a 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -1021,13 +1021,15 @@ static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in) (temp > CDC_NCM_MAX_DATAGRAM_SIZE) || (temp < ETH_HLEN)) { pr_debug("invalid frame detected (ignored)" "offset[%u]=%u, length=%u, skb=%p\n", - x, offset, temp, skb); + x, offset, temp, skb_in); if (!x) goto error; break; } else { skb = skb_clone(skb_in, GFP_ATOMIC); + if (!skb) + goto error; skb->len = temp; skb->data = ((u8 *)skb_in->data) + offset; skb_set_tail_pointer(skb, temp);
skb_clone() dynamically allocates memory and may fail. If it does it returns NULL. This means we'll dereference a NULL pointer in drivers/net/usb/cdc_ncm.c::cdc_ncm_rx_fixup(). As far as I can tell, the proper way to deal with this is simply to goto the error label. Furthermore gcc complains that 'skb' may be used uninitialized: drivers/net/usb/cdc_ncm.c: In function ‘cdc_ncm_rx_fixup’: drivers/net/usb/cdc_ncm.c:922:18: warning: ‘skb’ may be used uninitialized in this function and I believe it is right. On the line where we pr_debug("invalid frame detected (ignored)" ... we are using the local variable 'skb' but nothing has ever been assigned to that variable yet. I believe the correct fix for that is to use 'skb_in' instead. Signed-off-by: Jesper Juhl <jj@chaosbits.net> --- cdc_ncm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) compile tested only.