linux-kernel - [PATCH RESEND] 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>] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z36JJLAzwsFpggz2@qasdev.system>
Date: Wed, 8 Jan 2025 14:18:12 +0000
From: qasdev <qasdev00@...il.com>
To: dmitry.torokhov@...il.com
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH RESEND] drivers/input: Fix null pointer dereferences in
 input_ff_create() and input_ff_create_memless()

Resending this patch as I have not received feedback since my initial submission on December 25, 2024. 
Please let me know if additional changes or information are required.

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