linux-kernel - [PATCH] drivers/input: Fix null pointer dereferences in input_ff_create() and input_ff_create

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <Z2ycdA6j6oMvj7Td@qasdev.system>
Date: Wed, 25 Dec 2024 23:59:48 +0000
From: qasdev <qasdev00@...il.com>
To: dmitry.torokhov@...il.com
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] drivers/input: Fix null pointer dereferences in
 input_ff_create() and input_ff_create_memless()

This patch addresses two null pointer dereference bugs detected by KASAN 
in input_ff_create() (drivers/input/ff-core.c) and 
input_ff_create_memless() (drivers/input/ff-memless.c).

- input_ff_create() log: https://syzkaller.appspot.com/bug?extid=dd5f8d6456680e55eb0a
- input_ff_create_memless() log: https://syzkaller.appspot.com/x/report.txt?x=10a51adf980000

The root cause of the null pointer dereference in input_ff_create() is 
that it utilises the "no_free_ptr" macro in the line: 
"dev->ff = no_free_ptr(ff);"
which effectively sets "ff" to NULL and makes "dev->ff" point to what "ff" 
was initially pointing to. The code then attempts to incorrectly utilise
"ff" after it has been set to NULL causing a null pointer dereference to 
occur in the following:

"
   for_each_set_bit(i, dev->ffbit, FF_CNT)
       __set_bit(i, ff->ffbit);
   
   /* we can emulate RUMBLE with periodic effects */
   if (test_bit(FF_PERIODIC, ff->ffbit))
       __set_bit(FF_RUMBLE, dev->ffbit);
"

To fix this I changed all accesses to "ff" to be "dev->ff".

The root cause of the null pointer dereference in 
input_ff_create_memless() is that it also utilises the "no_free_ptr" macro 
in the line: "ff->private = no_free_ptr(ml);"
which sets "ml" to NULL and makes "ff->private" point to what "ml" was 
initially pointing to. The code then attempts to utilise "ml" after it has 
been set to NULL:

"
   for (i = 0; i < FF_MEMLESS_EFFECTS; i++)
        ml->states[i].effect = &ff->effects[i];
"

To fix this bug I moved the for loop before "ff->private = no_free_ptr(ml);".

Reported-by: syzbot <syzbot+dd5f8d6456680e55eb0a@...kaller.appspotmail.com>
Reported-by: Qasim Ijaz <qasdev00@...il.com>
Closes: https://syzkaller.appspot.com/bug?extid=dd5f8d6456680e55eb0a
Tested-by: syzbot <syzbot+dd5f8d6456680e55eb0a@...kaller.appspotmail.com>
Tested-by: Qasim Ijaz <qasdev00@...il.com>
Fixes: 5203b3a18c1b ("Input: ff-core - make use of __free() cleanup facility")
Signed-off-by: Qasim Ijaz <qasdev00@...il.com>
---
 drivers/input/ff-core.c    | 4 ++--
 drivers/input/ff-memless.c | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c
index a235d2eb6b31..d9995f47efdb 100644
--- a/drivers/input/ff-core.c
+++ b/drivers/input/ff-core.c
@@ -322,10 +322,10 @@ int input_ff_create(struct input_dev *dev, unsigned int max_effects)

 	/* Copy "true" bits into ff device bitmap */
 	for_each_set_bit(i, dev->ffbit, FF_CNT)
-		__set_bit(i, ff->ffbit);
+		__set_bit(i, dev->ff->ffbit);

 	/* we can emulate RUMBLE with periodic effects */
-	if (test_bit(FF_PERIODIC, ff->ffbit))
+	if (test_bit(FF_PERIODIC, dev->ff->ffbit))
 		__set_bit(FF_RUMBLE, dev->ffbit);

 	return 0;
diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index 0bbeceb35545..ce9fb88486ab 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -524,6 +524,10 @@ int input_ff_create_memless(struct input_dev *dev, void *data,
 		return error;

 	ff = dev->ff;
+
+	for (i = 0; i < FF_MEMLESS_EFFECTS; i++)
+		ml->states[i].effect = &ff->effects[i];
+
 	ff->private = no_free_ptr(ml);
 	ff->upload = ml_ff_upload;
 	ff->playback = ml_ff_playback;
@@ -538,9 +542,6 @@ int input_ff_create_memless(struct input_dev *dev, void *data,
 		set_bit(FF_SQUARE, dev->ffbit);
 	}

-	for (i = 0; i < FF_MEMLESS_EFFECTS; i++)
-		ml->states[i].effect = &ff->effects[i];
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(input_ff_create_memless);
--
2.39.5