[PATCH RESEND] drivers/input: Fix null pointer dereferences in input_ff_create() and input_ff_create_memless()

[PATCH RESEND] drivers/input: Fix null pointer dereferences in input_ff_create() and input_ff_create_memless()

[qasdev]

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@syzkaller.appspotmail.com>
Reported-by: Qasim Ijaz <qasdev00@gmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=dd5f8d6456680e55eb0a
Tested-by: syzbot <syzbot+dd5f8d6456680e55eb0a@syzkaller.appspotmail.com>
Tested-by: Qasim Ijaz <qasdev00@gmail.com>
Fixes: 5203b3a18c1b ("Input: ff-core - make use of __free() cleanup facility")
Signed-off-by: Qasim Ijaz <qasdev00@gmail.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

Re: [PATCH RESEND] drivers/input: Fix null pointer dereferences in input_ff_create() and input_ff_create_memless()

[Dan Carpenter]

On Wed, Jan 08, 2025 at 02:18:12PM +0000, qasdev wrote:
> 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.

The bug isn't present in linux-next.

The original syzbot bug was already fixed in November last year.  It
was fixed in a rebase so it doesn't exist in git history and I really
had to dig to understand what the issue was.

regards,
dan carpenter