* [PATCH] rsi: Fix failure to load firmware after memory leak fix and fix the leak
@ 2015-07-27 5:43 Mike Looijmans
2015-07-27 7:55 ` Alexey Khoroshilov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mike Looijmans @ 2015-07-27 5:43 UTC (permalink / raw)
To: linux-wireless-u79uwXL29TY76Z2rM5mHXA
Cc: kvalo-sgV2jX0FEOL9JmXXK+q4OQ, khoroshilov-ufN2psIa012HXe+LvDLADg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mike Looijmans,
stable-u79uwXL29TY76Z2rM5mHXA
Fixes commit eae79b4f3e82ca63a53478a161b190a0d38fe526 ("rsi: fix memory leak
in rsi_load_ta_instructions()") which stopped the driver from functioning.
Firmware data has been allocated using vmalloc(), resulting in memory
that cannot be used for DMA. Hence the firmware was first copied to a
buffer allocated with kmalloc() in the original code. This patch reverts
the commit and only calls "kfree()" to release the buffer after sending
the data. This fixes the memory leak without breaking the driver.
Add a comment to the kmemdup() calls to explain why this is done.
Tested on a Topic Miami-Florida board which contains the rsi SDIO chip.
Also added the same kfree() call to the USB glue driver. This was not
tested on actual hardware though, as I only have the SDIO version.
Signed-off-by: Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org>
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
drivers/net/wireless/rsi/rsi_91x_sdio_ops.c | 6 +++++-
drivers/net/wireless/rsi/rsi_91x_usb_ops.c | 2 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
index b6cc9ff..5c37a71 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
@@ -172,6 +172,7 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
(struct rsi_91x_sdiodev *)adapter->rsi_dev;
u32 len;
u32 num_blocks;
+ const u8 *fw;
const struct firmware *fw_entry = NULL;
u32 block_size = dev->tx_blk_size;
int status = 0;
@@ -200,6 +201,8 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
return status;
}
+ /* Copy firmware into DMA-accessible memory */
+ fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
len = fw_entry->size;
if (len % 4)
@@ -210,7 +213,8 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
rsi_dbg(INIT_ZONE, "%s: Instruction size:%d\n", __func__, len);
rsi_dbg(INIT_ZONE, "%s: num blocks: %d\n", __func__, num_blocks);
- status = rsi_copy_to_card(common, fw_entry->data, len, num_blocks);
+ status = rsi_copy_to_card(common, fw, len, num_blocks);
+ kfree(fw);
release_firmware(fw_entry);
return status;
}
diff --git a/drivers/net/wireless/rsi/rsi_91x_usb_ops.c b/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
index 1106ce7..088e28e 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
@@ -146,6 +146,7 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
return status;
}
+ /* Copy firmware into DMA-accessible memory */
fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
len = fw_entry->size;
@@ -158,6 +159,7 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
rsi_dbg(INIT_ZONE, "%s: num blocks: %d\n", __func__, num_blocks);
status = rsi_copy_to_card(common, fw, len, num_blocks);
+ kfree(fw);
release_firmware(fw_entry);
return status;
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] rsi: Fix failure to load firmware after memory leak fix and fix the leak
2015-07-27 5:43 [PATCH] rsi: Fix failure to load firmware after memory leak fix and fix the leak Mike Looijmans
@ 2015-07-27 7:55 ` Alexey Khoroshilov
2015-07-27 10:28 ` Kalle Valo
2015-07-28 5:51 ` [PATCH v2] " Mike Looijmans
2 siblings, 0 replies; 6+ messages in thread
From: Alexey Khoroshilov @ 2015-07-27 7:55 UTC (permalink / raw)
To: Mike Looijmans, linux-wireless; +Cc: kvalo, netdev, linux-kernel, stable
Reviewed-by: Alexey Khoroshilov <khoroshilov@ispras.ru> with small
suggestion. If we restore kmemdup() call, we have to handle ENOMEM
situations:
fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
if (!fw)
return -ENOMEM;
On 27.07.2015 12:43, Mike Looijmans wrote:
> Fixes commit eae79b4f3e82ca63a53478a161b190a0d38fe526 ("rsi: fix memory leak
> in rsi_load_ta_instructions()") which stopped the driver from functioning.
>
> Firmware data has been allocated using vmalloc(), resulting in memory
> that cannot be used for DMA. Hence the firmware was first copied to a
> buffer allocated with kmalloc() in the original code. This patch reverts
> the commit and only calls "kfree()" to release the buffer after sending
> the data. This fixes the memory leak without breaking the driver.
>
> Add a comment to the kmemdup() calls to explain why this is done.
>
> Tested on a Topic Miami-Florida board which contains the rsi SDIO chip.
>
> Also added the same kfree() call to the USB glue driver. This was not
> tested on actual hardware though, as I only have the SDIO version.
>
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> Cc: stable@vger.kernel.org
> ---
> drivers/net/wireless/rsi/rsi_91x_sdio_ops.c | 6 +++++-
> drivers/net/wireless/rsi/rsi_91x_usb_ops.c | 2 ++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
> index b6cc9ff..5c37a71 100644
> --- a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
> +++ b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
> @@ -172,6 +172,7 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
> (struct rsi_91x_sdiodev *)adapter->rsi_dev;
> u32 len;
> u32 num_blocks;
> + const u8 *fw;
> const struct firmware *fw_entry = NULL;
> u32 block_size = dev->tx_blk_size;
> int status = 0;
> @@ -200,6 +201,8 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
> return status;
> }
>
> + /* Copy firmware into DMA-accessible memory */
> + fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> len = fw_entry->size;
>
> if (len % 4)
> @@ -210,7 +213,8 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
> rsi_dbg(INIT_ZONE, "%s: Instruction size:%d\n", __func__, len);
> rsi_dbg(INIT_ZONE, "%s: num blocks: %d\n", __func__, num_blocks);
>
> - status = rsi_copy_to_card(common, fw_entry->data, len, num_blocks);
> + status = rsi_copy_to_card(common, fw, len, num_blocks);
> + kfree(fw);
> release_firmware(fw_entry);
> return status;
> }
> diff --git a/drivers/net/wireless/rsi/rsi_91x_usb_ops.c b/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
> index 1106ce7..088e28e 100644
> --- a/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
> +++ b/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
> @@ -146,6 +146,7 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
> return status;
> }
>
> + /* Copy firmware into DMA-accessible memory */
> fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
> len = fw_entry->size;
>
> @@ -158,6 +159,7 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
> rsi_dbg(INIT_ZONE, "%s: num blocks: %d\n", __func__, num_blocks);
>
> status = rsi_copy_to_card(common, fw, len, num_blocks);
> + kfree(fw);
> release_firmware(fw_entry);
> return status;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rsi: Fix failure to load firmware after memory leak fix and fix the leak
2015-07-27 5:43 [PATCH] rsi: Fix failure to load firmware after memory leak fix and fix the leak Mike Looijmans
2015-07-27 7:55 ` Alexey Khoroshilov
@ 2015-07-27 10:28 ` Kalle Valo
2015-07-28 5:53 ` Mike Looijmans
2015-07-28 5:51 ` [PATCH v2] " Mike Looijmans
2 siblings, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2015-07-27 10:28 UTC (permalink / raw)
To: Mike Looijmans; +Cc: linux-wireless, khoroshilov, netdev, linux-kernel, stable
Mike Looijmans <mike.looijmans@topic.nl> writes:
> Fixes commit eae79b4f3e82ca63a53478a161b190a0d38fe526 ("rsi: fix memory leak
> in rsi_load_ta_instructions()") which stopped the driver from functioning.
You can abbreviate the commit id:
Fixes commit eae79b4f3e82 ("rsi: fix memory leak in
rsi_load_ta_instructions()") which stopped the driver from functioning.
> Firmware data has been allocated using vmalloc(), resulting in memory
> that cannot be used for DMA. Hence the firmware was first copied to a
> buffer allocated with kmalloc() in the original code. This patch reverts
> the commit and only calls "kfree()" to release the buffer after sending
> the data. This fixes the memory leak without breaking the driver.
>
> Add a comment to the kmemdup() calls to explain why this is done.
>
> Tested on a Topic Miami-Florida board which contains the rsi SDIO chip.
>
> Also added the same kfree() call to the USB glue driver. This was not
> tested on actual hardware though, as I only have the SDIO version.
>
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
Add this before Signed-off-by line:
Fixes: eae79b4f3e82 ("rsi: fix memory leak in rsi_load_ta_instructions()")
> Cc: stable@vger.kernel.org
Also no need to send email to stable@vger.kernel.org list, this line is
enough and the stable team will pick the commit automatically.
--
Kalle Valo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rsi: Fix failure to load firmware after memory leak fix and fix the leak
2015-07-27 10:28 ` Kalle Valo
@ 2015-07-28 5:53 ` Mike Looijmans
0 siblings, 0 replies; 6+ messages in thread
From: Mike Looijmans @ 2015-07-28 5:53 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, khoroshilov, netdev, linux-kernel, stable
On 27-07-15 12:28, Kalle Valo wrote:
> Mike Looijmans <mike.looijmans@topic.nl> writes:
>
>> Fixes commit eae79b4f3e82ca63a53478a161b190a0d38fe526 ("rsi: fix memory leak
>> in rsi_load_ta_instructions()") which stopped the driver from functioning.
>
> You can abbreviate the commit id:
>
> Fixes commit eae79b4f3e82 ("rsi: fix memory leak in
> rsi_load_ta_instructions()") which stopped the driver from functioning.
>
>> Firmware data has been allocated using vmalloc(), resulting in memory
>> that cannot be used for DMA. Hence the firmware was first copied to a
>> buffer allocated with kmalloc() in the original code. This patch reverts
>> the commit and only calls "kfree()" to release the buffer after sending
>> the data. This fixes the memory leak without breaking the driver.
>>
>> Add a comment to the kmemdup() calls to explain why this is done.
>>
>> Tested on a Topic Miami-Florida board which contains the rsi SDIO chip.
>>
>> Also added the same kfree() call to the USB glue driver. This was not
>> tested on actual hardware though, as I only have the SDIO version.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>
> Add this before Signed-off-by line:
>
> Fixes: eae79b4f3e82 ("rsi: fix memory leak in rsi_load_ta_instructions()")
>
>> Cc: stable@vger.kernel.org
>
> Also no need to send email to stable@vger.kernel.org list, this line is
> enough and the stable team will pick the commit automatically.
I wondered why that happened, and just noticed that git send-email
automatically added this to the recipients. So it happened for v2 as well,
sorry for that.
Kind regards,
Mike Looijmans
System Expert
TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com
Please consider the environment before printing this e-mail
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] rsi: Fix failure to load firmware after memory leak fix and fix the leak
2015-07-27 5:43 [PATCH] rsi: Fix failure to load firmware after memory leak fix and fix the leak Mike Looijmans
2015-07-27 7:55 ` Alexey Khoroshilov
2015-07-27 10:28 ` Kalle Valo
@ 2015-07-28 5:51 ` Mike Looijmans
2015-07-31 6:23 ` [v2] rsi: Fix failure to load firmware after memory leak fix and fixthe leak Kalle Valo
2 siblings, 1 reply; 6+ messages in thread
From: Mike Looijmans @ 2015-07-28 5:51 UTC (permalink / raw)
To: linux-wireless
Cc: kvalo, khoroshilov, netdev, linux-kernel, Mike Looijmans, stable
Fixes commit eae79b4f3e82 ("rsi: fix memory leak in rsi_load_ta_instructions()")
which stopped the driver from functioning.
Firmware data has been allocated using vmalloc(), resulting in memory
that cannot be used for DMA. Hence the firmware was first copied to a
buffer allocated with kmalloc() in the original code. This patch reverts
the commit and only calls "kfree()" to release the buffer after sending
the data. This fixes the memory leak without breaking the driver.
Add a comment to the kmemdup() calls to explain why this is done, and abort
if memory allocation fails.
Tested on a Topic Miami-Florida board which contains the rsi SDIO chip.
Also added the same kfree() call to the USB glue driver. This was not
tested on actual hardware though, as I only have the SDIO version.
Fixes: eae79b4f3e82 ("rsi: fix memory leak in rsi_load_ta_instructions()")
Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
Cc: stable@vger.kernel.org
---
v2: Add "Fixes:" header and abbreviate git hashes.
Return -ENOMEM if kmemdup() fails.
drivers/net/wireless/rsi/rsi_91x_sdio_ops.c | 8 +++++++-
drivers/net/wireless/rsi/rsi_91x_usb_ops.c | 4 ++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
index b6cc9ff..1c6788a 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
@@ -172,6 +172,7 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
(struct rsi_91x_sdiodev *)adapter->rsi_dev;
u32 len;
u32 num_blocks;
+ const u8 *fw;
const struct firmware *fw_entry = NULL;
u32 block_size = dev->tx_blk_size;
int status = 0;
@@ -200,6 +201,10 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
return status;
}
+ /* Copy firmware into DMA-accessible memory */
+ fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
+ if (!fw)
+ return -ENOMEM;
len = fw_entry->size;
if (len % 4)
@@ -210,7 +215,8 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
rsi_dbg(INIT_ZONE, "%s: Instruction size:%d\n", __func__, len);
rsi_dbg(INIT_ZONE, "%s: num blocks: %d\n", __func__, num_blocks);
- status = rsi_copy_to_card(common, fw_entry->data, len, num_blocks);
+ status = rsi_copy_to_card(common, fw, len, num_blocks);
+ kfree(fw);
release_firmware(fw_entry);
return status;
}
diff --git a/drivers/net/wireless/rsi/rsi_91x_usb_ops.c b/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
index 1106ce7..30c2cf7 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb_ops.c
@@ -146,7 +146,10 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
return status;
}
+ /* Copy firmware into DMA-accessible memory */
fw = kmemdup(fw_entry->data, fw_entry->size, GFP_KERNEL);
+ if (!fw)
+ return -ENOMEM;
len = fw_entry->size;
if (len % 4)
@@ -158,6 +161,7 @@ static int rsi_load_ta_instructions(struct rsi_common *common)
rsi_dbg(INIT_ZONE, "%s: num blocks: %d\n", __func__, num_blocks);
status = rsi_copy_to_card(common, fw, len, num_blocks);
+ kfree(fw);
release_firmware(fw_entry);
return status;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [v2] rsi: Fix failure to load firmware after memory leak fix and fixthe leak
2015-07-28 5:51 ` [PATCH v2] " Mike Looijmans
@ 2015-07-31 6:23 ` Kalle Valo
0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2015-07-31 6:23 UTC (permalink / raw)
To: Mike Looijmans
Cc: linux-wireless, khoroshilov, netdev, linux-kernel, Mike Looijmans,
stable
> Fixes commit eae79b4f3e82 ("rsi: fix memory leak in rsi_load_ta_instructions()")
> which stopped the driver from functioning.
>
> Firmware data has been allocated using vmalloc(), resulting in memory
> that cannot be used for DMA. Hence the firmware was first copied to a
> buffer allocated with kmalloc() in the original code. This patch reverts
> the commit and only calls "kfree()" to release the buffer after sending
> the data. This fixes the memory leak without breaking the driver.
>
> Add a comment to the kmemdup() calls to explain why this is done, and abort
> if memory allocation fails.
>
> Tested on a Topic Miami-Florida board which contains the rsi SDIO chip.
>
> Also added the same kfree() call to the USB glue driver. This was not
> tested on actual hardware though, as I only have the SDIO version.
>
> Fixes: eae79b4f3e82 ("rsi: fix memory leak in rsi_load_ta_instructions()")
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> Cc: stable@vger.kernel.org
Thanks, applied to wireless-drivers.git.
Kalle Valo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-31 6:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-27 5:43 [PATCH] rsi: Fix failure to load firmware after memory leak fix and fix the leak Mike Looijmans
2015-07-27 7:55 ` Alexey Khoroshilov
2015-07-27 10:28 ` Kalle Valo
2015-07-28 5:53 ` Mike Looijmans
2015-07-28 5:51 ` [PATCH v2] " Mike Looijmans
2015-07-31 6:23 ` [v2] rsi: Fix failure to load firmware after memory leak fix and fixthe leak Kalle Valo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).