libertas: move firmware lifetime handling to firmware.c

Previously, each bus type was responsible for freeing the firmware
structure, but some did that badly.  Move responsibility for freeing
firmware into firmware.c so that it's done once and correctly, instead
of happening in multiple places in bus-specific code.

This fixes a use-after-free bug found by Dr. H. Nikolaus Schaller where
the SDIO code forgot to NULL priv->helper_fw after freeing it.

Signed-off-by: Dan Williams <dcbw@redhat.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
This commit is contained in:
Dan Williams 2013-10-14 17:51:55 -05:00 committed by John W. Linville
parent e07f01e4c7
commit 1dfba3060f
5 changed files with 16 additions and 26 deletions

View file

@ -53,6 +53,11 @@ static void main_firmware_cb(const struct firmware *firmware, void *context)
/* Firmware found! */ /* Firmware found! */
lbs_fw_loaded(priv, 0, priv->helper_fw, firmware); lbs_fw_loaded(priv, 0, priv->helper_fw, firmware);
if (priv->helper_fw) {
release_firmware (priv->helper_fw);
priv->helper_fw = NULL;
}
release_firmware (firmware);
} }
static void helper_firmware_cb(const struct firmware *firmware, void *context) static void helper_firmware_cb(const struct firmware *firmware, void *context)

View file

@ -754,14 +754,14 @@ static void if_cs_prog_firmware(struct lbs_private *priv, int ret,
if (ret == 0 && (card->model != MODEL_8305)) if (ret == 0 && (card->model != MODEL_8305))
ret = if_cs_prog_real(card, mainfw); ret = if_cs_prog_real(card, mainfw);
if (ret) if (ret)
goto out; return;
/* Now actually get the IRQ */ /* Now actually get the IRQ */
ret = request_irq(card->p_dev->irq, if_cs_interrupt, ret = request_irq(card->p_dev->irq, if_cs_interrupt,
IRQF_SHARED, DRV_NAME, card); IRQF_SHARED, DRV_NAME, card);
if (ret) { if (ret) {
pr_err("error in request_irq\n"); pr_err("error in request_irq\n");
goto out; return;
} }
/* /*
@ -777,10 +777,6 @@ static void if_cs_prog_firmware(struct lbs_private *priv, int ret,
pr_err("could not activate card\n"); pr_err("could not activate card\n");
free_irq(card->p_dev->irq, card); free_irq(card->p_dev->irq, card);
} }
out:
release_firmware(helper);
release_firmware(mainfw);
} }

View file

@ -708,20 +708,16 @@ static void if_sdio_do_prog_firmware(struct lbs_private *priv, int ret,
ret = if_sdio_prog_helper(card, helper); ret = if_sdio_prog_helper(card, helper);
if (ret) if (ret)
goto out; return;
lbs_deb_sdio("Helper firmware loaded\n"); lbs_deb_sdio("Helper firmware loaded\n");
ret = if_sdio_prog_real(card, mainfw); ret = if_sdio_prog_real(card, mainfw);
if (ret) if (ret)
goto out; return;
lbs_deb_sdio("Firmware loaded\n"); lbs_deb_sdio("Firmware loaded\n");
if_sdio_finish_power_on(card); if_sdio_finish_power_on(card);
out:
release_firmware(helper);
release_firmware(mainfw);
} }
static int if_sdio_prog_firmware(struct if_sdio_card *card) static int if_sdio_prog_firmware(struct if_sdio_card *card)

View file

@ -1094,11 +1094,7 @@ static int if_spi_init_card(struct if_spi_card *card)
goto out; goto out;
out: out:
release_firmware(helper);
release_firmware(mainfw);
lbs_deb_leave_args(LBS_DEB_SPI, "err %d\n", err); lbs_deb_leave_args(LBS_DEB_SPI, "err %d\n", err);
return err; return err;
} }

View file

@ -844,7 +844,7 @@ static void if_usb_prog_firmware(struct lbs_private *priv, int ret,
cardp->fw = fw; cardp->fw = fw;
if (check_fwfile_format(cardp->fw->data, cardp->fw->size)) { if (check_fwfile_format(cardp->fw->data, cardp->fw->size)) {
ret = -EINVAL; ret = -EINVAL;
goto release_fw; goto done;
} }
/* Cancel any pending usb business */ /* Cancel any pending usb business */
@ -861,7 +861,7 @@ restart:
if (if_usb_submit_rx_urb_fwload(cardp) < 0) { if (if_usb_submit_rx_urb_fwload(cardp) < 0) {
lbs_deb_usbd(&cardp->udev->dev, "URB submission is failed\n"); lbs_deb_usbd(&cardp->udev->dev, "URB submission is failed\n");
ret = -EIO; ret = -EIO;
goto release_fw; goto done;
} }
cardp->bootcmdresp = 0; cardp->bootcmdresp = 0;
@ -883,14 +883,14 @@ restart:
usb_kill_urb(cardp->tx_urb); usb_kill_urb(cardp->tx_urb);
if (if_usb_submit_rx_urb(cardp) < 0) if (if_usb_submit_rx_urb(cardp) < 0)
ret = -EIO; ret = -EIO;
goto release_fw; goto done;
} else if (cardp->bootcmdresp <= 0) { } else if (cardp->bootcmdresp <= 0) {
if (--reset_count >= 0) { if (--reset_count >= 0) {
if_usb_reset_device(cardp); if_usb_reset_device(cardp);
goto restart; goto restart;
} }
ret = -EIO; ret = -EIO;
goto release_fw; goto done;
} }
i = 0; i = 0;
@ -921,14 +921,14 @@ restart:
pr_info("FW download failure, time = %d ms\n", i * 100); pr_info("FW download failure, time = %d ms\n", i * 100);
ret = -EIO; ret = -EIO;
goto release_fw; goto done;
} }
cardp->priv->fw_ready = 1; cardp->priv->fw_ready = 1;
if_usb_submit_rx_urb(cardp); if_usb_submit_rx_urb(cardp);
if (lbs_start_card(priv)) if (lbs_start_card(priv))
goto release_fw; goto done;
if_usb_setup_firmware(priv); if_usb_setup_firmware(priv);
@ -939,11 +939,8 @@ restart:
if (lbs_host_sleep_cfg(priv, priv->wol_criteria, NULL)) if (lbs_host_sleep_cfg(priv, priv->wol_criteria, NULL))
priv->ehs_remove_supported = false; priv->ehs_remove_supported = false;
release_fw:
release_firmware(cardp->fw);
cardp->fw = NULL;
done: done:
cardp->fw = NULL;
lbs_deb_leave(LBS_DEB_USB); lbs_deb_leave(LBS_DEB_USB);
} }