* [PATCH 0/2] input/hid: Fix bitmap boundary validation @ 2020-08-17 11:26 Marc Zyngier 2020-08-17 11:26 ` [PATCH 1/2] Input; Sanitize event code before modifying bitmaps Marc Zyngier 2020-08-17 11:27 ` [PATCH 2/2] HID: core; Sanitize event code and type before mapping input Marc Zyngier 0 siblings, 2 replies; 5+ messages in thread From: Marc Zyngier @ 2020-08-17 11:26 UTC (permalink / raw) To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires Cc: linux-input, linux-kernel, stable 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] Input; Sanitize event code before modifying bitmaps 2020-08-17 11:26 [PATCH 0/2] input/hid: Fix bitmap boundary validation Marc Zyngier @ 2020-08-17 11:26 ` Marc Zyngier 2020-08-24 19:51 ` Dmitry Torokhov 2020-08-17 11:27 ` [PATCH 2/2] HID: core; Sanitize event code and type before mapping input Marc Zyngier 1 sibling, 1 reply; 5+ messages in thread From: Marc Zyngier @ 2020-08-17 11:26 UTC (permalink / raw) To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires Cc: linux-input, linux-kernel, stable 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(-) diff --git a/drivers/input/input.c b/drivers/input/input.c index 3cfd2c18eebd..1e77cf47aa44 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -1974,14 +1974,18 @@ EXPORT_SYMBOL(input_get_timestamp); * In addition to setting up corresponding bit in appropriate 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); break; case EV_REL: + code &= REL_MAX; __set_bit(code, dev->relbit); break; @@ -1990,26 +1994,32 @@ void input_set_capability(struct input_dev *dev, unsigned int type, unsigned int if (!dev->absinfo) return; + code &= ABS_MAX; __set_bit(code, dev->absbit); break; case EV_MSC: + code &= MSC_MAX; __set_bit(code, dev->mscbit); break; case EV_SW: + code &= SW_MAX; __set_bit(code, dev->swbit); break; case EV_LED: + code &= LED_MAX; __set_bit(code, dev->ledbit); break; case EV_SND: + code &= SND_MAX; __set_bit(code, dev->sndbit); break; case EV_FF: + code &= FF_MAX; __set_bit(code, dev->ffbit); break; @@ -2023,6 +2033,10 @@ void input_set_capability(struct input_dev *dev, unsigned int type, unsigned int return; } + if (unlikely(code != raw_code)) + pr_warn_ratelimited("%s: Truncated code %d to %d for type %d\n", + dev->name, raw_code, code, type); + __set_bit(type, dev->evbit); } EXPORT_SYMBOL(input_set_capability); -- 2.27.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Input; Sanitize event code before modifying bitmaps 2020-08-17 11:26 ` [PATCH 1/2] Input; Sanitize event code before modifying bitmaps Marc Zyngier @ 2020-08-24 19:51 ` Dmitry Torokhov 2020-08-26 13:32 ` Marc Zyngier 0 siblings, 1 reply; 5+ messages in thread From: Dmitry Torokhov @ 2020-08-24 19:51 UTC (permalink / raw) To: Marc Zyngier Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel, stable Hi Marc, On Mon, Aug 17, 2020 at 12:26:59PM +0100, Marc Zyngier wrote: > 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(-) > > diff --git a/drivers/input/input.c b/drivers/input/input.c > index 3cfd2c18eebd..1e77cf47aa44 100644 > --- a/drivers/input/input.c > +++ b/drivers/input/input.c > @@ -1974,14 +1974,18 @@ EXPORT_SYMBOL(input_get_timestamp); > * In addition to setting up corresponding bit in appropriate 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. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Input; Sanitize event code before modifying bitmaps 2020-08-24 19:51 ` Dmitry Torokhov @ 2020-08-26 13:32 ` Marc Zyngier 0 siblings, 0 replies; 5+ messages in thread From: Marc Zyngier @ 2020-08-26 13:32 UTC (permalink / raw) To: Dmitry Torokhov Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel, stable 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: >> 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(-) >> >> diff --git a/drivers/input/input.c b/drivers/input/input.c >> index 3cfd2c18eebd..1e77cf47aa44 100644 >> --- a/drivers/input/input.c >> +++ b/drivers/input/input.c >> @@ -1974,14 +1974,18 @@ EXPORT_SYMBOL(input_get_timestamp); >> * In addition to setting up corresponding bit in appropriate >> 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... ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] HID: core; Sanitize event code and type before mapping input 2020-08-17 11:26 [PATCH 0/2] input/hid: Fix bitmap boundary validation Marc Zyngier 2020-08-17 11:26 ` [PATCH 1/2] Input; Sanitize event code before modifying bitmaps Marc Zyngier @ 2020-08-17 11:27 ` Marc Zyngier 1 sibling, 0 replies; 5+ messages in thread From: Marc Zyngier @ 2020-08-17 11:27 UTC (permalink / raw) To: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires Cc: linux-input, linux-kernel, stable 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(-) diff --git a/include/linux/hid.h b/include/linux/hid.h index 875f71132b14..4cd87d0ec023 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -966,9 +966,6 @@ static inline void hid_map_usage(struct hid_input *hidinput, { struct input_dev *input = hidinput->input; - usage->type = type; - usage->code = c; - switch (type) { case EV_ABS: *bit = input->absbit; @@ -986,7 +983,20 @@ static inline void hid_map_usage(struct hid_input *hidinput, *bit = input->ledbit; *max = LED_MAX; break; + default: + *bit = NULL; + *max = 0; + usage->code = 0; + usage->type = 0; + return; } + + usage->type = type; + usage->code = c & *max; + + if (unlikely(usage->code != c)) + pr_warn_ratelimited("%s: Truncated code %d to %d for type %d\n", + input->name, c, usage->code, type); } /** @@ -1000,7 +1010,8 @@ static inline void hid_map_usage_clear(struct hid_input *hidinput, __u8 type, __u16 c) { hid_map_usage(hidinput, usage, bit, max, type, c); - clear_bit(c, *bit); + if (*bit) + clear_bit(usage->code, *bit); } /** -- 2.27.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-26 13:33 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-17 11:26 [PATCH 0/2] input/hid: Fix bitmap boundary validation Marc Zyngier 2020-08-17 11:26 ` [PATCH 1/2] Input; Sanitize event code before modifying bitmaps Marc Zyngier 2020-08-24 19:51 ` Dmitry Torokhov 2020-08-26 13:32 ` Marc Zyngier 2020-08-17 11:27 ` [PATCH 2/2] HID: core; Sanitize event code and type before mapping input Marc Zyngier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.