diff mbox series

HID: wacom: check input_dev->absinfo in wacom_bpt3_touch_msg

Message ID 20210517093403.74276-1-yguoaz@cse.ust.hk
State New
Headers show
Series HID: wacom: check input_dev->absinfo in wacom_bpt3_touch_msg | expand

Commit Message

Yiyuan Guo May 17, 2021, 9:34 a.m. UTC
The function wacom_bpt3_touch_msg calls input_abs_get_res(input,
ABS_MT_POSITION_X) to obtain x_res, which may equal to 0 if
input->absinfo is NULL. Since x_res is used as a divisor, this
may lead to divide by zero problem.

Signed-off-by: Yiyuan GUO <yguoaz@cse.ust.hk>
---
 drivers/hid/wacom_wac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jiri Kosina May 26, 2021, 10:50 a.m. UTC | #1
On Mon, 17 May 2021, Yiyuan GUO wrote:

> The function wacom_bpt3_touch_msg calls input_abs_get_res(input,

> ABS_MT_POSITION_X) to obtain x_res, which may equal to 0 if

> input->absinfo is NULL. Since x_res is used as a divisor, this

> may lead to divide by zero problem.

> 

> Signed-off-by: Yiyuan GUO <yguoaz@cse.ust.hk>

> ---

>  drivers/hid/wacom_wac.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c

> index 81d7d12bc..a5a6fb8bc 100644

> --- a/drivers/hid/wacom_wac.c

> +++ b/drivers/hid/wacom_wac.c

> @@ -2892,7 +2892,7 @@ static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)

>  	bool touch = data[1] & 0x80;

>  	int slot = input_mt_get_slot_by_key(input, data[0]);

>  

> -	if (slot < 0)

> +	if (slot < 0 || !input->absinfo)

>  		return;

>  

>  	touch = touch && report_touch_events(wacom);


CCing Wacom driver maintainers in order to get their ack.

-- 
Jiri Kosina
SUSE Labs
Gerecke, Jason May 28, 2021, 2:19 p.m. UTC | #2
From: Jiri Kosina <jikos@kernel.org>
>  
> On Mon, 17 May 2021, Yiyuan GUO wrote:
> 
> > The function wacom_bpt3_touch_msg calls input_abs_get_res(input,
> > ABS_MT_POSITION_X) to obtain x_res, which may equal to 0 if
> > input->absinfo is NULL. Since x_res is used as a divisor, this
> > may lead to divide by zero problem.
> >
> > Signed-off-by: Yiyuan GUO <yguoaz@cse.ust.hk>
> > ---
> >  drivers/hid/wacom_wac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > index 81d7d12bc..a5a6fb8bc 100644
> > --- a/drivers/hid/wacom_wac.c
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -2892,7 +2892,7 @@ static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
> >       bool touch = data[1] & 0x80;
> >       int slot = input_mt_get_slot_by_key(input, data[0]);
> >
> > -     if (slot < 0)
> > +     if (slot < 0 || !input->absinfo)
> >               return;
> >
> >       touch = touch && report_touch_events(wacom);
> 
> CCing Wacom driver maintainers in order to get their ack.
> 
> --
> Jiri Kosina
> SUSE Labs

A NULL input->absinfo is very much an unexpected condition. We've either failed somewhere during setup or things have gone off the rails afterwards. Silently limping along like this is a bad idea. I'd really like to see an error message logged and the device removed if possible.

Jason Gerecke
Dmitry Torokhov June 3, 2021, 1:33 a.m. UTC | #3
On Fri, May 28, 2021 at 02:19:37PM +0000, Gerecke, Jason wrote:
> From: Jiri Kosina <jikos@kernel.org>

> >  

> > On Mon, 17 May 2021, Yiyuan GUO wrote:

> > 

> > > The function wacom_bpt3_touch_msg calls input_abs_get_res(input,

> > > ABS_MT_POSITION_X) to obtain x_res, which may equal to 0 if

> > > input->absinfo is NULL. Since x_res is used as a divisor, this

> > > may lead to divide by zero problem.

> > >

> > > Signed-off-by: Yiyuan GUO <yguoaz@cse.ust.hk>

> > > ---

> > >  drivers/hid/wacom_wac.c | 2 +-

> > >  1 file changed, 1 insertion(+), 1 deletion(-)

> > >

> > > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c

> > > index 81d7d12bc..a5a6fb8bc 100644

> > > --- a/drivers/hid/wacom_wac.c

> > > +++ b/drivers/hid/wacom_wac.c

> > > @@ -2892,7 +2892,7 @@ static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)

> > >       bool touch = data[1] & 0x80;

> > >       int slot = input_mt_get_slot_by_key(input, data[0]);

> > >

> > > -     if (slot < 0)

> > > +     if (slot < 0 || !input->absinfo)

> > >               return;

> > >

> > >       touch = touch && report_touch_events(wacom);

> > 

> > CCing Wacom driver maintainers in order to get their ack.

> > 

> > --

> > Jiri Kosina

> > SUSE Labs

> 

> A NULL input->absinfo is very much an unexpected condition. We've

> either failed somewhere during setup or things have gone off the rails

> afterwards. Silently limping along like this is a bad idea. I'd really

> like to see an error message logged and the device removed if

> possible.


Input core (input_register_device) will refuse registering an input
device claiming to be absolute (EV_ABS present in dev->absbit) but not
having dev->absinfo allocated, so this is not going to happen in real
life.

Thanks.

-- 
Dmitry
diff mbox series

Patch

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 81d7d12bc..a5a6fb8bc 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2892,7 +2892,7 @@  static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
 	bool touch = data[1] & 0x80;
 	int slot = input_mt_get_slot_by_key(input, data[0]);
 
-	if (slot < 0)
+	if (slot < 0 || !input->absinfo)
 		return;
 
 	touch = touch && report_touch_events(wacom);