* [PATCH v2 1/2] HID: i2c-hid: fix size check and type usage @ 2018-01-08 2:41 Aaron Ma 2018-01-08 2:41 ` [PATCH v2 2/2] HID: core: Fix size as type u32 Aaron Ma 0 siblings, 1 reply; 7+ messages in thread From: Aaron Ma @ 2018-01-08 2:41 UTC (permalink / raw) To: linux-kernel, linux-input, jikos, benjamin.tissoires, aaron.ma When convert char array with signed int, if the inbuf[x] is negative then upper bits will be set to 1. Fix this by using u8 instead of char. ret_size has to be at least 3, hid_input_report use it after minus 2 bytes. Cc: stable@vger.kernel.org Signed-off-by: Aaron Ma <aaron.ma@canonical.com> --- drivers/hid/i2c-hid/i2c-hid.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index e054ee43c1e2..d17d1950392b 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -144,10 +144,10 @@ struct i2c_hid { * register of the HID * descriptor. */ unsigned int bufsize; /* i2c buffer size */ - char *inbuf; /* Input buffer */ - char *rawbuf; /* Raw Input buffer */ - char *cmdbuf; /* Command buffer */ - char *argsbuf; /* Command arguments buffer */ + u8 *inbuf; /* Input buffer */ + u8 *rawbuf; /* Raw Input buffer */ + u8 *cmdbuf; /* Command buffer */ + u8 *argsbuf; /* Command arguments buffer */ unsigned long flags; /* device flags */ unsigned long quirks; /* Various quirks */ @@ -455,7 +455,8 @@ static int i2c_hid_hwreset(struct i2c_client *client) static void i2c_hid_get_input(struct i2c_hid *ihid) { - int ret, ret_size; + int ret; + u32 ret_size; int size = le16_to_cpu(ihid->hdesc.wMaxInputLength); if (size > ihid->bufsize) @@ -480,7 +481,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid) return; } - if (ret_size > size) { + if ((ret_size > size) || (ret_size <= 2)) { dev_err(&ihid->client->dev, "%s: incomplete report (%d/%d)\n", __func__, size, ret_size); return; -- 2.14.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] HID: core: Fix size as type u32 2018-01-08 2:41 [PATCH v2 1/2] HID: i2c-hid: fix size check and type usage Aaron Ma @ 2018-01-08 2:41 ` Aaron Ma 2018-02-03 3:57 ` Aaron Ma ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Aaron Ma @ 2018-01-08 2:41 UTC (permalink / raw) To: linux-kernel, linux-input, jikos, benjamin.tissoires, aaron.ma When size is negative, calling memset will make segment fault. Declare the size as type u32 to keep memset safe. size in struct hid_report is unsigned, fix return type of hid_report_len to u32. Cc: stable@vger.kernel.org Signed-off-by: Aaron Ma <aaron.ma@canonical.com> --- drivers/hid/hid-core.c | 10 +++++----- include/linux/hid.h | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 0c3f608131cf..cf81c53e3b98 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags) * of implement() working on 8 byte chunks */ - int len = hid_report_len(report) + 7; + u32 len = hid_report_len(report) + 7; return kmalloc(len, flags); } @@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct hid_report *report, { char *buf; int ret; - int len; + u32 len; buf = hid_alloc_report_buf(report, GFP_KERNEL); if (!buf) @@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct hid_report *report, } EXPORT_SYMBOL_GPL(__hid_request); -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt) { struct hid_report_enum *report_enum = hid->report_enum + type; struct hid_report *report; struct hid_driver *hdrv; unsigned int a; - int rsize, csize = size; + u32 rsize, csize = size; u8 *cdata = data; int ret = 0; @@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event); * * This is data entry for lower layers. */ -int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int interrupt) +int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt) { struct hid_report_enum *report_enum; struct hid_driver *hdrv; diff --git a/include/linux/hid.h b/include/linux/hid.h index d491027a7c22..9bc296eebc98 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, unsigned int force); extern void hidinput_disconnect(struct hid_device *); int hid_set_field(struct hid_field *, unsigned, __s32); -int hid_input_report(struct hid_device *, int type, u8 *, int, int); +int hid_input_report(struct hid_device *, int type, u8 *, u32, int); int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field); struct hid_field *hidinput_get_led_field(struct hid_device *hid); unsigned int hidinput_count_leds(struct hid_device *hid); @@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device *hdev) * * @report: the report we want to know the length */ -static inline int hid_report_len(struct hid_report *report) +static inline u32 hid_report_len(struct hid_report *report) { /* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */ return ((report->size - 1) >> 3) + 1 + (report->id > 0); } -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt); /* HID quirks API */ -- 2.14.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] HID: core: Fix size as type u32 2018-01-08 2:41 ` [PATCH v2 2/2] HID: core: Fix size as type u32 Aaron Ma @ 2018-02-03 3:57 ` Aaron Ma 2018-02-16 12:32 ` Jiri Kosina 2018-02-03 7:55 ` Marcus Folkesson 2018-02-03 15:57 ` [PATCH] HID: Fix hid_report_len usage Aaron Ma 2 siblings, 1 reply; 7+ messages in thread From: Aaron Ma @ 2018-02-03 3:57 UTC (permalink / raw) To: linux-kernel, linux-input, jikos, benjamin.tissoires Hi: Could anyone review and apply these 2 patch? Regards, Aaron ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] HID: core: Fix size as type u32 2018-02-03 3:57 ` Aaron Ma @ 2018-02-16 12:32 ` Jiri Kosina 0 siblings, 0 replies; 7+ messages in thread From: Jiri Kosina @ 2018-02-16 12:32 UTC (permalink / raw) To: Aaron Ma; +Cc: linux-kernel, linux-input, benjamin.tissoires On Sat, 3 Feb 2018, Aaron Ma wrote: > Could anyone review and apply these 2 patch? I have applied all 3 patches to for-4.16/upstream-fixes. Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] HID: core: Fix size as type u32 2018-01-08 2:41 ` [PATCH v2 2/2] HID: core: Fix size as type u32 Aaron Ma 2018-02-03 3:57 ` Aaron Ma @ 2018-02-03 7:55 ` Marcus Folkesson 2018-02-03 14:28 ` Aaron Ma 2018-02-03 15:57 ` [PATCH] HID: Fix hid_report_len usage Aaron Ma 2 siblings, 1 reply; 7+ messages in thread From: Marcus Folkesson @ 2018-02-03 7:55 UTC (permalink / raw) To: Aaron Ma; +Cc: linux-kernel, linux-input, jikos, benjamin.tissoires [-- Attachment #1: Type: text/plain, Size: 4086 bytes --] Hi Aaron, On Mon, Jan 08, 2018 at 10:41:41AM +0800, Aaron Ma wrote: > When size is negative, calling memset will make segment fault. > Declare the size as type u32 to keep memset safe. > > size in struct hid_report is unsigned, fix return type of > hid_report_len to u32. > > Cc: stable@vger.kernel.org > Signed-off-by: Aaron Ma <aaron.ma@canonical.com> > --- > drivers/hid/hid-core.c | 10 +++++----- > include/linux/hid.h | 6 +++--- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 0c3f608131cf..cf81c53e3b98 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags) > * of implement() working on 8 byte chunks > */ > > - int len = hid_report_len(report) + 7; > + u32 len = hid_report_len(report) + 7; > > return kmalloc(len, flags); > } > @@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct hid_report *report, > { > char *buf; > int ret; > - int len; > + u32 len; > > buf = hid_alloc_report_buf(report, GFP_KERNEL); > if (!buf) > @@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct hid_report *report, > } > EXPORT_SYMBOL_GPL(__hid_request); > > -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, > +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, > int interrupt) > { > struct hid_report_enum *report_enum = hid->report_enum + type; > struct hid_report *report; > struct hid_driver *hdrv; > unsigned int a; > - int rsize, csize = size; > + u32 rsize, csize = size; > u8 *cdata = data; > int ret = 0; > > @@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event); > * > * This is data entry for lower layers. > */ > -int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int interrupt) > +int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt) > { > struct hid_report_enum *report_enum; > struct hid_driver *hdrv; > diff --git a/include/linux/hid.h b/include/linux/hid.h > index d491027a7c22..9bc296eebc98 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, unsigned int force); > extern void hidinput_disconnect(struct hid_device *); > > int hid_set_field(struct hid_field *, unsigned, __s32); > -int hid_input_report(struct hid_device *, int type, u8 *, int, int); > +int hid_input_report(struct hid_device *, int type, u8 *, u32, int); > int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field); > struct hid_field *hidinput_get_led_field(struct hid_device *hid); > unsigned int hidinput_count_leds(struct hid_device *hid); > @@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device *hdev) > * > * @report: the report we want to know the length > */ > -static inline int hid_report_len(struct hid_report *report) > +static inline u32 hid_report_len(struct hid_report *report) hid_report_len() is used in several files. If we think it is a good idea to change the return type, we should fix these files as well. [08:47:56]marcus@little:~/git/linux$ git grep -l hid_report_len drivers/hid/hid-core.c drivers/hid/hid-input.c drivers/hid/hid-multitouch.c drivers/hid/hid-rmi.c drivers/hid/usbhid/hid-core.c drivers/hid/wacom_sys.c drivers/staging/greybus/hid.c include/linux/hid.h > { > /* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */ > return ((report->size - 1) >> 3) + 1 + (report->id > 0); > } > > -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, > +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, > int interrupt); > > /* HID quirks API */ > -- > 2.14.3 Best regards Marcus Folkesson > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] HID: core: Fix size as type u32 2018-02-03 7:55 ` Marcus Folkesson @ 2018-02-03 14:28 ` Aaron Ma 0 siblings, 0 replies; 7+ messages in thread From: Aaron Ma @ 2018-02-03 14:28 UTC (permalink / raw) To: Marcus Folkesson; +Cc: linux-kernel, linux-input, jikos, benjamin.tissoires On 02/03/2018 03:55 PM, Marcus Folkesson wrote: > Hi Aaron, > > On Mon, Jan 08, 2018 at 10:41:41AM +0800, Aaron Ma wrote: >> When size is negative, calling memset will make segment fault. >> Declare the size as type u32 to keep memset safe. >> >> size in struct hid_report is unsigned, fix return type of >> hid_report_len to u32. >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Aaron Ma <aaron.ma@canonical.com> >> --- >> drivers/hid/hid-core.c | 10 +++++----- >> include/linux/hid.h | 6 +++--- >> 2 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c >> index 0c3f608131cf..cf81c53e3b98 100644 >> --- a/drivers/hid/hid-core.c >> +++ b/drivers/hid/hid-core.c >> @@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags) >> * of implement() working on 8 byte chunks >> */ >> >> - int len = hid_report_len(report) + 7; >> + u32 len = hid_report_len(report) + 7; >> >> return kmalloc(len, flags); >> } >> @@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct hid_report *report, >> { >> char *buf; >> int ret; >> - int len; >> + u32 len; >> >> buf = hid_alloc_report_buf(report, GFP_KERNEL); >> if (!buf) >> @@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct hid_report *report, >> } >> EXPORT_SYMBOL_GPL(__hid_request); >> >> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, >> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, >> int interrupt) >> { >> struct hid_report_enum *report_enum = hid->report_enum + type; >> struct hid_report *report; >> struct hid_driver *hdrv; >> unsigned int a; >> - int rsize, csize = size; >> + u32 rsize, csize = size; >> u8 *cdata = data; >> int ret = 0; >> >> @@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event); >> * >> * This is data entry for lower layers. >> */ >> -int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int interrupt) >> +int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt) >> { >> struct hid_report_enum *report_enum; >> struct hid_driver *hdrv; >> diff --git a/include/linux/hid.h b/include/linux/hid.h >> index d491027a7c22..9bc296eebc98 100644 >> --- a/include/linux/hid.h >> +++ b/include/linux/hid.h >> @@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, unsigned int force); >> extern void hidinput_disconnect(struct hid_device *); >> >> int hid_set_field(struct hid_field *, unsigned, __s32); >> -int hid_input_report(struct hid_device *, int type, u8 *, int, int); >> +int hid_input_report(struct hid_device *, int type, u8 *, u32, int); >> int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field); >> struct hid_field *hidinput_get_led_field(struct hid_device *hid); >> unsigned int hidinput_count_leds(struct hid_device *hid); >> @@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device *hdev) >> * >> * @report: the report we want to know the length >> */ >> -static inline int hid_report_len(struct hid_report *report) >> +static inline u32 hid_report_len(struct hid_report *report) > hid_report_len() is used in several files. > If we think it is a good idea to change the return type, we should fix > these files as well. > > [08:47:56]marcus@little:~/git/linux$ git grep -l hid_report_len > drivers/hid/hid-core.c > drivers/hid/hid-input.c > drivers/hid/hid-multitouch.c > drivers/hid/hid-rmi.c > drivers/hid/usbhid/hid-core.c > drivers/hid/wacom_sys.c > drivers/staging/greybus/hid.c > include/linux/hid.h Sure, I will fix these return type in a new patch. Thanks, Aaron > >> { >> /* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */ >> return ((report->size - 1) >> 3) + 1 + (report->id > 0); >> } >> >> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, >> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, >> int interrupt); >> >> /* HID quirks API */ >> -- >> 2.14.3 > Best regards > Marcus Folkesson ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] HID: Fix hid_report_len usage 2018-01-08 2:41 ` [PATCH v2 2/2] HID: core: Fix size as type u32 Aaron Ma 2018-02-03 3:57 ` Aaron Ma 2018-02-03 7:55 ` Marcus Folkesson @ 2018-02-03 15:57 ` Aaron Ma 2 siblings, 0 replies; 7+ messages in thread From: Aaron Ma @ 2018-02-03 15:57 UTC (permalink / raw) To: aaron.ma, jikos, benjamin.tissoires, linux-input, linux-kernel Follow the change of return type u32 of hid_report_len, fix all the types of variables those get the return value of hid_report_len to u32, and all other code already uses u32. Cc: stable@vger.kernel.org Signed-off-by: Aaron Ma <aaron.ma@canonical.com> --- drivers/hid/hid-input.c | 3 ++- drivers/hid/hid-multitouch.c | 5 +++-- drivers/hid/hid-rmi.c | 4 ++-- drivers/hid/wacom_sys.c | 4 ++-- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 04d01b57d94c..d86398755b0d 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1368,7 +1368,8 @@ static void hidinput_led_worker(struct work_struct *work) led_work); struct hid_field *field; struct hid_report *report; - int len, ret; + int ret; + u32 len; __u8 *buf; field = hidinput_get_led_field(hid); diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 3b4739bde05d..2e1736ba2444 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -370,7 +370,8 @@ static const struct attribute_group mt_attribute_group = { static void mt_get_feature(struct hid_device *hdev, struct hid_report *report) { struct mt_device *td = hid_get_drvdata(hdev); - int ret, size = hid_report_len(report); + int ret; + u32 size = hid_report_len(report); u8 *buf; /* @@ -1183,7 +1184,7 @@ static void mt_set_input_mode(struct hid_device *hdev) struct hid_report_enum *re; struct mt_class *cls = &td->mtclass; char *buf; - int report_len; + u32 report_len; if (td->inputmode < 0) return; diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c index c6c05df3e8d2..9c9362149641 100644 --- a/drivers/hid/hid-rmi.c +++ b/drivers/hid/hid-rmi.c @@ -89,8 +89,8 @@ struct rmi_data { u8 *writeReport; u8 *readReport; - int input_report_size; - int output_report_size; + u32 input_report_size; + u32 output_report_size; unsigned long flags; diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index 409543160af7..b54ef1ffcbec 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -219,7 +219,7 @@ static void wacom_feature_mapping(struct hid_device *hdev, unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid); u8 *data; int ret; - int n; + u32 n; switch (equivalent_usage) { case HID_DG_CONTACTMAX: @@ -519,7 +519,7 @@ static int wacom_set_device_mode(struct hid_device *hdev, u8 *rep_data; struct hid_report *r; struct hid_report_enum *re; - int length; + u32 length; int error = -ENOMEM, limit = 0; if (wacom_wac->mode_report < 0) -- 2.14.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-02-16 12:32 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-08 2:41 [PATCH v2 1/2] HID: i2c-hid: fix size check and type usage Aaron Ma 2018-01-08 2:41 ` [PATCH v2 2/2] HID: core: Fix size as type u32 Aaron Ma 2018-02-03 3:57 ` Aaron Ma 2018-02-16 12:32 ` Jiri Kosina 2018-02-03 7:55 ` Marcus Folkesson 2018-02-03 14:28 ` Aaron Ma 2018-02-03 15:57 ` [PATCH] HID: Fix hid_report_len usage Aaron Ma
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).