| Message ID | 20190308051117.21899-1-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 94a9992f7dbdfb28976b565af220e0c4a117144a |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: Increase maximum report size allowed by hid_field_extract() | expand |
On Fri, 8 Mar 2019, Kai-Heng Feng wrote: > Commit 71f6fa90a353 ("HID: increase maximum global item tag report size > to 256") increases the max report size from 128 to 256. > > We also need to update the report size in hid_field_extract() otherwise > it complains and truncates now valid report size: > [ 406.165461] hid-sensor-hub 001F:8086:22D8.0002: hid_field_extract() called with n (192) > 32! (kworker/5:1) > > BugLink: https://bugs.launchpad.net/bugs/1818547 > Fixes: 71f6fa90a353 ("HID: increase maximum global item tag report size to 256") > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Applied, thanks.
Hi, On Fri, Mar 8, 2019 at 6:11 AM Kai-Heng Feng <kai.heng.feng@canonical.com> wrote: > > Commit 71f6fa90a353 ("HID: increase maximum global item tag report size > to 256") increases the max report size from 128 to 256. > > We also need to update the report size in hid_field_extract() otherwise > it complains and truncates now valid report size: > [ 406.165461] hid-sensor-hub 001F:8086:22D8.0002: hid_field_extract() called with n (192) > 32! (kworker/5:1) > > BugLink: https://bugs.launchpad.net/bugs/1818547 > Fixes: 71f6fa90a353 ("HID: increase maximum global item tag report size to 256") > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/hid/hid-core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 9993b692598f..860e21ec6a49 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1301,10 +1301,10 @@ static u32 __extract(u8 *report, unsigned offset, int n) > u32 hid_field_extract(const struct hid_device *hid, u8 *report, > unsigned offset, unsigned n) Ronald (Cc-ed) raised quite a good point: what's the benefit of removing the error message if this function (and __extract) can only report an unsigned 32 bits value? My take is we should revert 94a9992f7dbdfb28976b upstream and think at a better solution. Cheers, Benjamin > { > - if (n > 32) { > - hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n", > + if (n > 256) { > + hid_warn(hid, "hid_field_extract() called with n (%d) > 256! (%s)\n", > n, current->comm); > - n = 32; > + n = 256; > } > > return __extract(report, offset, n); > -- > 2.17.1 >
at 17:42, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > Hi, > > > On Fri, Mar 8, 2019 at 6:11 AM Kai-Heng Feng > <kai.heng.feng@canonical.com> wrote: >> Commit 71f6fa90a353 ("HID: increase maximum global item tag report size >> to 256") increases the max report size from 128 to 256. >> >> We also need to update the report size in hid_field_extract() otherwise >> it complains and truncates now valid report size: >> [ 406.165461] hid-sensor-hub 001F:8086:22D8.0002: hid_field_extract() >> called with n (192) > 32! (kworker/5:1) >> >> BugLink: https://bugs.launchpad.net/bugs/1818547 >> Fixes: 71f6fa90a353 ("HID: increase maximum global item tag report size >> to 256") >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >> --- >> drivers/hid/hid-core.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c >> index 9993b692598f..860e21ec6a49 100644 >> --- a/drivers/hid/hid-core.c >> +++ b/drivers/hid/hid-core.c >> @@ -1301,10 +1301,10 @@ static u32 __extract(u8 *report, unsigned >> offset, int n) >> u32 hid_field_extract(const struct hid_device *hid, u8 *report, >> unsigned offset, unsigned n) > > Ronald (Cc-ed) raised quite a good point: > what's the benefit of removing the error message if this function (and > __extract) can only report an unsigned 32 bits value? I didn’t spot this, sorry. > > My take is we should revert 94a9992f7dbdfb28976b upstream and think at > a better solution. I think using a new fix to replace it will be a better approach, as it at least partially solves the issue. Kai-Heng > > Cheers, > Benjamin > >> { >> - if (n > 32) { >> - hid_warn(hid, "hid_field_extract() called with n (%d) > >> 32! (%s)\n", >> + if (n > 256) { >> + hid_warn(hid, "hid_field_extract() called with n (%d) > >> 256! (%s)\n", >> n, current->comm); >> - n = 32; >> + n = 256; >> } >> >> return __extract(report, offset, n); >> — >> 2.17.1
On Fri, 26 Apr 2019, Kai-Heng Feng wrote: > >Ronald (Cc-ed) raised quite a good point: > >what's the benefit of removing the error message if this function (and > >__extract) can only report an unsigned 32 bits value? > > I didn’t spot this, sorry. > > > > >My take is we should revert 94a9992f7dbdfb28976b upstream and think at > >a better solution. > > I think using a new fix to replace it will be a better approach, as it at > least partially solves the issue. Guys, did this fall in between cracks? Is anyone planning to send a fixup? Thanks,
On Thu, May 9, 2019 at 9:30 PM Jiri Kosina <jikos@kernel.org> wrote: > > On Fri, 26 Apr 2019, Kai-Heng Feng wrote: > > > >Ronald (Cc-ed) raised quite a good point: > > >what's the benefit of removing the error message if this function (and > > >__extract) can only report an unsigned 32 bits value? > > > > I didn’t spot this, sorry. > > > > > > > >My take is we should revert 94a9992f7dbdfb28976b upstream and think at > > >a better solution. > > > > I think using a new fix to replace it will be a better approach, as it at > > least partially solves the issue. > > Guys, did this fall in between cracks? Is anyone planning to send a fixup? > Kai-Heng, have you been able to work on that? Cheers, Benjamin
> On May 21, 2019, at 9:58 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > On Thu, May 9, 2019 at 9:30 PM Jiri Kosina <jikos@kernel.org> wrote: >> >> On Fri, 26 Apr 2019, Kai-Heng Feng wrote: >> >>>> Ronald (Cc-ed) raised quite a good point: >>>> what's the benefit of removing the error message if this function (and >>>> __extract) can only report an unsigned 32 bits value? >>> >>> I didn’t spot this, sorry. >>> >>>> >>>> My take is we should revert 94a9992f7dbdfb28976b upstream and think at >>>> a better solution. >>> >>> I think using a new fix to replace it will be a better approach, as it at >>> least partially solves the issue. >> >> Guys, did this fall in between cracks? Is anyone planning to send a fixup? >> > > Kai-Heng, have you been able to work on that? Sorry, I haven’t been able to work on this. Please revert the commit and possibly use *_once() macro to reduce the noise. Kai-Heng > > Cheers, > Benjamin
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 9993b692598f..860e21ec6a49 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1301,10 +1301,10 @@ static u32 __extract(u8 *report, unsigned offset, int n) u32 hid_field_extract(const struct hid_device *hid, u8 *report, unsigned offset, unsigned n) { - if (n > 32) { - hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n", + if (n > 256) { + hid_warn(hid, "hid_field_extract() called with n (%d) > 256! (%s)\n", n, current->comm); - n = 32; + n = 256; } return __extract(report, offset, n);
Commit 71f6fa90a353 ("HID: increase maximum global item tag report size to 256") increases the max report size from 128 to 256. We also need to update the report size in hid_field_extract() otherwise it complains and truncates now valid report size: [ 406.165461] hid-sensor-hub 001F:8086:22D8.0002: hid_field_extract() called with n (192) > 32! (kworker/5:1) BugLink: https://bugs.launchpad.net/bugs/1818547 Fixes: 71f6fa90a353 ("HID: increase maximum global item tag report size to 256") Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/hid/hid-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)