* [bug report] HID: wacom: Introduce new 'touch_input' device
@ 2017-04-12 20:31 Dan Carpenter
2017-04-25 18:29 ` [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference Jason Gerecke
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2017-04-12 20:31 UTC (permalink / raw)
To: killertofu; +Cc: linux-input
Hello Jason Gerecke,
This is a semi-automatic email about new static checker warnings.
The patch 2a6cdbdd4cc0: "HID: wacom: Introduce new 'touch_input'
device" from Jun 15, 2015, leads to the following Smatch complaint:
drivers/hid/wacom_wac.c:1586 wacom_tpc_irq()
error: we previously assumed 'wacom->touch_input' could be null (see line 1577)
drivers/hid/wacom_wac.c
1576 "%s: received report #%d\n", __func__, data[0]);
1577 else if (wacom->touch_input)
^^^^^^^^^^^^^^^^^^
Patch adds new check for NULL.
1578 dev_dbg(wacom->touch_input->dev.parent,
1579 "%s: received report #%d\n", __func__, data[0]);
1580
1581 switch (len) {
1582 case WACOM_PKGLEN_TPC1FG:
1583 return wacom_tpc_single_touch(wacom, len);
1584
1585 case WACOM_PKGLEN_TPC2FG:
1586 return wacom_tpc_mt_touch(wacom);
^^^^^
Not checked inside this function call.
1587
1588 case WACOM_PKGLEN_PENABLED:
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference 2017-04-12 20:31 [bug report] HID: wacom: Introduce new 'touch_input' device Dan Carpenter @ 2017-04-25 18:29 ` Jason Gerecke 2017-04-25 20:56 ` Ping Cheng 2017-05-05 12:53 ` Jiri Kosina 0 siblings, 2 replies; 6+ messages in thread From: Jason Gerecke @ 2017-04-25 18:29 UTC (permalink / raw) To: linux-input Cc: Jiri Kosina, Benjamin Tissoires, Ping Cheng, Aaron Skomra, Jason Gerecke, Jason Gerecke The following Smatch complaint was generated in response to commit 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device"): drivers/hid/wacom_wac.c:1586 wacom_tpc_irq() error: we previously assumed 'wacom->touch_input' could be null (see line 1577) The 'touch_input' and 'pen_input' variables point to the 'struct input_dev' used for relaying touch and pen events to userspace, respectively. If a device does not have a touch interface or pen interface, the associated input variable is NULL. The 'wacom_tpc_irq()' function is responsible for forwarding input reports to a more-specific IRQ handler function. An unknown report could theoretically be mistaken as e.g. a touch report on a device which does not have a touch interface. This can be prevented by only calling the pen/touch functions are called when the pen/touch pointers are valid. Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> --- drivers/hid/wacom_wac.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 6b8f6b816195..b963499e3351 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -1571,37 +1571,38 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) { unsigned char *data = wacom->data; - if (wacom->pen_input) + if (wacom->pen_input) { dev_dbg(wacom->pen_input->dev.parent, "%s: received report #%d\n", __func__, data[0]); - else if (wacom->touch_input) + + if (len == WACOM_PKGLEN_PENABLED || + data[0] == WACOM_REPORT_PENABLED) + return wacom_tpc_pen(wacom); + } + else if (wacom->touch_input) { dev_dbg(wacom->touch_input->dev.parent, "%s: received report #%d\n", __func__, data[0]); - switch (len) { - case WACOM_PKGLEN_TPC1FG: - return wacom_tpc_single_touch(wacom, len); + switch (len) { + case WACOM_PKGLEN_TPC1FG: + return wacom_tpc_single_touch(wacom, len); - case WACOM_PKGLEN_TPC2FG: - return wacom_tpc_mt_touch(wacom); + case WACOM_PKGLEN_TPC2FG: + return wacom_tpc_mt_touch(wacom); - case WACOM_PKGLEN_PENABLED: - return wacom_tpc_pen(wacom); + default: + switch (data[0]) { + case WACOM_REPORT_TPC1FG: + case WACOM_REPORT_TPCHID: + case WACOM_REPORT_TPCST: + case WACOM_REPORT_TPC1FGE: + return wacom_tpc_single_touch(wacom, len); - default: - switch (data[0]) { - case WACOM_REPORT_TPC1FG: - case WACOM_REPORT_TPCHID: - case WACOM_REPORT_TPCST: - case WACOM_REPORT_TPC1FGE: - return wacom_tpc_single_touch(wacom, len); - - case WACOM_REPORT_TPCMT: - case WACOM_REPORT_TPCMT2: - return wacom_mt_touch(wacom); + case WACOM_REPORT_TPCMT: + case WACOM_REPORT_TPCMT2: + return wacom_mt_touch(wacom); - case WACOM_REPORT_PENABLED: - return wacom_tpc_pen(wacom); + } } } -- 2.12.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference 2017-04-25 18:29 ` [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference Jason Gerecke @ 2017-04-25 20:56 ` Ping Cheng 2017-05-02 21:04 ` Jason Gerecke 2017-05-05 12:53 ` Jiri Kosina 1 sibling, 1 reply; 6+ messages in thread From: Ping Cheng @ 2017-04-25 20:56 UTC (permalink / raw) To: Jason Gerecke Cc: linux-input, Jiri Kosina, Benjamin Tissoires, Aaron Skomra, Jason Gerecke On Tuesday, April 25, 2017, Jason Gerecke <killertofu@gmail.com> wrote: > > The following Smatch complaint was generated in response to commit > 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device"): > > drivers/hid/wacom_wac.c:1586 wacom_tpc_irq() > error: we previously assumed 'wacom->touch_input' could be null (see line 1577) > > The 'touch_input' and 'pen_input' variables point to the 'struct input_dev' > used for relaying touch and pen events to userspace, respectively. If a > device does not have a touch interface or pen interface, the associated > input variable is NULL. The 'wacom_tpc_irq()' function is responsible for > forwarding input reports to a more-specific IRQ handler function. An > unknown report could theoretically be mistaken as e.g. a touch report > on a device which does not have a touch interface. This can be prevented > by only calling the pen/touch functions are called when the pen/touch > pointers are valid. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> Reviewed-by: Ping Cheng <ping.cheng@wacom.com> Looks good to me. Cheers, Ping > > --- > drivers/hid/wacom_wac.c | 45 +++++++++++++++++++++++---------------------- > 1 file changed, 23 insertions(+), 22 deletions(-) > > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c > index 6b8f6b816195..b963499e3351 100644 > --- a/drivers/hid/wacom_wac.c > +++ b/drivers/hid/wacom_wac.c > @@ -1571,37 +1571,38 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) > { > unsigned char *data = wacom->data; > > - if (wacom->pen_input) > + if (wacom->pen_input) { > dev_dbg(wacom->pen_input->dev.parent, > "%s: received report #%d\n", __func__, data[0]); > - else if (wacom->touch_input) > + > + if (len == WACOM_PKGLEN_PENABLED || > + data[0] == WACOM_REPORT_PENABLED) > + return wacom_tpc_pen(wacom); > + } > + else if (wacom->touch_input) { > dev_dbg(wacom->touch_input->dev.parent, > "%s: received report #%d\n", __func__, data[0]); > > - switch (len) { > - case WACOM_PKGLEN_TPC1FG: > - return wacom_tpc_single_touch(wacom, len); > + switch (len) { > + case WACOM_PKGLEN_TPC1FG: > + return wacom_tpc_single_touch(wacom, len); > > - case WACOM_PKGLEN_TPC2FG: > - return wacom_tpc_mt_touch(wacom); > + case WACOM_PKGLEN_TPC2FG: > + return wacom_tpc_mt_touch(wacom); > > - case WACOM_PKGLEN_PENABLED: > - return wacom_tpc_pen(wacom); > + default: > + switch (data[0]) { > + case WACOM_REPORT_TPC1FG: > + case WACOM_REPORT_TPCHID: > + case WACOM_REPORT_TPCST: > + case WACOM_REPORT_TPC1FGE: > + return wacom_tpc_single_touch(wacom, len); > > - default: > - switch (data[0]) { > - case WACOM_REPORT_TPC1FG: > - case WACOM_REPORT_TPCHID: > - case WACOM_REPORT_TPCST: > - case WACOM_REPORT_TPC1FGE: > - return wacom_tpc_single_touch(wacom, len); > - > - case WACOM_REPORT_TPCMT: > - case WACOM_REPORT_TPCMT2: > - return wacom_mt_touch(wacom); > + case WACOM_REPORT_TPCMT: > + case WACOM_REPORT_TPCMT2: > + return wacom_mt_touch(wacom); > > - case WACOM_REPORT_PENABLED: > - return wacom_tpc_pen(wacom); > + } > } > } > > -- > 2.12.2 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference 2017-04-25 20:56 ` Ping Cheng @ 2017-05-02 21:04 ` Jason Gerecke 2017-05-03 9:26 ` Jiri Kosina 0 siblings, 1 reply; 6+ messages in thread From: Jason Gerecke @ 2017-05-02 21:04 UTC (permalink / raw) To: Ping Cheng Cc: linux-input, Jiri Kosina, Benjamin Tissoires, Aaron Skomra, Jason Gerecke Just making sure this doesn't get lost in the cracks. Jason --- Now instead of four in the eights place / you’ve got three, ‘Cause you added one / (That is to say, eight) to the two, / But you can’t take seven from three, / So you look at the sixty-fours.... On Tue, Apr 25, 2017 at 1:56 PM, Ping Cheng <pinglinux@gmail.com> wrote: > On Tuesday, April 25, 2017, Jason Gerecke <killertofu@gmail.com> wrote: >> >> The following Smatch complaint was generated in response to commit >> 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device"): >> >> drivers/hid/wacom_wac.c:1586 wacom_tpc_irq() >> error: we previously assumed 'wacom->touch_input' could be null (see line 1577) >> >> The 'touch_input' and 'pen_input' variables point to the 'struct input_dev' >> used for relaying touch and pen events to userspace, respectively. If a >> device does not have a touch interface or pen interface, the associated >> input variable is NULL. The 'wacom_tpc_irq()' function is responsible for >> forwarding input reports to a more-specific IRQ handler function. An >> unknown report could theoretically be mistaken as e.g. a touch report >> on a device which does not have a touch interface. This can be prevented >> by only calling the pen/touch functions are called when the pen/touch >> pointers are valid. >> >> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > > Reviewed-by: Ping Cheng <ping.cheng@wacom.com> > > Looks good to me. > > Cheers, > Ping > >> >> --- >> drivers/hid/wacom_wac.c | 45 +++++++++++++++++++++++---------------------- >> 1 file changed, 23 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c >> index 6b8f6b816195..b963499e3351 100644 >> --- a/drivers/hid/wacom_wac.c >> +++ b/drivers/hid/wacom_wac.c >> @@ -1571,37 +1571,38 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) >> { >> unsigned char *data = wacom->data; >> >> - if (wacom->pen_input) >> + if (wacom->pen_input) { >> dev_dbg(wacom->pen_input->dev.parent, >> "%s: received report #%d\n", __func__, data[0]); >> - else if (wacom->touch_input) >> + >> + if (len == WACOM_PKGLEN_PENABLED || >> + data[0] == WACOM_REPORT_PENABLED) >> + return wacom_tpc_pen(wacom); >> + } >> + else if (wacom->touch_input) { >> dev_dbg(wacom->touch_input->dev.parent, >> "%s: received report #%d\n", __func__, data[0]); >> >> - switch (len) { >> - case WACOM_PKGLEN_TPC1FG: >> - return wacom_tpc_single_touch(wacom, len); >> + switch (len) { >> + case WACOM_PKGLEN_TPC1FG: >> + return wacom_tpc_single_touch(wacom, len); >> >> - case WACOM_PKGLEN_TPC2FG: >> - return wacom_tpc_mt_touch(wacom); >> + case WACOM_PKGLEN_TPC2FG: >> + return wacom_tpc_mt_touch(wacom); >> >> - case WACOM_PKGLEN_PENABLED: >> - return wacom_tpc_pen(wacom); >> + default: >> + switch (data[0]) { >> + case WACOM_REPORT_TPC1FG: >> + case WACOM_REPORT_TPCHID: >> + case WACOM_REPORT_TPCST: >> + case WACOM_REPORT_TPC1FGE: >> + return wacom_tpc_single_touch(wacom, len); >> >> - default: >> - switch (data[0]) { >> - case WACOM_REPORT_TPC1FG: >> - case WACOM_REPORT_TPCHID: >> - case WACOM_REPORT_TPCST: >> - case WACOM_REPORT_TPC1FGE: >> - return wacom_tpc_single_touch(wacom, len); >> - >> - case WACOM_REPORT_TPCMT: >> - case WACOM_REPORT_TPCMT2: >> - return wacom_mt_touch(wacom); >> + case WACOM_REPORT_TPCMT: >> + case WACOM_REPORT_TPCMT2: >> + return wacom_mt_touch(wacom); >> >> - case WACOM_REPORT_PENABLED: >> - return wacom_tpc_pen(wacom); >> + } >> } >> } >> >> -- >> 2.12.2 >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference 2017-05-02 21:04 ` Jason Gerecke @ 2017-05-03 9:26 ` Jiri Kosina 0 siblings, 0 replies; 6+ messages in thread From: Jiri Kosina @ 2017-05-03 9:26 UTC (permalink / raw) To: Jason Gerecke Cc: Ping Cheng, linux-input, Benjamin Tissoires, Aaron Skomra, Jason Gerecke On Tue, 2 May 2017, Jason Gerecke wrote: > Just making sure this doesn't get lost in the cracks. I will definitely be picking it up for 4.12-rc. I believe you wanted to add Fixes: tag and -stable inclusion anotation as well. Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference 2017-04-25 18:29 ` [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference Jason Gerecke 2017-04-25 20:56 ` Ping Cheng @ 2017-05-05 12:53 ` Jiri Kosina 1 sibling, 0 replies; 6+ messages in thread From: Jiri Kosina @ 2017-05-05 12:53 UTC (permalink / raw) To: Jason Gerecke Cc: linux-input, Benjamin Tissoires, Ping Cheng, Aaron Skomra, Jason Gerecke On Tue, 25 Apr 2017, Jason Gerecke wrote: > The following Smatch complaint was generated in response to commit > 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device"): > > drivers/hid/wacom_wac.c:1586 wacom_tpc_irq() > error: we previously assumed 'wacom->touch_input' could be null (see line 1577) > > The 'touch_input' and 'pen_input' variables point to the 'struct input_dev' > used for relaying touch and pen events to userspace, respectively. If a > device does not have a touch interface or pen interface, the associated > input variable is NULL. The 'wacom_tpc_irq()' function is responsible for > forwarding input reports to a more-specific IRQ handler function. An > unknown report could theoretically be mistaken as e.g. a touch report > on a device which does not have a touch interface. This can be prevented > by only calling the pen/touch functions are called when the pen/touch > pointers are valid. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> Applied to for-4.12/upstream-fixes branch with these tags: Fixes: 2a6cdbd ("HID: wacom: Introduce new 'touch_input' device") Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> Reviewed-by: Ping Cheng <ping.cheng@wacom.com> Cc: stable@vger.kernel.org -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-05 12:53 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-12 20:31 [bug report] HID: wacom: Introduce new 'touch_input' device Dan Carpenter 2017-04-25 18:29 ` [PATCH] HID: wacom: Have wacom_tpc_irq guard against possible NULL dereference Jason Gerecke 2017-04-25 20:56 ` Ping Cheng 2017-05-02 21:04 ` Jason Gerecke 2017-05-03 9:26 ` Jiri Kosina 2017-05-05 12:53 ` Jiri Kosina
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.