Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220803225716.1287921-1-vi@endrift.com> <CABBYNZKcAZeDqjeHzTfqmvKS3Tbur-rRi6+uPL4x4t8MnWp5ug@mail.gmail.com>
 <c0f52a2d-19ff-acb2-92ac-cec87819a2cf@endrift.com>
In-Reply-To: <c0f52a2d-19ff-acb2-92ac-cec87819a2cf@endrift.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Wed, 3 Aug 2022 17:16:48 -0700
Message-ID: <CABBYNZLtWN7oWBRc-YjV6baCPpkh_J-2mUMO83A_3y=Xtn==dw@mail.gmail.com>
Subject: Re: [PATCH BlueZ] hog-lib: Increase maximum report map size
To:     Vicki Pfau <vi@endrift.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Vicki,

On Wed, Aug 3, 2022 at 5:05 PM Vicki Pfau <vi@endrift.com> wrote:
>
>
>
> On 8/3/22 16:55, Luiz Augusto von Dentz wrote:
> > Hi Vicki,
> >
> > On Wed, Aug 3, 2022 at 4:07 PM Vicki Pfau <vi@endrift.com> wrote:
> >>
> >> Though a 512 byte report map size seems plenty large, there exist some=
 devices
> >> (e.g. Brydge W-Touch) that send larger reports. There is no protocol-d=
efined
> >> maximum size so doubling the maximum size is safe, and should hopefull=
y fix
> >> most real-world failures.
> >> ---
> >>  profiles/input/hog-lib.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> >> index 4a9c60185..9f3eb428c 100644
> >> --- a/profiles/input/hog-lib.c
> >> +++ b/profiles/input/hog-lib.c
> >> @@ -64,7 +64,7 @@
> >>  #define HOG_PROTO_MODE_BOOT    0
> >>  #define HOG_PROTO_MODE_REPORT  1
> >>
> >> -#define HOG_REPORT_MAP_MAX_SIZE        512
> >> +#define HOG_REPORT_MAP_MAX_SIZE        1024
> >>  #define HID_INFO_SIZE                  4
> >>  #define ATT_NOTIFICATION_HEADER_SIZE   3
> >
> > Afaik 512 is the maximum length an attribute can have even when using
> > read long procedure:
> >
> > BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 3, Part F
> > page 1416:
> >
> > The maximum length of an attribute value shall be 512 octets.
> >
> > And
> >
> > BLUETOOTH SPECIFICATION
> > HID Service Specification
> > Page 16 of 26
> >
> > 2.6.1 Report Map Characteristic Behavior
> > The GATT Read Characteristic Value or Read Long Characteristic Values s=
ub-
> > procedures are used to read the Report Map characteristic value.
> > The length of the Report Map characteristic value is limited to 512 oct=
ets.
> >
> > So I believe the device is not compliant and very likely needs to have
> > multiple instances of HID Service instead of combining everything in a
> > single instance.
> >
> >> --
> >> 2.37.1
> >>
> >
> >
>
> Ah, that's strange. I looked through the spec but didn't see those. That =
said, while the device may be non-compliant, the device is on the market an=
d I doubt I could get them to update the firmware as a random third party. =
It works on Windows, so clearly Windows doesn't have a problem with its non=
compliance. So this raises the question, how should Linux handle non-compli=
ant hardware, especially when it could easily be made to work just by bendi=
ng the rules in this one instance? I can absolutely change the commit messa=
ge since it's erroneous, but the question then comes down to how should it =
be handled at all.

While I agree this could be worked around it is probably worth
checking with the manufacturer if it is aware of the problem because
even if we were to allow reading past 512 bytes offset in the future
there may be qualification tests enforcing not to do so, besides
versions up to BlueZ 5.65 would still not work anyway so I thing
letting the manufacturer know there is a problem with their
implementation is actually worth a shot here.

--=20
Luiz Augusto von Dentz