Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9c9b24b2 authored by Takashi Iwai's avatar Takashi Iwai Committed by Greg Kroah-Hartman
Browse files

xc2028: Fix use-after-free bug properly 



commit 22a1e7783e173ab3d86018eb590107d68df46c11 upstream.

The commit 8dfbcc4351a0 ("[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: commit 8dfbcc4351a0 ('[media] xc2028: avoid use after free')
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: default avatarAmit Pundir <amit.pundir@linaro.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent dd216cef

drivers/media/tuners/tuner-xc2028.c

+16 −21
Original line number Original line Diff line number Diff line
@@ -281,6 +281,14 @@ static void free_firmware(struct xc2028_data *priv) 
	int i;

	int i;

	tuner_dbg("%s called\n", __func__);

	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)

	if (!priv->firm)

		return;

		return;
@@ -291,9 +299,6 @@ static void free_firmware(struct xc2028_data *priv) 




	priv->firm = NULL;

	priv->firm = NULL;

	priv->firm_size = 0;

	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,

static int load_all_firmwares(struct dvb_frontend *fe,
@@ -884,9 +889,8 @@ read_not_reliable: 
	return 0;

	return 0;





fail:

fail:

	priv->state = XC2028_NO_FIRMWARE;

	free_firmware(priv);





	memset(&priv->cur_fw, 0, sizeof(priv->cur_fw));

	if (retry_count < 8) {

	if (retry_count < 8) {

		msleep(50);

		msleep(50);

		retry_count++;

		retry_count++;
@@ -1332,11 +1336,8 @@ static int xc2028_dvb_release(struct dvb_frontend *fe) 
	mutex_lock(&xc2028_list_mutex);

	mutex_lock(&xc2028_list_mutex);





	/* only perform final cleanup if this is the last instance */

	/* 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);

		free_firmware(priv);

		kfree(priv->ctrl.fname);

		priv->ctrl.fname = NULL;

	}





	if (priv)

	if (priv)

		hybrid_tuner_release_state(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.

	 * 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));

	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

	 * 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 (priv->state == XC2028_NO_FIRMWARE) {

		if (!firmware_name[0])

		if (!firmware_name[0])

			priv->fname = priv->ctrl.fname;

			priv->fname = kstrdup(p->fname, GFP_KERNEL);

		else

		else

			priv->fname = firmware_name;

			priv->fname = firmware_name;





		if (!priv->fname) {

			rc = -ENOMEM;

			goto unlock;

		}



		rc = request_firmware_nowait(THIS_MODULE, 1,

		rc = request_firmware_nowait(THIS_MODULE, 1,

					     priv->fname,

					     priv->fname,

					     priv->i2c_props.adap->dev.parent,

					     priv->i2c_props.adap->dev.parent,