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> <CABBYNZLtWN7oWBRc-YjV6baCPpkh_J-2mUMO83A_3y=Xtn==dw@mail.gmail.com>
 <dbeb83e5-9986-ca5d-e71f-cdaa579794bb@endrift.com> <CABBYNZLHFAj4LwaMTEvCmysF2D__xrhJT9ORY-qmm_XYcGjjtQ@mail.gmail.com>
In-Reply-To: <CABBYNZLHFAj4LwaMTEvCmysF2D__xrhJT9ORY-qmm_XYcGjjtQ@mail.gmail.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Mon, 15 Aug 2022 11:09:41 -0700
Message-ID: <CABBYNZKbCcz7p+EaNpWZkuX77BJ-DsoT=BRUQGwJjAhTesLhXA@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 10, 2022 at 1:13 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Vicki,
>
> On Tue, Aug 9, 2022 at 7:02 PM Vicki Pfau <vi@endrift.com> wrote:
> >
> >
> >
> > On 8/3/22 17:16, Luiz Augusto von Dentz wrote:
> > > 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 protoc=
ol-defined
> > >>>> maximum size so doubling the maximum size is safe, and should hope=
fully 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 usi=
ng
> > >>> 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 Valu=
es sub-
> > >>> procedures are used to read the Report Map characteristic value.
> > >>> The length of the Report Map characteristic value is limited to 512=
 octets.
> > >>>
> > >>> So I believe the device is not compliant and very likely needs to h=
ave
> > >>> multiple instances of HID Service instead of combining everything i=
n 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 mark=
et and I doubt I could get them to update the firmware as a random third pa=
rty. It works on Windows, so clearly Windows doesn't have a problem with it=
s noncompliance. So this raises the question, how should Linux handle non-c=
ompliant hardware, especially when it could easily be made to work just by =
bending the rules in this one instance? I can absolutely change the commit =
message since it's erroneous, but the question then comes down to how shoul=
d 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.
> > >
> > Brydge replied with the standard tech support "this is only supported o=
n Windows, so there probably won't be a firmware update" reply, despite its=
 noncompliance. And since I doubt Windows will add a change to limit it, we=
ll, that kind of limits our options here to either "enforce compliance and =
break non-compliant hardware" or "figure out a way to bend the rules". Give=
n that BlueZ, upon expanding the maximum size, does successfully read the o=
verly-long report map (it does use the read blob with offset message to get=
 the last several bytes), it does work as intended if we ignore that specif=
ic rule. Though obviously that's up to the bluetooth maintainers to solve, =
so at this point I'm just tossing my two cents at it.
>
> Well we can do what Windows is doing but let's have it documented
> then, we could as well scalate this with Bluetooth SIG since the
> manufacturer seem to be intentionally creating interoperability
> problems with a standard service, but that would likely take a lot
> more time and most likely will need to be resolved by introducing a
> test that enforces that HoG client don't attempt to read past 512
> bytes offset.
>
> Btw, if there is no limitation on how big the report map can get in
> HID, I'd leave it up to the kernel to figure out if that is acceptable
> or not, so I'd remove the 1024.

Are you still planning to update this change? Btw, how about Android,
does it handle such devices?

--=20
Luiz Augusto von Dentz