Commit 6ab8d4d1eb6a7db792397e4a6a61a5a0a51a7411
Committed by
Greg Kroah-Hartman
1 parent
97ab28073b
Exists in
smarc-ti-linux-3.14.y
and in
1 other branch
can: kvaser_usb: Don't free packets when tight on URBs
commit b442723fcec445fb0ae1104888dd22cd285e0a91 upstream. Flooding the Kvaser CAN to USB dongle with multiple reads and writes in high frequency caused seemingly-random panics in the kernel. On further inspection, it seems the driver erroneously freed the to-be-transmitted packet upon getting tight on URBs and returning NETDEV_TX_BUSY, leading to invalid memory writes and double frees at a later point in time. Note: Finding no more URBs/transmit-contexts and returning NETDEV_TX_BUSY is a driver bug in and out of itself: it means that our start/stop queue flow control is broken. This patch only fixes the (buggy) error handling code; the root cause shall be fixed in a later commit. Acked-by: Olivier Sobrie <olivier@sobrie.be> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Showing 1 changed file with 4 additions and 6 deletions Side-by-side Diff
drivers/net/can/usb/kvaser_usb.c
... | ... | @@ -1285,12 +1285,14 @@ |
1285 | 1285 | if (!urb) { |
1286 | 1286 | netdev_err(netdev, "No memory left for URBs\n"); |
1287 | 1287 | stats->tx_dropped++; |
1288 | - goto nourbmem; | |
1288 | + dev_kfree_skb(skb); | |
1289 | + return NETDEV_TX_OK; | |
1289 | 1290 | } |
1290 | 1291 | |
1291 | 1292 | buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC); |
1292 | 1293 | if (!buf) { |
1293 | 1294 | stats->tx_dropped++; |
1295 | + dev_kfree_skb(skb); | |
1294 | 1296 | goto nobufmem; |
1295 | 1297 | } |
1296 | 1298 | |
... | ... | @@ -1325,6 +1327,7 @@ |
1325 | 1327 | } |
1326 | 1328 | } |
1327 | 1329 | |
1330 | + /* This should never happen; it implies a flow control bug */ | |
1328 | 1331 | if (!context) { |
1329 | 1332 | netdev_warn(netdev, "cannot find free context\n"); |
1330 | 1333 | ret = NETDEV_TX_BUSY; |
... | ... | @@ -1355,9 +1358,6 @@ |
1355 | 1358 | if (unlikely(err)) { |
1356 | 1359 | can_free_echo_skb(netdev, context->echo_index); |
1357 | 1360 | |
1358 | - skb = NULL; /* set to NULL to avoid double free in | |
1359 | - * dev_kfree_skb(skb) */ | |
1360 | - | |
1361 | 1361 | atomic_dec(&priv->active_tx_urbs); |
1362 | 1362 | usb_unanchor_urb(urb); |
1363 | 1363 | |
... | ... | @@ -1379,8 +1379,6 @@ |
1379 | 1379 | kfree(buf); |
1380 | 1380 | nobufmem: |
1381 | 1381 | usb_free_urb(urb); |
1382 | -nourbmem: | |
1383 | - dev_kfree_skb(skb); | |
1384 | 1382 | return ret; |
1385 | 1383 | } |
1386 | 1384 |