From: Marc Zyngier <maz@kernel.org> Date: 2020-08-17 11:27:14
It recently became apparent that some of the low-level input and hid
helpers lack some form of input validation when associating an event
code with their internal capability bitmap, leading to potential
memory corruption.
These two patches address two occurrences of that issue, by masking
out the top bits of the event code (all capability bitmaps are
conveniently sized as power of twos), and spitting out a warning for
further debugging.
Marc Zyngier (2):
Input; Sanitize event code before modifying bitmaps
HID: core; Sanitize event code and type before mapping input
drivers/input/input.c | 16 +++++++++++++++-
include/linux/hid.h | 19 +++++++++++++++----
2 files changed, 30 insertions(+), 5 deletions(-)
--
2.27.0
From: Marc Zyngier <maz@kernel.org> Date: 2020-08-17 11:27:16
When calling into input_set_capability(), the passed event code is
blindly used to set a bit in a number of bitmaps, without checking
whether this actually fits the expected size of the bitmap.
This event code can come from a variety of sources, including devices
masquerading as input devices, only a bit more "programmable".
Instead of taking the raw event code, sanitize it to the actual bitmap
size and output a warning to let the user know.
These checks are, at least in spirit, in keeping with cb222aed03d7
("Input: add safety guards to input_set_keycode()").
Cc: stable@vger.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/input/input.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
Hi Marc,
On Mon, Aug 17, 2020 at 12:26:59PM +0100, Marc Zyngier wrote:
quoted hunk
When calling into input_set_capability(), the passed event code is
blindly used to set a bit in a number of bitmaps, without checking
whether this actually fits the expected size of the bitmap.
This event code can come from a variety of sources, including devices
masquerading as input devices, only a bit more "programmable".
Instead of taking the raw event code, sanitize it to the actual bitmap
size and output a warning to let the user know.
These checks are, at least in spirit, in keeping with cb222aed03d7
("Input: add safety guards to input_set_keycode()").
Cc: stable@vger.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/input/input.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
I would much rather prefer we did not simply set some random bits in
this case, but instead complained loudly and refused to alter anything.
The function is not exported directly to userspace, so we expect callers
to give us sane inputs, and I believe WARN() splash in case of bad
inputs would be the best solution here.
Thanks.
--
Dmitry
From: Marc Zyngier <maz@kernel.org> Date: 2020-08-26 13:33:16
Hi Dmitry,
On 2020-08-24 20:51, Dmitry Torokhov wrote:
Hi Marc,
On Mon, Aug 17, 2020 at 12:26:59PM +0100, Marc Zyngier wrote:
quoted
When calling into input_set_capability(), the passed event code is
blindly used to set a bit in a number of bitmaps, without checking
whether this actually fits the expected size of the bitmap.
This event code can come from a variety of sources, including devices
masquerading as input devices, only a bit more "programmable".
Instead of taking the raw event code, sanitize it to the actual bitmap
size and output a warning to let the user know.
These checks are, at least in spirit, in keeping with cb222aed03d7
("Input: add safety guards to input_set_keycode()").
Cc: stable@vger.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/input/input.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
capability
* bitmap the function also adjusts dev->evbit.
*/
-void input_set_capability(struct input_dev *dev, unsigned int type,
unsigned int code)
+void input_set_capability(struct input_dev *dev, unsigned int type,
unsigned int raw_code)
{
+ unsigned int code = raw_code;
+
switch (type) {
case EV_KEY:
+ code &= KEY_MAX;
__set_bit(code, dev->keybit);
I would much rather prefer we did not simply set some random bits in
this case, but instead complained loudly and refused to alter anything.
The function is not exported directly to userspace, so we expect
callers
to give us sane inputs, and I believe WARN() splash in case of bad
inputs would be the best solution here.
Fair enough. I've moved the checking to the HID layer (using
hid_map_usage() as the validation primitive), which makes
changing input_set_capability() irrelevant.
I'll post v2 shortly in the form of a single patch.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
From: Marc Zyngier <maz@kernel.org> Date: 2020-08-17 11:27:21
When calling into hid_map_usage(), the passed event code is
blindly stored as is, even if it doesn't fit in the associated bitmap.
This event code can come from a variety of sources, including devices
masquerading as input devices, only a bit more "programmable".
Instead of taking the raw event code, sanitize it to the actual bitmap
size and output a warning to let the user know.
While we're at it, sanitize the hid_usage structure if the type isn't
known, conveniently placing a NULL pointer as the bitmap in order to
catch unexpected uses.
Cc: stable@vger.kernel.org
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
include/linux/hid.h | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)