| Message ID | 1305834082-14888-1-git-send-email-armando.visconti@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Thu, 19 May 2011, Armando Visconti wrote: > I'm using a Data Modul EasyTouch USB multitouch controller, > which is issuing a hid report with a size equals to 0. The rsize > value gets set to 536870912 and Linux is crashing in the memset > because the value is too big. > > Signed-off-by: Armando Visconti <armando.visconti@st.com> > --- > drivers/hid/hid-core.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index c3d6626..3e972e3 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1045,6 +1045,9 @@ void hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, > > rsize = ((report->size - 1) >> 3) + 1; > > + if (rsize > HID_MAX_BUFFER_SIZE) > + rsize = HID_MAX_BUFFER_SIZE; > + > if (csize < rsize) { > dbg_hid("report %d is too short, (%d < %d)\n", report->id, > csize, rsize); Applied, thank you.
On Thu, 19 May 2011 21:41:22 +0200, Armando Visconti said: > I'm using a Data Modul EasyTouch USB multitouch controller, > which is issuing a hid report with a size equals to 0. The rsize > value gets set to 536870912 and Linux is crashing in the memset > because the value is too big. > > Signed-off-by: Armando Visconti <armando.visconti@st.com> > --- > drivers/hid/hid-core.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index c3d6626..3e972e3 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1045,6 +1045,9 @@ void hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, > > rsize = ((report->size - 1) >> 3) + 1; > > + if (rsize > HID_MAX_BUFFER_SIZE) > + rsize = HID_MAX_BUFFER_SIZE; > + > if (csize < rsize) { > dbg_hid("report %d is too short, (%d < %d)\n", report->id, > csize, rsize); I'm thinking this is papering over the bug, and causing us to process a max-sized buffer when the other end gave *zero* bytes back - this can't be good. Probably should be more like this: if (!report->size) then /* rsize = MIN_VALID_SIZE; /* whatever it should be here */ else { rsize = ((report->size - 1) >> 3) + 1; if (rsize > HID_MAX_BUFFER_SIZE) rsize = HID_MAX_BUFFER_SIZE; } if (csize < rsize) { That last if() still looks squirrely - if we have an effectively zero rsize, the report is short and the dbg_hid should fire. Did we want "csize > rsize" instead?
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index c3d6626..3e972e3 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1045,6 +1045,9 @@ void hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size, rsize = ((report->size - 1) >> 3) + 1; + if (rsize > HID_MAX_BUFFER_SIZE) + rsize = HID_MAX_BUFFER_SIZE; + if (csize < rsize) { dbg_hid("report %d is too short, (%d < %d)\n", report->id, csize, rsize);
I'm using a Data Modul EasyTouch USB multitouch controller, which is issuing a hid report with a size equals to 0. The rsize value gets set to 536870912 and Linux is crashing in the memset because the value is too big. Signed-off-by: Armando Visconti <armando.visconti@st.com> --- drivers/hid/hid-core.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)