1. Patchew
  2. linux
  3. View series

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

Jiri Kosina posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/hid/hid-hyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] HID: hyperv: make pointer arithmetics understandable for FORTIFY_SOURCE
Posted by Jiri Kosina 1 month, 1 week ago 
From: Jiri Kosina <jkosina@suse.com>

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@suse.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
---
 drivers/hid/hid-hyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index 6579bd19da13..4a10b24da9fa 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -687,7 +687,7 @@ static void mousevsc_device_info_valid_descriptor(struct kunit *test)
 
 	info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
 	info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(4);
-	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
+	report = (u8 *)info + offsetof(struct synthhid_device_info, hid_descriptor) + info->hid_descriptor.bLength;
 	memset(report, 0x42, 4);
 
 	mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 4);
@@ -713,7 +713,7 @@ static void mousevsc_device_info_report_desc_oob(struct kunit *test)
 
 	info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
 	info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(64);
-	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
+	report = (u8 *)info + offsetof(struct synthhid_device_info, hid_descriptor) + info->hid_descriptor.bLength;
 	memset(report, 0x42, 8);
 
 	mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 8);

-- 
Jiri Kosina
SUSE Labs
[PATCH v2] HID: hyperv: make pointer arithmetics understandable for FORTIFY_SOURCE
Posted by Jiri Kosina 1 month, 1 week ago 
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@suse.com>
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@suse.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
---
 drivers/hid/hid-hyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index 6579bd19da13..cfc061dbdd24 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -687,7 +687,7 @@ static void mousevsc_device_info_valid_descriptor(struct kunit *test)
 
 	info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
 	info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(4);
-	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
+	report = (u8 *)(info + 1);
 	memset(report, 0x42, 4);
 
 	mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 4);
@@ -713,7 +713,7 @@ static void mousevsc_device_info_report_desc_oob(struct kunit *test)
 
 	info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
 	info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(64);
-	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
+	report = (u8 *)(info + 1);
 	memset(report, 0x42, 8);
 
 	mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 8);

-- 
Jiri Kosina
SUSE Labs
Re: [PATCH v2] HID: hyperv: make pointer arithmetics understandable for FORTIFY_SOURCE
Posted by Juergen Gross 1 month, 1 week ago 
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@suse.com>
> 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@suse.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.com>

Tested-by: Jürgen Groß <jgross@suse.com>


Jürgen
Re: [PATCH v2] HID: hyperv: make pointer arithmetics understandable for FORTIFY_SOURCE
Posted by Jiri Kosina 1 month, 1 week ago 
On Fri, 21 Aug 2026, Juergen Gross wrote:

> Tested-by: Jürgen Groß <jgross@suse.com>

Thanks, now applied.

-- 
Jiri Kosina
SUSE Labs
Re: [PATCH] HID: hyperv: make pointer arithmetics understandable for FORTIFY_SOURCE
Posted by Jürgen Groß 1 month, 1 week ago 
On 21.08.26 12:23, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.com>
> 
> 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@suse.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.com>

Tested-by: Jürgen Groß <jgross@suse.com>


Jürgen

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.