| Message ID | 1456439339-18458-1-git-send-email-colin.king@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 25 February 2016 17:28:59 GMT-05:00, Colin King <colin.king@canonical.com> wrote: >From: Colin Ian King <colin.king@canonical.com> > >If the allocation of ivp fails the error handling attempts to >free an uninitialized dma_buf; this data structure just contains >garbage on the stack, so the freeing will cause issues when the >urb, buf and dma fields are cleaned. Fix this by handling the >ivp check and Commit message looks truncated? >Signed-off-by: Colin Ian King <colin.king@canonical.com> >--- > drivers/net/wireless/mediatek/mt7601u/mcu.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > >diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c >b/drivers/net/wireless/mediatek/mt7601u/mcu.c >index fbb1986..06ac657 100644 >--- a/drivers/net/wireless/mediatek/mt7601u/mcu.c >+++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c >@@ -359,13 +359,13 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, >const struct mt76_fw *fw) > struct mt7601u_dma_buf dma_buf; > void *ivb; > u32 ilm_len, dlm_len; >- int i, ret; >+ int i, ret = -ENOMEM; No need... > > ivb = kmemdup(fw->ivb, sizeof(fw->ivb), GFP_KERNEL); >- if (!ivb || mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { >- ret = -ENOMEM; >+ if (!ivb) >+ goto error_ivb; ...just return -ENOMEM here, because... >+error_ivb: >+ kfree(ivb); ...calling kfree on a null pointer is redundant. Thanks!! [trimming To/CC a bit] -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c index fbb1986..06ac657 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c @@ -359,13 +359,13 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, const struct mt76_fw *fw) struct mt7601u_dma_buf dma_buf; void *ivb; u32 ilm_len, dlm_len; - int i, ret; + int i, ret = -ENOMEM; ivb = kmemdup(fw->ivb, sizeof(fw->ivb), GFP_KERNEL); - if (!ivb || mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) { - ret = -ENOMEM; + if (!ivb) + goto error_ivb; + if (mt7601u_usb_alloc_buf(dev, MCU_FW_URB_SIZE, &dma_buf)) goto error; - } ilm_len = le32_to_cpu(fw->hdr.ilm_len) - sizeof(fw->ivb); dev_dbg(dev->dev, "loading FW - ILM %u + IVB %zu\n", @@ -396,8 +396,9 @@ mt7601u_upload_firmware(struct mt7601u_dev *dev, const struct mt76_fw *fw) dev_dbg(dev->dev, "Firmware running!\n"); error: - kfree(ivb); mt7601u_usb_free_buf(dev, &dma_buf); +error_ivb: + kfree(ivb); return ret; }