Skip to contentSkip to conversation

Patch "xc2028: Fix use-after-free bug properly" has been added to the 4.4-stable tree

PATCH [email protected] 1 participant 1 message May 23, 2017

Subsystems (2)
Stable branch Media (V4L/DVB)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

xc2028: Fix use-after-free bug properly

to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
xc2028-fix-use-after-free-bug-properly.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 22a1e7783e173ab3d86018eb590107d68df46c11 Mon Sep 17 00:00:00 2001

From: Takashi Iwai <[email protected]>
Date: Thu, 17 Nov 2016 10:49:31 +0100
Subject: xc2028: Fix use-after-free bug properly

From: Takashi Iwai <[email protected]>

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 Takashi Iwai <[email protected]>
Signed-off-by Mauro Carvalho Chehab <[email protected]>
Signed-off-by Amit Pundir <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/media/tuners/tuner-xc2028.c |   37 +++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)
drivers/media/tuners/tuner-xc2028.c +16 -21
@@ -281,6 +281,14 @@ static void free_firmware(struct xc2028_
 	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_
 
 	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
 	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_
 
 	/*
 	 * 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_
 
 	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,

Patches currently in stable-queue which might be from [email protected] are

queue-4.4/proc-fix-unbalanced-hard-link-numbers.patch queue-4.4/xc2028-fix-use-after-free-bug-properly.patch