Hi Juha,
On Thu, Dec 22, 2016 at 3:17 AM, Juha Kuikka <juha.kuikka@xxxxxxxxxxx> wrote:
> I believe I have identified an issue with the HID-over-GATT (HoG) where,
> through hidraw, the HIDIOCGFEATURE does not work correctly.
>
> The symptom is that the ioctl call returns immediately with bogus data,
> before the Read Request to the peripheral has been completed.
>
> I believe the issue is caused by the hog-lib.c registering a handler for
> both UHID_FEATURE and UHID_GET_REPORT events, which in the uhid header
> file turn out to be the same enum.
>
> This causes the get_report() to get called first, it issues the Read
> Request and waits for it's completion. After this the get_feature() is
> immediately called with the same uhid message, which sends the
> UHID_FEATURE_ANSWER in to the kernel with stale data, which then gets
> returned to the hidraw caller.
>
> I have fixed this by removing the get_feature() as it is unnecessary
> anyway. See attached patch.
>
> I have tested with against both old and new uhid API (kernels 3.8 and
> 4.4).
> ---
> profiles/input/hog-lib.c | 33 ---------------------------------
> 1 file changed, 33 deletions(-)
>
> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> index e376c2b..75c6078 100644
> --- a/profiles/input/hog-lib.c
> +++ b/profiles/input/hog-lib.c
> @@ -704,38 +704,6 @@ static void forward_report(struct uhid_event *ev, void *user_data)
> data, size, NULL, NULL);
> }
>
> -static void get_feature(struct uhid_event *ev, void *user_data)
> -{
> - struct bt_hog *hog = user_data;
> - struct report *report;
> - struct uhid_event rsp;
> - int err;
> -
> - memset(&rsp, 0, sizeof(rsp));
> - rsp.type = UHID_FEATURE_ANSWER;
> - rsp.u.feature_answer.id = ev->u.feature.id;
> -
> - report = find_report_by_rtype(hog, ev->u.feature.rtype,
> - ev->u.feature.rnum);
> - if (!report) {
> - rsp.u.feature_answer.err = ENOTSUP;
> - goto done;
> - }
> -
> - if (!report->value) {
> - rsp.u.feature_answer.err = EIO;
> - goto done;
> - }
> -
> - rsp.u.feature_answer.size = report->len;
> - memcpy(rsp.u.feature_answer.data, report->value, report->len);
> -
> -done:
> - err = bt_uhid_send(hog->uhid, &rsp);
> - if (err < 0)
> - error("bt_uhid_send: %s", strerror(-err));
> -}
> -
> static void set_report_cb(guint8 status, const guint8 *pdu,
> guint16 plen, gpointer user_data)
> {
> @@ -1034,7 +1002,6 @@ static void report_map_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
> }
>
> bt_uhid_register(hog->uhid, UHID_OUTPUT, forward_report, hog);
> - bt_uhid_register(hog->uhid, UHID_FEATURE, get_feature, hog);
> bt_uhid_register(hog->uhid, UHID_GET_REPORT, get_report, hog);
> bt_uhid_register(hog->uhid, UHID_SET_REPORT, set_report, hog);
>
> --
> 1.9.1
Applied, thanks.
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
