CVE-2020-25669: Use After Free (CWE-416)
value: A vulnerability was found in the Linux Kernel where the function sunkbd_reinit having been scheduled by sunkbd_interrupt before sunkbd being freed. Though the dangling pointer is set to NULL in sunkbd_disconnect, there is still an alias in sunkbd_reinit causing Use After Free.
CWE description: The product reuses or references memory after it has been freed. At some point afterward, the memory may be allocated again and saved in another pointer, while the original pointer references a location somewhere within the new allocation. Any operations using the original pointer are no longer valid because the memory "belongs" to the code that operates on the new pointer.
CVSS v3 base score: 7.8
Published: 2021-05-26T12:15Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: drivers/input/keyboard/sunkbd.c
Commit date: 2020-11-08 21:59:07 -0800
Commit message: Input: sunkbd - avoid use-after-free in teardown paths

We need to make sure we cancel the reinit work before we tear down the
driver structures.

Reported-by: Bodong Zhao <nopitydays@gmail.com>
Tested-by: Bodong Zhao <nopitydays@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Diff: diff --git a/drivers/input/keyboard/sunkbd.c b/drivers/input/keyboard/sunkbd.c
index 27126e621eb6..d450f11b98a7 100644
--- a/drivers/input/keyboard/sunkbd.c
+++ b/drivers/input/keyboard/sunkbd.c
@@ -97,11 +97,12 @@ static irqreturn_t sunkbd_interrupt(struct serio *serio,
 	}
 
 	switch (data) {
 
 	case SUNKBD_RET_RESET:
-		schedule_work(&sunkbd->tq);
+		if (sunkbd->enabled)
+			schedule_work(&sunkbd->tq);
 		sunkbd->reset = -1;
 		break;
 
 	case SUNKBD_RET_LAYOUT:
 		sunkbd->layout = -1;
@@ -198,20 +199,16 @@ static int sunkbd_initialize(struct sunkbd *sunkbd)
 
 	return 0;
 }
 
 /*
- * sunkbd_reinit() sets leds and beeps to a state the computer remembers they
- * were in.
+ * sunkbd_set_leds_beeps() sets leds and beeps to a state the computer remembers
+ * they were in.
  */
 
-static void sunkbd_reinit(struct work_struct *work)
+static void sunkbd_set_leds_beeps(struct sunkbd *sunkbd)
 {
-	struct sunkbd *sunkbd = container_of(work, struct sunkbd, tq);
-
-	wait_event_interruptible_timeout(sunkbd->wait, sunkbd->reset >= 0, HZ);
-
 	serio_write(sunkbd->serio, SUNKBD_CMD_SETLED);
 	serio_write(sunkbd->serio,
 		(!!test_bit(LED_CAPSL,   sunkbd->dev->led) << 3) |
 		(!!test_bit(LED_SCROLLL, sunkbd->dev->led) << 2) |
 		(!!test_bit(LED_COMPOSE, sunkbd->dev->led) << 1) |
@@ -220,15 +217,43 @@ static void sunkbd_reinit(struct work_struct *work)
 		SUNKBD_CMD_NOCLICK - !!test_bit(SND_CLICK, sunkbd->dev->snd));
 	serio_write(sunkbd->serio,
 		SUNKBD_CMD_BELLOFF - !!test_bit(SND_BELL, sunkbd->dev->snd));
 }
 
+
+/*
+ * sunkbd_reinit() wait for the keyboard reset to complete and restores state
+ * of leds and beeps.
+ */
+
+static void sunkbd_reinit(struct work_struct *work)
+{
+	struct sunkbd *sunkbd = container_of(work, struct sunkbd, tq);
+
+	/*
+	 * It is OK that we check sunkbd->enabled without pausing serio,
+	 * as we only want to catch true->false transition that will
+	 * happen once and we will be woken up for it.
+	 */
+	wait_event_interruptible_timeout(sunkbd->wait,
+					 sunkbd->reset >= 0 || !sunkbd->enabled,
+					 HZ);
+
+	if (sunkbd->reset >= 0 && sunkbd->enabled)
+		sunkbd_set_leds_beeps(sunkbd);
+}
+
 static void sunkbd_enable(struct sunkbd *sunkbd, bool enable)
 {
 	serio_pause_rx(sunkbd->serio);
 	sunkbd->enabled = enable;
 	serio_continue_rx(sunkbd->serio);
+
+	if (!enable) {
+		wake_up_interruptible(&sunkbd->wait);
+		cancel_work_sync(&sunkbd->tq);
+	}
 }
 
 /*
  * sunkbd_connect() probes for a Sun keyboard and fills the necessary
  * structures.

Vulnerable code: 		schedule_work(&sunkbd->tq);
 * sunkbd_reinit() sets leds and beeps to a state the computer remembers they
 * were in.
static void sunkbd_reinit(struct work_struct *work)
	struct sunkbd *sunkbd = container_of(work, struct sunkbd, tq);

	wait_event_interruptible_timeout(sunkbd->wait, sunkbd->reset >= 0, HZ);

Fixed code: 		if (sunkbd->enabled)
			schedule_work(&sunkbd->tq);
 * sunkbd_set_leds_beeps() sets leds and beeps to a state the computer remembers
 * they were in.
static void sunkbd_set_leds_beeps(struct sunkbd *sunkbd)

/*
 * sunkbd_reinit() wait for the keyboard reset to complete and restores state
 * of leds and beeps.
 */

static void sunkbd_reinit(struct work_struct *work)
{
	struct sunkbd *sunkbd = container_of(work, struct sunkbd, tq);

	/*
	 * It is OK that we check sunkbd->enabled without pausing serio,
	 * as we only want to catch true->false transition that will
	 * happen once and we will be woken up for it.
	 */
	wait_event_interruptible_timeout(sunkbd->wait,
					 sunkbd->reset >= 0 || !sunkbd->enabled,
					 HZ);

	if (sunkbd->reset >= 0 && sunkbd->enabled)
		sunkbd_set_leds_beeps(sunkbd);
}


	if (!enable) {
		wake_up_interruptible(&sunkbd->wait);
		cancel_work_sync(&sunkbd->tq);
	}