xc2028: Fix use-after-free bug properly
The commit8dfbcc4351
("[media] xc2028: avoid use after free") tried to address the reported use-after-free by clearing the reference. However, it's clearing the wrong pointer; it sets NULL to priv->ctrl.fname, but it's anyway overwritten by the next line memcpy(&priv->ctrl, p, sizeof(priv->ctrl)). OTOH, the actual code accessing the freed string is the strcmp() call with priv->fname: if (!firmware_name[0] && p->fname && priv->fname && strcmp(p->fname, priv->fname)) free_firmware(priv); where priv->fname points to the previous file name, and this was already freed by kfree(). For fixing the bug properly, this patch does the following: - Keep the copy of firmware file name in only priv->fname, priv->ctrl.fname isn't changed; - The allocation is done only when the firmware gets loaded; - The kfree() is called in free_firmware() commonly Fixes: commit8dfbcc4351
('[media] xc2028: avoid use after free') Cc: <stable@vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
This commit is contained in:
parent
9c763584b7
commit
22a1e7783e
|
@ -281,6 +281,14 @@ static void free_firmware(struct xc2028_data *priv)
|
|||
int i;
|
||||
tuner_dbg("%s called\n", __func__);
|
||||
|
||||
/* free allocated f/w string */
|
||||
if (priv->fname != firmware_name)
|
||||
kfree(priv->fname);
|
||||
priv->fname = NULL;
|
||||
|
||||
priv->state = XC2028_NO_FIRMWARE;
|
||||
memset(&priv->cur_fw, 0, sizeof(priv->cur_fw));
|
||||
|
||||
if (!priv->firm)
|
||||
return;
|
||||
|
||||
|
@ -291,9 +299,6 @@ static void free_firmware(struct xc2028_data *priv)
|
|||
|
||||
priv->firm = NULL;
|
||||
priv->firm_size = 0;
|
||||
priv->state = XC2028_NO_FIRMWARE;
|
||||
|
||||
memset(&priv->cur_fw, 0, sizeof(priv->cur_fw));
|
||||
}
|
||||
|
||||
static int load_all_firmwares(struct dvb_frontend *fe,
|
||||
|
@ -884,9 +889,8 @@ read_not_reliable:
|
|||
return 0;
|
||||
|
||||
fail:
|
||||
priv->state = XC2028_NO_FIRMWARE;
|
||||
free_firmware(priv);
|
||||
|
||||
memset(&priv->cur_fw, 0, sizeof(priv->cur_fw));
|
||||
if (retry_count < 8) {
|
||||
msleep(50);
|
||||
retry_count++;
|
||||
|
@ -1332,11 +1336,8 @@ static int xc2028_dvb_release(struct dvb_frontend *fe)
|
|||
mutex_lock(&xc2028_list_mutex);
|
||||
|
||||
/* only perform final cleanup if this is the last instance */
|
||||
if (hybrid_tuner_report_instance_count(priv) == 1) {
|
||||
if (hybrid_tuner_report_instance_count(priv) == 1)
|
||||
free_firmware(priv);
|
||||
kfree(priv->ctrl.fname);
|
||||
priv->ctrl.fname = NULL;
|
||||
}
|
||||
|
||||
if (priv)
|
||||
hybrid_tuner_release_state(priv);
|
||||
|
@ -1399,19 +1400,8 @@ static int xc2028_set_config(struct dvb_frontend *fe, void *priv_cfg)
|
|||
|
||||
/*
|
||||
* Copy the config data.
|
||||
* For the firmware name, keep a local copy of the string,
|
||||
* in order to avoid troubles during device release.
|
||||
*/
|
||||
kfree(priv->ctrl.fname);
|
||||
priv->ctrl.fname = NULL;
|
||||
memcpy(&priv->ctrl, p, sizeof(priv->ctrl));
|
||||
if (p->fname) {
|
||||
priv->ctrl.fname = kstrdup(p->fname, GFP_KERNEL);
|
||||
if (priv->ctrl.fname == NULL) {
|
||||
rc = -ENOMEM;
|
||||
goto unlock;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* If firmware name changed, frees firmware. As free_firmware will
|
||||
|
@ -1426,10 +1416,15 @@ static int xc2028_set_config(struct dvb_frontend *fe, void *priv_cfg)
|
|||
|
||||
if (priv->state == XC2028_NO_FIRMWARE) {
|
||||
if (!firmware_name[0])
|
||||
priv->fname = priv->ctrl.fname;
|
||||
priv->fname = kstrdup(p->fname, GFP_KERNEL);
|
||||
else
|
||||
priv->fname = firmware_name;
|
||||
|
||||
if (!priv->fname) {
|
||||
rc = -ENOMEM;
|
||||
goto unlock;
|
||||
}
|
||||
|
||||
rc = request_firmware_nowait(THIS_MODULE, 1,
|
||||
priv->fname,
|
||||
priv->i2c_props.adap->dev.parent,
|
||||
|
|
Loading…
Reference in New Issue