* [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 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 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
* 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 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
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).