| Message ID | 1387451372-6881-1-git-send-email-dh.herrmann@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
Hi David, > HID core expects the input buffers to be at least of size 4096 > (HID_MAX_BUFFER_SIZE). Other sizes will result in buffer-overflows if an > input-report is smaller than advertised. We could, like i2c, compute the > biggest report-size instead of using HID_MAX_BUFFER_SIZE, but this will > blow up if report-descriptors are changed after ->start() has been called. > So lets be safe and just use the biggest buffer we have. > > Note that this adds an additional copy to the HIDP input path. If there is > a way to make sure the skb-buf is big enough, we should use that instead. > > The best way would be to make hid-core honor the @size argument, though, > that sounds easier than it is. So lets just fix the buffer-overflows for > now and afterwards look for a faster way for all transport drivers. > > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> > --- > Hi > > Any ideas how to improve this patch? I'd like to avoid the extra copy but I have > no clue how the skb stuff works exactly. the buffers are coming straight from L2CAP. They might come in fragments. We have to reassemble them and while there are large packets for sure, we rather not want to allocate 4096 for every reassembled packet to make HID happy. I am not super familiar with the underlying memory management of SKBs, but I assume they use slices of some sort internally. So uses large SKBs for 20 byte HID report will cause an issue with all other protocols running on top of L2CAP> > I also haven't figured out a nice way to make HID-core honor the "size" > parameter. hid-input depends on getting the whole input-report. I think this needs clearly fixing. > Comments welcome! > David > > net/bluetooth/hidp/core.c | 16 ++++++++++++++-- > net/bluetooth/hidp/hidp.h | 4 ++++ > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > index 292e619..d9fb934 100644 > --- a/net/bluetooth/hidp/core.c > +++ b/net/bluetooth/hidp/core.c > @@ -430,6 +430,16 @@ static void hidp_del_timer(struct hidp_session *session) > del_timer(&session->timer); > } > > +static void hidp_process_report(struct hidp_session *session, > + int type, const u8 *data, int len, int intr) > +{ > + if (len > HID_MAX_BUFFER_SIZE) > + len = HID_MAX_BUFFER_SIZE; > + > + memcpy(session->input_buf, data, len); > + hid_input_report(session->hid, type, session->input_buf, len, intr); > +} > + > static void hidp_process_handshake(struct hidp_session *session, > unsigned char param) > { > @@ -502,7 +512,8 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb, > hidp_input_report(session, skb); > > if (session->hid) > - hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 0); > + hidp_process_report(session, HID_INPUT_REPORT, > + skb->data, skb->len, 0); > break; > > case HIDP_DATA_RTYPE_OTHER: > @@ -584,7 +595,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session, > hidp_input_report(session, skb); > > if (session->hid) { > - hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 1); > + hidp_process_report(session, HID_INPUT_REPORT, > + skb->data, skb->len, 1); > BT_DBG("report len %d", skb->len); > } > } else { > diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h > index ab52414..8798492 100644 > --- a/net/bluetooth/hidp/hidp.h > +++ b/net/bluetooth/hidp/hidp.h > @@ -24,6 +24,7 @@ > #define __HIDP_H > > #include <linux/types.h> > +#include <linux/hid.h> > #include <linux/kref.h> > #include <net/bluetooth/bluetooth.h> > #include <net/bluetooth/l2cap.h> > @@ -179,6 +180,9 @@ struct hidp_session { > > /* Used in hidp_output_raw_report() */ > int output_report_success; /* boolean */ > + > + /* temporary input buffer */ > + u8 input_buf[HID_MAX_BUFFER_SIZE]; > }; The report does not need any specific alignment? Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Marcel On Fri, Dec 20, 2013 at 2:36 PM, Marcel Holtmann <marcel@holtmann.org> wrote: > Hi David, > >> HID core expects the input buffers to be at least of size 4096 >> (HID_MAX_BUFFER_SIZE). Other sizes will result in buffer-overflows if an >> input-report is smaller than advertised. We could, like i2c, compute the >> biggest report-size instead of using HID_MAX_BUFFER_SIZE, but this will >> blow up if report-descriptors are changed after ->start() has been called. >> So lets be safe and just use the biggest buffer we have. >> >> Note that this adds an additional copy to the HIDP input path. If there is >> a way to make sure the skb-buf is big enough, we should use that instead. >> >> The best way would be to make hid-core honor the @size argument, though, >> that sounds easier than it is. So lets just fix the buffer-overflows for >> now and afterwards look for a faster way for all transport drivers. >> >> Signed-off-by: David Herrmann <dh.herrmann@gmail.com> >> --- >> Hi >> >> Any ideas how to improve this patch? I'd like to avoid the extra copy but I have >> no clue how the skb stuff works exactly. > > the buffers are coming straight from L2CAP. They might come in fragments. We have to reassemble them and while there are large packets for sure, we rather not want to allocate 4096 for every reassembled packet to make HID happy. > > I am not super familiar with the underlying memory management of SKBs, but I assume they use slices of some sort internally. So uses large SKBs for 20 byte HID report will cause an issue with all other protocols running on top of L2CAP I was more thinking of an skb call to increase the underlying buffer of a single package. We could call this together with skb_linearize() in HIDP to guarantee the skb-buf is big enough. Technically, this will probably result in the same behavior as my own patch so probably not the way to go. >> I also haven't figured out a nice way to make HID-core honor the "size" >> parameter. hid-input depends on getting the whole input-report. > > I think this needs clearly fixing. And we have a volunteer, yippie! ;) I think hid_input_report() in hid-core.c is the only place that relies on this. Maybe it really is easier to fix it. But I am not even slightly familiar with that code. So if no-one of the HID core developers steps up to fix it, we should take the transport-driver fixes instead. As nearly all transport-drivers are affected by this, maybe we should even move this buffer into hid_device and provide hid_input_truncated_report() which does this. Anyhow, waiting for Jiri's comments on this. >> Comments welcome! >> David >> >> net/bluetooth/hidp/core.c | 16 ++++++++++++++-- >> net/bluetooth/hidp/hidp.h | 4 ++++ >> 2 files changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c >> index 292e619..d9fb934 100644 >> --- a/net/bluetooth/hidp/core.c >> +++ b/net/bluetooth/hidp/core.c >> @@ -430,6 +430,16 @@ static void hidp_del_timer(struct hidp_session *session) >> del_timer(&session->timer); >> } >> >> +static void hidp_process_report(struct hidp_session *session, >> + int type, const u8 *data, int len, int intr) >> +{ >> + if (len > HID_MAX_BUFFER_SIZE) >> + len = HID_MAX_BUFFER_SIZE; >> + >> + memcpy(session->input_buf, data, len); >> + hid_input_report(session->hid, type, session->input_buf, len, intr); >> +} >> + >> static void hidp_process_handshake(struct hidp_session *session, >> unsigned char param) >> { >> @@ -502,7 +512,8 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb, >> hidp_input_report(session, skb); >> >> if (session->hid) >> - hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 0); >> + hidp_process_report(session, HID_INPUT_REPORT, >> + skb->data, skb->len, 0); >> break; >> >> case HIDP_DATA_RTYPE_OTHER: >> @@ -584,7 +595,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session, >> hidp_input_report(session, skb); >> >> if (session->hid) { >> - hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 1); >> + hidp_process_report(session, HID_INPUT_REPORT, >> + skb->data, skb->len, 1); >> BT_DBG("report len %d", skb->len); >> } >> } else { >> diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h >> index ab52414..8798492 100644 >> --- a/net/bluetooth/hidp/hidp.h >> +++ b/net/bluetooth/hidp/hidp.h >> @@ -24,6 +24,7 @@ >> #define __HIDP_H >> >> #include <linux/types.h> >> +#include <linux/hid.h> >> #include <linux/kref.h> >> #include <net/bluetooth/bluetooth.h> >> #include <net/bluetooth/l2cap.h> >> @@ -179,6 +180,9 @@ struct hidp_session { >> >> /* Used in hidp_output_raw_report() */ >> int output_report_success; /* boolean */ >> + >> + /* temporary input buffer */ >> + u8 input_buf[HID_MAX_BUFFER_SIZE]; >> }; > > The report does not need any specific alignment? This is already aligned to native size, isn't it? Anyhow, HID core does not have any alignment-restrictions I am aware of. Jiri? I thought the extract()/implement() stuff was fixed recently. Thanks David -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 27 Dec 2013, David Herrmann wrote: > >> I also haven't figured out a nice way to make HID-core honor the > >> "size" parameter. hid-input depends on getting the whole > >> input-report. > > > > I think this needs clearly fixing. > > And we have a volunteer, yippie! ;) > > I think hid_input_report() in hid-core.c is the only place that relies > on this. Maybe it really is easier to fix it. But I am not even > slightly familiar with that code. So if no-one of the HID core > developers steps up to fix it, we should take the transport-driver > fixes instead. As nearly all transport-drivers are affected by this, > maybe we should even move this buffer into hid_device and provide > hid_input_truncated_report() which does this. > > Anyhow, waiting for Jiri's comments on this. Hmm, this is really unfortunate situation. I am now looking into making hid_input_report() honor 'size' properly, but no matter how it'll be done in the end, it'll absolutely not be a simple 'fix'. So definitely can be done for 3.15 or so, but not as a fix for current kernels. So doing kzalloc(rsize, GFP_ATOMIC) in the HID-core for now, and copying the buffer around, seems like only viable solution for now, with the outlook of removing this ugliness once hid-core honors 'size' properly. I will keep looking into this and maybe some easy way how to hack this together will materialize, but I don't currently see it. Hmm ...
Hi Jiri On Tue, Jan 7, 2014 at 1:11 PM, Jiri Kosina <jkosina@suse.cz> wrote: > On Fri, 27 Dec 2013, David Herrmann wrote: > >> >> I also haven't figured out a nice way to make HID-core honor the >> >> "size" parameter. hid-input depends on getting the whole >> >> input-report. >> > >> > I think this needs clearly fixing. >> >> And we have a volunteer, yippie! ;) >> >> I think hid_input_report() in hid-core.c is the only place that relies >> on this. Maybe it really is easier to fix it. But I am not even >> slightly familiar with that code. So if no-one of the HID core >> developers steps up to fix it, we should take the transport-driver >> fixes instead. As nearly all transport-drivers are affected by this, >> maybe we should even move this buffer into hid_device and provide >> hid_input_truncated_report() which does this. >> >> Anyhow, waiting for Jiri's comments on this. > > Hmm, this is really unfortunate situation. > > I am now looking into making hid_input_report() honor 'size' properly, but > no matter how it'll be done in the end, it'll absolutely not be a simple > 'fix'. So definitely can be done for 3.15 or so, but not as a fix for > current kernels. > > So doing kzalloc(rsize, GFP_ATOMIC) in the HID-core for now, and copying > the buffer around, seems like only viable solution for now, with the > outlook of removing this ugliness once hid-core honors 'size' properly. Should I resend the patches and move it to hid_input_report() for now? Or are you getting something in yourself? Given the amount of reports on the list and bugzilla, I think we should get this fix in asap. We can always fix it properly in -next. Thanks David > I will keep looking into this and maybe some easy way how to hack this > together will materialize, but I don't currently see it. > > Hmm ... > > -- > Jiri Kosina > SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 7 Jan 2014, David Herrmann wrote: > > So doing kzalloc(rsize, GFP_ATOMIC) in the HID-core for now, and copying > > the buffer around, seems like only viable solution for now, with the > > outlook of removing this ugliness once hid-core honors 'size' properly. > > Should I resend the patches and move it to hid_input_report() for now? > Or are you getting something in yourself? Due to various reasons I will not have access to any testing HW for the upcoming 2-3 days, so if you can cook something up in that timeframe, it'd be appreciated. Otherwise I'll be working on it by the end of this week. > Given the amount of reports on the list and bugzilla, I think we should > get this fix in asap. We can always fix it properly in -next. Agreed. Thanks,
On Tue, 7 Jan 2014, Jiri Kosina wrote: > > > So doing kzalloc(rsize, GFP_ATOMIC) in the HID-core for now, and copying > > > the buffer around, seems like only viable solution for now, with the > > > outlook of removing this ugliness once hid-core honors 'size' properly. > > > > Should I resend the patches and move it to hid_input_report() for now? > > Or are you getting something in yourself? > > Due to various reasons I will not have access to any testing HW for the > upcoming 2-3 days, so if you can cook something up in that timeframe, it'd > be appreciated. > > Otherwise I'll be working on it by the end of this week. David, just got back to this, finally ... did you have time to work on this at all, or should I just start from scratch? > > Given the amount of reports on the list and bugzilla, I think we should > > get this fix in asap. We can always fix it properly in -next. > > Agreed.
Hi On Tue, Jan 28, 2014 at 9:53 PM, Jiri Kosina <jkosina@suse.cz> wrote: > On Tue, 7 Jan 2014, Jiri Kosina wrote: > >> > > So doing kzalloc(rsize, GFP_ATOMIC) in the HID-core for now, and copying >> > > the buffer around, seems like only viable solution for now, with the >> > > outlook of removing this ugliness once hid-core honors 'size' properly. >> > >> > Should I resend the patches and move it to hid_input_report() for now? >> > Or are you getting something in yourself? >> >> Due to various reasons I will not have access to any testing HW for the >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd >> be appreciated. >> >> Otherwise I'll be working on it by the end of this week. > > David, > > just got back to this, finally ... did you have time to work on this at > all, or should I just start from scratch? Sorry, no. Fosdem is this weekend and I needed to get my code ready for that. But I'll finally have time again next week. Thanks David -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 29 Jan 2014, David Herrmann wrote: > >> Due to various reasons I will not have access to any testing HW for the > >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd > >> be appreciated. > >> > >> Otherwise I'll be working on it by the end of this week. > > > > David, > > > > just got back to this, finally ... did you have time to work on this at > > all, or should I just start from scratch? > > Sorry, no. Fosdem is this weekend and I needed to get my code ready > for that. But I'll finally have time again next week. Okay, thanks. I then guess we should proceed with this bandaid (double allocation) for 3.14
Hi Jiri On Mon, Feb 3, 2014 at 11:08 AM, Jiri Kosina <jkosina@suse.cz> wrote: > On Wed, 29 Jan 2014, David Herrmann wrote: > >> >> Due to various reasons I will not have access to any testing HW for the >> >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd >> >> be appreciated. >> >> >> >> Otherwise I'll be working on it by the end of this week. >> > >> > David, >> > >> > just got back to this, finally ... did you have time to work on this at >> > all, or should I just start from scratch? >> >> Sorry, no. Fosdem is this weekend and I needed to get my code ready >> for that. But I'll finally have time again next week. > > Okay, thanks. I then guess we should proceed with this bandaid (double > allocation) for 3.14 It would be really nice if we could get the HIDP patch into 3.14-rc2 and backported to stable. There have been quite a bunch of reports now and I dislike adding a GFP_ATOMIC allocation in HID core. I am back home now and will try to make HID core honor the length, but that's quite invasive and shouldn't be used for stable. Sorry for the delay, David -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 3 Feb 2014, David Herrmann wrote: > >> >> Due to various reasons I will not have access to any testing HW for the > >> >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd > >> >> be appreciated. > >> >> > >> >> Otherwise I'll be working on it by the end of this week. > >> > > >> > David, > >> > > >> > just got back to this, finally ... did you have time to work on this at > >> > all, or should I just start from scratch? > >> > >> Sorry, no. Fosdem is this weekend and I needed to get my code ready > >> for that. But I'll finally have time again next week. > > > > Okay, thanks. I then guess we should proceed with this bandaid (double > > allocation) for 3.14 > > It would be really nice if we could get the HIDP patch into 3.14-rc2 > and backported to stable. There have been quite a bunch of reports now > and I dislike adding a GFP_ATOMIC allocation in HID core. I agree with David; Gustavo, what is your take on this, please? Thanks,
Hi Jiri, >>>>>> Due to various reasons I will not have access to any testing HW for the >>>>>> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd >>>>>> be appreciated. >>>>>> >>>>>> Otherwise I'll be working on it by the end of this week. >>>>> >>>>> David, >>>>> >>>>> just got back to this, finally ... did you have time to work on this at >>>>> all, or should I just start from scratch? >>>> >>>> Sorry, no. Fosdem is this weekend and I needed to get my code ready >>>> for that. But I'll finally have time again next week. >>> >>> Okay, thanks. I then guess we should proceed with this bandaid (double >>> allocation) for 3.14 >> >> It would be really nice if we could get the HIDP patch into 3.14-rc2 >> and backported to stable. There have been quite a bunch of reports now >> and I dislike adding a GFP_ATOMIC allocation in HID core. > > I agree with David; Gustavo, what is your take on this, please? I leave this up to Gustavo to get this into wireless tree for 3.14-rc2. Acked-by: Marcel Holtmann <marcel@holtmann.org> Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 4 Feb 2014, Marcel Holtmann wrote: > >>>>> just got back to this, finally ... did you have time to work on this at > >>>>> all, or should I just start from scratch? > >>>> > >>>> Sorry, no. Fosdem is this weekend and I needed to get my code ready > >>>> for that. But I'll finally have time again next week. > >>> > >>> Okay, thanks. I then guess we should proceed with this bandaid (double > >>> allocation) for 3.14 > >> > >> It would be really nice if we could get the HIDP patch into 3.14-rc2 > >> and backported to stable. There have been quite a bunch of reports now > >> and I dislike adding a GFP_ATOMIC allocation in HID core. > > > > I agree with David; Gustavo, what is your take on this, please? > > I leave this up to Gustavo to get this into wireless tree for 3.14-rc2. > > Acked-by: Marcel Holtmann <marcel@holtmann.org> Thanks a lot. Gustavo, what is your take on this please? I can take it through hid.git in case you don't have anything else queued for -rc2.
On Wed, 5 Feb 2014, Jiri Kosina wrote: > > >>>>> just got back to this, finally ... did you have time to work on this at > > >>>>> all, or should I just start from scratch? > > >>>> > > >>>> Sorry, no. Fosdem is this weekend and I needed to get my code ready > > >>>> for that. But I'll finally have time again next week. > > >>> > > >>> Okay, thanks. I then guess we should proceed with this bandaid (double > > >>> allocation) for 3.14 > > >> > > >> It would be really nice if we could get the HIDP patch into 3.14-rc2 > > >> and backported to stable. There have been quite a bunch of reports now > > >> and I dislike adding a GFP_ATOMIC allocation in HID core. > > > > > > I agree with David; Gustavo, what is your take on this, please? > > > > I leave this up to Gustavo to get this into wireless tree for 3.14-rc2. > > > > Acked-by: Marcel Holtmann <marcel@holtmann.org> > > Thanks a lot. > > Gustavo, what is your take on this please? I can take it through hid.git > in case you don't have anything else queued for -rc2. ... ping? In case this doesn't get reacted upon by the end of the week, I am inclined to take it through hid.git. Thanks,
Hi Jiri, >>>>>>>> just got back to this, finally ... did you have time to work on this at >>>>>>>> all, or should I just start from scratch? >>>>>>> >>>>>>> Sorry, no. Fosdem is this weekend and I needed to get my code ready >>>>>>> for that. But I'll finally have time again next week. >>>>>> >>>>>> Okay, thanks. I then guess we should proceed with this bandaid (double >>>>>> allocation) for 3.14 >>>>> >>>>> It would be really nice if we could get the HIDP patch into 3.14-rc2 >>>>> and backported to stable. There have been quite a bunch of reports now >>>>> and I dislike adding a GFP_ATOMIC allocation in HID core. >>>> >>>> I agree with David; Gustavo, what is your take on this, please? >>> >>> I leave this up to Gustavo to get this into wireless tree for 3.14-rc2. >>> >>> Acked-by: Marcel Holtmann <marcel@holtmann.org> >> >> Thanks a lot. >> >> Gustavo, what is your take on this please? I can take it through hid.git >> in case you don't have anything else queued for -rc2. > > ... ping? > > In case this doesn't get reacted upon by the end of the week, I am > inclined to take it through hid.git. take it through hid.git please. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 17 Feb 2014, Marcel Holtmann wrote: > >> Gustavo, what is your take on this please? I can take it through hid.git > >> in case you don't have anything else queued for -rc2. > > > > ... ping? > > > > In case this doesn't get reacted upon by the end of the week, I am > > inclined to take it through hid.git. > > take it through hid.git please. Now applied, thanks.
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 292e619..d9fb934 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -430,6 +430,16 @@ static void hidp_del_timer(struct hidp_session *session) del_timer(&session->timer); } +static void hidp_process_report(struct hidp_session *session, + int type, const u8 *data, int len, int intr) +{ + if (len > HID_MAX_BUFFER_SIZE) + len = HID_MAX_BUFFER_SIZE; + + memcpy(session->input_buf, data, len); + hid_input_report(session->hid, type, session->input_buf, len, intr); +} + static void hidp_process_handshake(struct hidp_session *session, unsigned char param) { @@ -502,7 +512,8 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb, hidp_input_report(session, skb); if (session->hid) - hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 0); + hidp_process_report(session, HID_INPUT_REPORT, + skb->data, skb->len, 0); break; case HIDP_DATA_RTYPE_OTHER: @@ -584,7 +595,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session, hidp_input_report(session, skb); if (session->hid) { - hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 1); + hidp_process_report(session, HID_INPUT_REPORT, + skb->data, skb->len, 1); BT_DBG("report len %d", skb->len); } } else { diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h index ab52414..8798492 100644 --- a/net/bluetooth/hidp/hidp.h +++ b/net/bluetooth/hidp/hidp.h @@ -24,6 +24,7 @@ #define __HIDP_H #include <linux/types.h> +#include <linux/hid.h> #include <linux/kref.h> #include <net/bluetooth/bluetooth.h> #include <net/bluetooth/l2cap.h> @@ -179,6 +180,9 @@ struct hidp_session { /* Used in hidp_output_raw_report() */ int output_report_success; /* boolean */ + + /* temporary input buffer */ + u8 input_buf[HID_MAX_BUFFER_SIZE]; }; /* HIDP init defines */
HID core expects the input buffers to be at least of size 4096 (HID_MAX_BUFFER_SIZE). Other sizes will result in buffer-overflows if an input-report is smaller than advertised. We could, like i2c, compute the biggest report-size instead of using HID_MAX_BUFFER_SIZE, but this will blow up if report-descriptors are changed after ->start() has been called. So lets be safe and just use the biggest buffer we have. Note that this adds an additional copy to the HIDP input path. If there is a way to make sure the skb-buf is big enough, we should use that instead. The best way would be to make hid-core honor the @size argument, though, that sounds easier than it is. So lets just fix the buffer-overflows for now and afterwards look for a faster way for all transport drivers. Signed-off-by: David Herrmann <dh.herrmann@gmail.com> --- Hi Any ideas how to improve this patch? I'd like to avoid the extra copy but I have no clue how the skb stuff works exactly. I also haven't figured out a nice way to make HID-core honor the "size" parameter. hid-input depends on getting the whole input-report. Comments welcome! David net/bluetooth/hidp/core.c | 16 ++++++++++++++-- net/bluetooth/hidp/hidp.h | 4 ++++ 2 files changed, 18 insertions(+), 2 deletions(-)