From 94912e8df4e5005cb5fa3f3603741eb5e8bf3334 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 5 Apr 2018 11:20:06 -0500 Subject: [PATCH 1/4] ieee802154: mcr20a: Fix memory leak in mcr20a_probe Free allocated memory for pdata before return. Addresses-Coverity-ID: 1466096 ("Resource leak") Fixes: 8c6ad9cc5157 ("ieee802154: Add NXP MCR20A IEEE 802.15.4 transceiver driver") Signed-off-by: Gustavo A. R. Silva Acked-by: Xue Liu Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/mcr20a.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c index 55a22c7618082..944470d69ba9a 100644 --- a/drivers/net/ieee802154/mcr20a.c +++ b/drivers/net/ieee802154/mcr20a.c @@ -1267,7 +1267,7 @@ mcr20a_probe(struct spi_device *spi) ret = mcr20a_get_platform_data(spi, pdata); if (ret < 0) { dev_crit(&spi->dev, "mcr20a_get_platform_data failed.\n"); - return ret; + goto free_pdata; } /* init reset gpio */ @@ -1275,7 +1275,7 @@ mcr20a_probe(struct spi_device *spi) ret = devm_gpio_request_one(&spi->dev, pdata->rst_gpio, GPIOF_OUT_INIT_HIGH, "reset"); if (ret) - return ret; + goto free_pdata; } /* reset mcr20a */ @@ -1291,7 +1291,8 @@ mcr20a_probe(struct spi_device *spi) hw = ieee802154_alloc_hw(sizeof(*lp), &mcr20a_hw_ops); if (!hw) { dev_crit(&spi->dev, "ieee802154_alloc_hw failed\n"); - return -ENOMEM; + ret = -ENOMEM; + goto free_pdata; } /* init mcr20a local data */ @@ -1366,6 +1367,8 @@ mcr20a_probe(struct spi_device *spi) free_dev: ieee802154_free_hw(lp->hw); +free_pdata: + kfree(pdata); return ret; } From f9e628a6645ed4f42a15dfa3c5af45a916a228e4 Mon Sep 17 00:00:00 2001 From: Jia-Ju Bai Date: Wed, 11 Apr 2018 10:14:10 +0800 Subject: [PATCH 2/4] net: ieee802154: atusb: Replace GFP_ATOMIC with GFP_KERNEL in atusb_probe atusb_probe() is never called in atomic context. This function is only set as ".probe" in struct usb_driver. Despite never getting called from atomic context, atusb_probe() calls usb_alloc_urb() with GFP_ATOMIC, which does not sleep for allocation. GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL, which can sleep and improve the possibility of sucessful allocation. This is found by a static analysis tool named DCNS written by myself. And I also manually check it. Signed-off-by: Jia-Ju Bai Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/atusb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c index 9fb9b565a002f..4f684cbcdc57e 100644 --- a/drivers/net/ieee802154/atusb.c +++ b/drivers/net/ieee802154/atusb.c @@ -1045,7 +1045,7 @@ static int atusb_probe(struct usb_interface *interface, atusb->tx_dr.bRequest = ATUSB_TX; atusb->tx_dr.wValue = cpu_to_le16(0); - atusb->tx_urb = usb_alloc_urb(0, GFP_ATOMIC); + atusb->tx_urb = usb_alloc_urb(0, GFP_KERNEL); if (!atusb->tx_urb) goto fail; From 7e0ffee1b2e7d26cc2147be0d9d5186823e923f3 Mon Sep 17 00:00:00 2001 From: Stefan Schmidt Date: Thu, 12 Apr 2018 14:28:49 +0200 Subject: [PATCH 3/4] net: ieee802154: mcr20a: do not leak resources on error path We already allocated the device and platform data at this point. Instead of simply return from the probe function we need to cleanup the resources first. Signed-off-by: Stefan Schmidt Acked-by: Xue Liu --- drivers/net/ieee802154/mcr20a.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c index 944470d69ba9a..de0d7f28a181c 100644 --- a/drivers/net/ieee802154/mcr20a.c +++ b/drivers/net/ieee802154/mcr20a.c @@ -1309,8 +1309,10 @@ mcr20a_probe(struct spi_device *spi) /* init buf */ lp->buf = devm_kzalloc(&spi->dev, SPI_COMMAND_BUFFER, GFP_KERNEL); - if (!lp->buf) - return -ENOMEM; + if (!lp->buf) { + ret = -ENOMEM; + goto free_dev; + } mcr20a_setup_tx_spi_messages(lp); mcr20a_setup_rx_spi_messages(lp); From f18fa5de5ba7f1d6650951502bb96a6e4715a948 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Fri, 20 Apr 2018 14:54:13 -0400 Subject: [PATCH 4/4] net: ieee802154: 6lowpan: fix frag reassembly This patch initialize stack variables which are used in frag_lowpan_compare_key to zero. In my case there are padding bytes in the structures ieee802154_addr as well in frag_lowpan_compare_key. Otherwise the key variable contains random bytes. The result is that a compare of two keys by memcmp works incorrect. Fixes: 648700f76b03 ("inet: frags: use rhashtables for reassembly units") Signed-off-by: Alexander Aring Reported-by: Stefan Schmidt Signed-off-by: Stefan Schmidt --- net/ieee802154/6lowpan/6lowpan_i.h | 4 ++-- net/ieee802154/6lowpan/reassembly.c | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/net/ieee802154/6lowpan/6lowpan_i.h b/net/ieee802154/6lowpan/6lowpan_i.h index b8d95cb71c25d..44a7e16bf3b5e 100644 --- a/net/ieee802154/6lowpan/6lowpan_i.h +++ b/net/ieee802154/6lowpan/6lowpan_i.h @@ -20,8 +20,8 @@ typedef unsigned __bitwise lowpan_rx_result; struct frag_lowpan_compare_key { u16 tag; u16 d_size; - const struct ieee802154_addr src; - const struct ieee802154_addr dst; + struct ieee802154_addr src; + struct ieee802154_addr dst; }; /* Equivalent of ipv4 struct ipq diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c index 1790b65944b3e..2cc224106b692 100644 --- a/net/ieee802154/6lowpan/reassembly.c +++ b/net/ieee802154/6lowpan/reassembly.c @@ -75,14 +75,14 @@ fq_find(struct net *net, const struct lowpan_802154_cb *cb, { struct netns_ieee802154_lowpan *ieee802154_lowpan = net_ieee802154_lowpan(net); - struct frag_lowpan_compare_key key = { - .tag = cb->d_tag, - .d_size = cb->d_size, - .src = *src, - .dst = *dst, - }; + struct frag_lowpan_compare_key key = {}; struct inet_frag_queue *q; + key.tag = cb->d_tag; + key.d_size = cb->d_size; + key.src = *src; + key.dst = *dst; + q = inet_frag_find(&ieee802154_lowpan->frags, &key); if (!q) return NULL; @@ -372,7 +372,7 @@ int lowpan_frag_rcv(struct sk_buff *skb, u8 frag_type) struct lowpan_frag_queue *fq; struct net *net = dev_net(skb->dev); struct lowpan_802154_cb *cb = lowpan_802154_cb(skb); - struct ieee802154_hdr hdr; + struct ieee802154_hdr hdr = {}; int err; if (ieee802154_hdr_peek_addrs(skb, &hdr) < 0)