From 27ce405039bfe6d3f4143415c638f56a3df77dca Mon Sep 17 00:00:00 2001 From: Jiri Kosina Date: Wed, 10 Jul 2013 19:56:27 +0200 Subject: HID: fix data access in implement() implement() is setting bytes in LE data stream. In case the data is not aligned to 64bits, it reads past the allocated buffer. It doesn't really change any value there (it's properly bitmasked), but in case that this read past the boundary hits a page boundary, pagefault happens when accessing 64bits of 'x' in implement(), and kernel oopses. This happens much more often when numbered reports are in use, as the initial 8bit skip in the buffer makes the whole process work on values which are not aligned to 64bits. This problem dates back to attempts in 2005 and 2006 to make implement() and extract() as generic as possible, and even back then the problem was realized by Adam Kroperlin, but falsely assumed to be impossible to cause any harm: http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html I have made several attempts at fixing it "on the spot" directly in implement(), but the results were horrible; the special casing for processing last 64bit chunk and switching to different math makes it unreadable mess. I therefore took a path to allocate a few bytes more which will never make it into final report, but are there as a cushion for all the 64bit math operations happening in implement() and extract(). All callers of hid_output_report() are converted at the same time to allocate the buffer by newly introduced hid_alloc_report_buf() helper. Bruno noticed that the whole raw_size test can be dropped as well, as hid_alloc_report_buf() makes sure that the buffer is always of a proper size. Reviewed-by: Benjamin Tissoires Acked-by: Gustavo Padovan Signed-off-by: Jiri Kosina --- net/bluetooth/hidp/core.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'net') diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 46c6a148f0b..212980ff99b 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -231,17 +231,21 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb) static int hidp_send_report(struct hidp_session *session, struct hid_report *report) { - unsigned char buf[32], hdr; - int rsize; + unsigned char hdr; + u8 *buf; + int rsize, ret; - rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0); - if (rsize > sizeof(buf)) + buf = hid_alloc_report_buf(report, GFP_ATOMIC); + if (!buf) return -EIO; hid_output_report(report, buf); hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT; - return hidp_send_intr_message(session, hdr, buf, rsize); + ret = hidp_send_intr_message(session, hdr, buf, rsize); + + kfree(buf); + return ret; } static int hidp_get_raw_report(struct hid_device *hid, -- cgit v1.2.3-70-g09d2 From bc197eedef1ae082ec662c64c3f4aa302821fb7a Mon Sep 17 00:00:00 2001 From: Jiri Kosina Date: Mon, 22 Jul 2013 17:11:44 +0200 Subject: HID: fix unused rsize usage 27ce4050 ("HID: fix data access in implement()") by mistake removed a setting of buffer size in hidp. Fix that by putting it back. Reported-by: kbuild test robot Signed-off-by: Jiri Kosina --- net/bluetooth/hidp/core.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net') diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 212980ff99b..2e658ade445 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -242,6 +242,7 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep hid_output_report(report, buf); hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT; + rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0); ret = hidp_send_intr_message(session, hdr, buf, rsize); kfree(buf); -- cgit v1.2.3-70-g09d2