* [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file
@ 2012-03-16 0:08 Michal Schmidt
2012-03-16 0:08 ` [PATCH net 2/2] bnx2x: fix memory leak in bnx2x_init_firmware() Michal Schmidt
2012-03-16 7:24 ` [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file Eilon Greenstein
0 siblings, 2 replies; 6+ messages in thread
From: Michal Schmidt @ 2012-03-16 0:08 UTC (permalink / raw)
To: netdev; +Cc: Yuval Mintz, Yaniv Rosner, Eilon Greenstein, Dmitry Kravkov
If the requested firmware is deemed corrupt and then released, reset the
pointer to NULL in order to avoid double-freeing it in
bnx2x_release_firmware() or dereferencing it in bnx2x_init_firmware().
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 2545213..00ff62f 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -10901,6 +10901,7 @@ init_ops_alloc_err:
kfree(bp->init_data);
request_firmware_exit:
release_firmware(bp->firmware);
+ bp->firmware = NULL;
return rc;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] bnx2x: fix memory leak in bnx2x_init_firmware()
2012-03-16 0:08 [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file Michal Schmidt
@ 2012-03-16 0:08 ` Michal Schmidt
2012-03-16 7:24 ` Eilon Greenstein
2012-03-16 7:24 ` [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file Eilon Greenstein
1 sibling, 1 reply; 6+ messages in thread
From: Michal Schmidt @ 2012-03-16 0:08 UTC (permalink / raw)
To: netdev; +Cc: Yuval Mintz, Yaniv Rosner, Eilon Greenstein, Dmitry Kravkov
When cycling the interface down and up, bnx2x_init_firmware() knows that
the firmware is already loaded, but nevertheless it allocates certain
arrays anew (init_data, init_ops, init_ops_offsets, iro_arr). The old
arrays are leaked.
Fix the leaks by returning early if the firmware was already loaded.
Because if the firmware is loaded, so are the arrays.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 50 ++++++++++-----------
1 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 00ff62f..b69f876 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -10824,38 +10824,36 @@ do { \
int bnx2x_init_firmware(struct bnx2x *bp)
{
+ const char *fw_file_name;
struct bnx2x_fw_file_hdr *fw_hdr;
int rc;
+ if (bp->firmware)
+ return 0;
- if (!bp->firmware) {
- const char *fw_file_name;
-
- if (CHIP_IS_E1(bp))
- fw_file_name = FW_FILE_NAME_E1;
- else if (CHIP_IS_E1H(bp))
- fw_file_name = FW_FILE_NAME_E1H;
- else if (!CHIP_IS_E1x(bp))
- fw_file_name = FW_FILE_NAME_E2;
- else {
- BNX2X_ERR("Unsupported chip revision\n");
- return -EINVAL;
- }
- BNX2X_DEV_INFO("Loading %s\n", fw_file_name);
+ if (CHIP_IS_E1(bp))
+ fw_file_name = FW_FILE_NAME_E1;
+ else if (CHIP_IS_E1H(bp))
+ fw_file_name = FW_FILE_NAME_E1H;
+ else if (!CHIP_IS_E1x(bp))
+ fw_file_name = FW_FILE_NAME_E2;
+ else {
+ BNX2X_ERR("Unsupported chip revision\n");
+ return -EINVAL;
+ }
+ BNX2X_DEV_INFO("Loading %s\n", fw_file_name);
- rc = request_firmware(&bp->firmware, fw_file_name,
- &bp->pdev->dev);
- if (rc) {
- BNX2X_ERR("Can't load firmware file %s\n",
- fw_file_name);
- goto request_firmware_exit;
- }
+ rc = request_firmware(&bp->firmware, fw_file_name, &bp->pdev->dev);
+ if (rc) {
+ BNX2X_ERR("Can't load firmware file %s\n",
+ fw_file_name);
+ goto request_firmware_exit;
+ }
- rc = bnx2x_check_firmware(bp);
- if (rc) {
- BNX2X_ERR("Corrupt firmware file %s\n", fw_file_name);
- goto request_firmware_exit;
- }
+ rc = bnx2x_check_firmware(bp);
+ if (rc) {
+ BNX2X_ERR("Corrupt firmware file %s\n", fw_file_name);
+ goto request_firmware_exit;
}
fw_hdr = (struct bnx2x_fw_file_hdr *)bp->firmware->data;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file
2012-03-16 0:08 [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file Michal Schmidt
2012-03-16 0:08 ` [PATCH net 2/2] bnx2x: fix memory leak in bnx2x_init_firmware() Michal Schmidt
@ 2012-03-16 7:24 ` Eilon Greenstein
2012-03-16 8:57 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Eilon Greenstein @ 2012-03-16 7:24 UTC (permalink / raw)
To: Michal Schmidt; +Cc: netdev, Yuval Mintz, Yaniv Rosner, Dmitry Kravkov
On Fri, 2012-03-16 at 01:08 +0100, Michal Schmidt wrote:
> If the requested firmware is deemed corrupt and then released, reset the
> pointer to NULL in order to avoid double-freeing it in
> bnx2x_release_firmware() or dereferencing it in bnx2x_init_firmware().
>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Acked-by: Eilon Greenstein <eilong@broadcom.com>
Thanks Michal!
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index 2545213..00ff62f 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -10901,6 +10901,7 @@ init_ops_alloc_err:
> kfree(bp->init_data);
> request_firmware_exit:
> release_firmware(bp->firmware);
> + bp->firmware = NULL;
>
> return rc;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] bnx2x: fix memory leak in bnx2x_init_firmware()
2012-03-16 0:08 ` [PATCH net 2/2] bnx2x: fix memory leak in bnx2x_init_firmware() Michal Schmidt
@ 2012-03-16 7:24 ` Eilon Greenstein
2012-03-16 8:57 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Eilon Greenstein @ 2012-03-16 7:24 UTC (permalink / raw)
To: Michal Schmidt; +Cc: netdev, Yuval Mintz, Yaniv Rosner, Dmitry Kravkov
On Fri, 2012-03-16 at 01:08 +0100, Michal Schmidt wrote:
> When cycling the interface down and up, bnx2x_init_firmware() knows that
> the firmware is already loaded, but nevertheless it allocates certain
> arrays anew (init_data, init_ops, init_ops_offsets, iro_arr). The old
> arrays are leaked.
>
> Fix the leaks by returning early if the firmware was already loaded.
> Because if the firmware is loaded, so are the arrays.
>
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
Acked-by: Eilon Greenstein <eilong@broadcom.com>
Thanks Michal!
> ---
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 50 ++++++++++-----------
> 1 files changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index 00ff62f..b69f876 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -10824,38 +10824,36 @@ do { \
>
> int bnx2x_init_firmware(struct bnx2x *bp)
> {
> + const char *fw_file_name;
> struct bnx2x_fw_file_hdr *fw_hdr;
> int rc;
>
> + if (bp->firmware)
> + return 0;
>
> - if (!bp->firmware) {
> - const char *fw_file_name;
> -
> - if (CHIP_IS_E1(bp))
> - fw_file_name = FW_FILE_NAME_E1;
> - else if (CHIP_IS_E1H(bp))
> - fw_file_name = FW_FILE_NAME_E1H;
> - else if (!CHIP_IS_E1x(bp))
> - fw_file_name = FW_FILE_NAME_E2;
> - else {
> - BNX2X_ERR("Unsupported chip revision\n");
> - return -EINVAL;
> - }
> - BNX2X_DEV_INFO("Loading %s\n", fw_file_name);
> + if (CHIP_IS_E1(bp))
> + fw_file_name = FW_FILE_NAME_E1;
> + else if (CHIP_IS_E1H(bp))
> + fw_file_name = FW_FILE_NAME_E1H;
> + else if (!CHIP_IS_E1x(bp))
> + fw_file_name = FW_FILE_NAME_E2;
> + else {
> + BNX2X_ERR("Unsupported chip revision\n");
> + return -EINVAL;
> + }
> + BNX2X_DEV_INFO("Loading %s\n", fw_file_name);
>
> - rc = request_firmware(&bp->firmware, fw_file_name,
> - &bp->pdev->dev);
> - if (rc) {
> - BNX2X_ERR("Can't load firmware file %s\n",
> - fw_file_name);
> - goto request_firmware_exit;
> - }
> + rc = request_firmware(&bp->firmware, fw_file_name, &bp->pdev->dev);
> + if (rc) {
> + BNX2X_ERR("Can't load firmware file %s\n",
> + fw_file_name);
> + goto request_firmware_exit;
> + }
>
> - rc = bnx2x_check_firmware(bp);
> - if (rc) {
> - BNX2X_ERR("Corrupt firmware file %s\n", fw_file_name);
> - goto request_firmware_exit;
> - }
> + rc = bnx2x_check_firmware(bp);
> + if (rc) {
> + BNX2X_ERR("Corrupt firmware file %s\n", fw_file_name);
> + goto request_firmware_exit;
> }
>
> fw_hdr = (struct bnx2x_fw_file_hdr *)bp->firmware->data;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file
2012-03-16 7:24 ` [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file Eilon Greenstein
@ 2012-03-16 8:57 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-03-16 8:57 UTC (permalink / raw)
To: eilong; +Cc: mschmidt, netdev, yuvalmin, yanivr, dmitry
From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Fri, 16 Mar 2012 09:24:09 +0200
> On Fri, 2012-03-16 at 01:08 +0100, Michal Schmidt wrote:
>> If the requested firmware is deemed corrupt and then released, reset the
>> pointer to NULL in order to avoid double-freeing it in
>> bnx2x_release_firmware() or dereferencing it in bnx2x_init_firmware().
>>
>> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
>
> Acked-by: Eilon Greenstein <eilong@broadcom.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] bnx2x: fix memory leak in bnx2x_init_firmware()
2012-03-16 7:24 ` Eilon Greenstein
@ 2012-03-16 8:57 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-03-16 8:57 UTC (permalink / raw)
To: eilong; +Cc: mschmidt, netdev, yuvalmin, yanivr, dmitry
From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Fri, 16 Mar 2012 09:24:34 +0200
> On Fri, 2012-03-16 at 01:08 +0100, Michal Schmidt wrote:
>> When cycling the interface down and up, bnx2x_init_firmware() knows that
>> the firmware is already loaded, but nevertheless it allocates certain
>> arrays anew (init_data, init_ops, init_ops_offsets, iro_arr). The old
>> arrays are leaked.
>>
>> Fix the leaks by returning early if the firmware was already loaded.
>> Because if the firmware is loaded, so are the arrays.
>>
>> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
>
>
> Acked-by: Eilon Greenstein <eilong@broadcom.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-03-16 8:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-16 0:08 [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file Michal Schmidt
2012-03-16 0:08 ` [PATCH net 2/2] bnx2x: fix memory leak in bnx2x_init_firmware() Michal Schmidt
2012-03-16 7:24 ` Eilon Greenstein
2012-03-16 8:57 ` David Miller
2012-03-16 7:24 ` [PATCH net 1/2] bnx2x: fix a crash on corrupt firmware file Eilon Greenstein
2012-03-16 8:57 ` David Miller
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).