[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20210621200623.1690329-1-avlarkin82@gmail.com>
Date: Mon, 21 Jun 2021 23:06:23 +0300
From: Alexander Larkin <avlarkin82@...il.com>
To: torvalds@...ux-foundation.org
Cc: avlarkin82@...il.com, dan.carpenter@...cle.com,
dmitry.torokhov@...il.com, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org, murray.mcallister@...il.com,
security@...nel.org
Subject: Re: [PATCH] Input: joydev - prevent potential write out of bounds in ioctl
I did manual testing with virtual machine (for /dev/input/js0 the driver name is "VirtualBox USB Tablet").
I used program jswap.c from https://bbs.archlinux.org/viewtopic.php?pid=641803#p641803
I compiled both kernel and all modules (including new module with the patch suggested by Linus) and if using updated module:
1. No fail for the poc anymore.
2. Different tests like "./jswap b:0,1 /dev/input/js0" or "./jswap b:4,5 /dev/input/js0"" gives expected result (with seeing the result by cmd "./jswap /dev/input/js0"").
3. Tried to reboot with older kernel (where panic happened) and replayed these few manual tests: results are the same. Both for this older kernel (before fix) for poc.c still seeing fail.
4. For being sure that the kernel module being tested is the same as source code that
I'm compiled, I used command "objdump -S joydev.ko" (after decompress of /lib/../joydev.ko.xz).
I noted one other minor issue when used jswap.
For some reason jswap expected that ioctl JSIOCGBTNMAP returns 0 on success
(because there is separate JSIOCGBUTTONS for getting amount of buttons),
but instead JSIOCGBTNMAP returns number of buttons too (like JSIOCGBUTTONS is).
Is it mistake inside jswap.c and should then keep it as is for joydev.c?
So, for making jswap working, first I had to change return status check of JSIOCGBUTTONS
inside this test jswap.c program (and I don't know if possible to change return status of
JSIOCGBTNMAP to zero on success or of should keep it as is, because many other programs
could be broken if fixing success ret status to zero instead of amount of return values).
Dmitry, what do you think of this?
If no other tests required, maybe let's commit patch that is suggested by Linus.
Dmitry, could you please continue with this?
Thank you.
Powered by blists - more mailing lists