Re: [PATCH v2] HID: hyperv: make pointer arithmetics understandable for FORTIFY_SOURCE

[Juergen Gross]

On 21.08.26 15:39, Jiri Kosina wrote:
> On Fri, 21 Aug 2026, Benjamin Tissoires wrote:
>
> > > -	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
> > > +	report = (u8 *)info + offsetof(struct synthhid_device_info, hid_descriptor) + info->hid_descriptor.bLength;
> >
> > Isn't that overcomplicated?:
> >
> > Above we have:
> > struct synthhid_device_info {
> > 	struct synthhid_msg_hdr header;
> > 	struct hv_input_dev_info hid_dev_info;
> > 	struct hid_descriptor hid_descriptor;
> > };
> >    ...
> > 	struct synthhid_device_info *info;
> > 	...
> > 	info = kunit_kzalloc(test, sizeof(*info) + 4, GFP_KERNEL);
> >
> > so info is 0 allocated with the sizeof(struct synthhid_device_info) plus
> > 4 for the report (immediately after).
> >
> > with the bLength being set a couple of lines above, we basically have:
> > + report = (u8 *)info + offsetof(struct synthhid_device_info, hid_descriptor) + sizeof(struct hid_descriptor);
> >
> > So pointer address + offset of the last field + size of the last field.
> >
> > Isn't that equivalent to:
> >    report = (u8 *)info + sizeof(*info);
> >
> > or even: `(u8 *)(info + 1)`?
>
> I considered that one as well, but then I decided to keep what the
> original author intended as perhaps it's more self-explanatory what we're
> actually doing. But the argument that whenever a new field is added after
> hid_descriptor, this will silently break, is convicing enough :)
>
> Thanks. v2 below.
>
> Juergen, could you please re-Ack your Tested-by: on this? For some reason,
> my allyesconfig with gcc-15 doesn't trigger it.
>
>
>
>
> From: Jiri Kosina <jkosina@xxxxxxxx>
> Subject: [PATCH] HID: hyperv: make pointer arithmetics understandable for FORTIFY_SOURCE
>
> Commit 83df7b5fa6735b5084ecd2 ("HID: hyperv: add KUnit coverage for device info
> bounds") introduced this piece of code
>
> 	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
> 	memset(report, 0x42, 4);
>
> to populate the report descriptor, making use of the fact that the report
> &info->hid_descriptor points to a struct hid_descriptor (which is a fixed-size struct).
> GCC's FORTIFY_SOURCE infer the object size from that specific struct field rather than the
> outer dynamically allocated info buffer. As a result, writing past sizeof(struct hid_descriptor)
> triggers the __write_overflow_field warning.
>
> Calculate the pointer offset using info directly, so the compiler evaluates the
> memory bounds against the allocated flexible layout of struct
> synthhid_device_info instead of the nested struct.
>
> Fixes: 83df7b5fa6735b5084ecd2 ("HID: hyperv: add KUnit coverage for device info bounds")
> Reported-by: Jürgen Groß <jgross@xxxxxxxx>
> Signed-off-by: Jiri Kosina <jkosina@xxxxxxxx>

Tested-by: Jürgen Groß <jgross@xxxxxxxx>


Jürgen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature