HID: hyperv: Replace one-element array with flexible-array member

PATCH Applied Paulo Miguel Almeida 4 participants 5 messages Oct 17, 2022 – Oct 18, 2022

Subsystems (3)

HID Hyper-V/Azure core and drivers Microsoft MANA RDMA driver

lore

Share & tools

lore

Paulo Miguel Almeida

author

2022-10-17 07:51 UTC #0 lore raw

PATCH 1/1 HID: hyperv: Replace one-element array with flexible-array member

One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in structs synthhid_msg, synthhid_input_report, pipe_prt_msg and refactor the rest of the code accordingly.

This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1].

Link https://github.com/KSPP/linux/issues/79

Link https://github.com/KSPP/linux/issues/210

Link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]

Signed-off-by Paulo Miguel Almeida <[email protected]>

---
 drivers/hid/hid-hyperv.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

drivers/hid/hid-hyperv.c +7 ~~-10~~

index e0bc73124196..208cf8d981a5 100644
@@ -61,7 +61,7 @@ struct synthhid_msg_hdr {
 
 struct synthhid_msg {
 	struct synthhid_msg_hdr header;
-	char data[1]; /* Enclosed message */
+	char data[]; /* Enclosed message */
 };
 
 union synthhid_version {

@@ -99,7 +99,7 @@ struct synthhid_device_info_ack {
 
 struct synthhid_input_report {
 	struct synthhid_msg_hdr header;
-	char buffer[1];
+	char buffer[];
 };
 
 #pragma pack(pop)

@@ -118,7 +118,7 @@ enum pipe_prot_msg_type {
 struct pipe_prt_msg {
 	enum pipe_prot_msg_type type;
 	u32 size;
-	char data[1];
+	char data[];
 };
 
 struct  mousevsc_prt_msg {

@@ -232,7 +232,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
 
 	ret = vmbus_sendpacket(input_device->device->channel,
 			&ack,
-			sizeof(struct pipe_prt_msg) - sizeof(unsigned char) +
+			sizeof(struct pipe_prt_msg) +
 			sizeof(struct synthhid_device_info_ack),
 			(unsigned long)&ack,
 			VM_PKT_DATA_INBAND,

@@ -271,16 +271,14 @@ static void mousevsc_on_receive(struct hv_device *device,
 		 * malicious/buggy hypervisor/host, add a check here to
 		 * ensure we don't corrupt memory.
 		 */
-		if ((pipe_msg->size + sizeof(struct pipe_prt_msg)
-			- sizeof(unsigned char))
+		if (struct_size(pipe_msg, data, pipe_msg->size)
 			> sizeof(struct mousevsc_prt_msg)) {
 			WARN_ON(1);
 			break;
 		}
 
 		memcpy(&input_dev->protocol_resp, pipe_msg,
-		       pipe_msg->size + sizeof(struct pipe_prt_msg) -
-		       sizeof(unsigned char));
+				struct_size(pipe_msg, data, pipe_msg->size));
 		complete(&input_dev->wait_event);
 		break;

@@ -359,8 +357,7 @@ static int mousevsc_connect_to_vsp(struct hv_device *device)
 	request->request.version_requested.version = SYNTHHID_INPUT_VERSION;
 
 	ret = vmbus_sendpacket(device->channel, request,
-				sizeof(struct pipe_prt_msg) -
-				sizeof(unsigned char) +
+				sizeof(struct pipe_prt_msg) +
 				sizeof(struct synthhid_protocol_request),
 				(unsigned long)request,
 				VM_PKT_DATA_INBAND,

Signature · 1 line

2.37.3

Benjamin Tissoires

Reviewed-by on v1 1/1

2022-10-17 08:31 UTC #1 lore raw

Quoted 10 lines · Paulo Miguel Almeida

One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in structs synthhid_msg, synthhid_input_report, pipe_prt_msg and refactor the rest of the code accordingly.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1].

Quoted 6 lines

Link https://github.com/KSPP/linux/issues/79
Link https://github.com/KSPP/linux/issues/210
Link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Signed-off-by Paulo Miguel Almeida <[email protected]>
---

FWIW, this is

Reviewed-by Benjamin Tissoires <[email protected]>

Couple of questions though:
- do the hyperv want to take this patch through their tree or should I (I plan on sending some fixes for 6.1-rc2 this week, so I can piggy back this one)
- on the driver itself, please see inline:

Quoted 8 lines

 drivers/hid/hid-hyperv.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

drivers/hid/hid-hyperv.c +0 -0

index e0bc73124196..208cf8d981a5 100644

drivers/hid/hid-hyperv.c struct synthhid_msg_hdr { · line 65

Quoted 6 lines

drivers/hid/hid-hyperv.c +1 -1

@@ -61,7 +61,7 @@ struct synthhid_msg_hdr {
 
 struct synthhid_msg {
        struct synthhid_msg_hdr header;
-       char data[1]; /* Enclosed message */
+       char data[]; /* Enclosed message */
 };

IMO that struct has no real use. We just use it in mousevsc_on_receive() to dereference the first field only, the header. So how about we have a followup cleanup patch that just removes it and in mousevsc_on_receive() we convert those usages directly to struct synthhid_msg_hdr?

Cheers,
Benjamin

Quoted 61 lines

drivers/hid/hid-hyperv.c +6 ~~-10~~

 union synthhid_version {

@@ -99,7 +99,7 @@ struct synthhid_device_info_ack {
 
 struct synthhid_input_report {
        struct synthhid_msg_hdr header;
-       char buffer[1];
+       char buffer[];
 };
 
 #pragma pack(pop)

@@ -118,7 +118,7 @@ enum pipe_prot_msg_type {
 struct pipe_prt_msg {
        enum pipe_prot_msg_type type;
        u32 size;
-       char data[1];
+       char data[];
 };
 
 struct  mousevsc_prt_msg {

@@ -232,7 +232,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
 
        ret = vmbus_sendpacket(input_device->device->channel,
                        &ack,
-                       sizeof(struct pipe_prt_msg) - sizeof(unsigned char) +
+                       sizeof(struct pipe_prt_msg) +
                        sizeof(struct synthhid_device_info_ack),
                        (unsigned long)&ack,
                        VM_PKT_DATA_INBAND,

@@ -271,16 +271,14 @@ static void mousevsc_on_receive(struct hv_device *device,
                 * malicious/buggy hypervisor/host, add a check here to
                 * ensure we don't corrupt memory.
                 */
-               if ((pipe_msg->size + sizeof(struct pipe_prt_msg)
-                       - sizeof(unsigned char))
+               if (struct_size(pipe_msg, data, pipe_msg->size)
                        > sizeof(struct mousevsc_prt_msg)) {
                        WARN_ON(1);
                        break;
                }
 
                memcpy(&input_dev->protocol_resp, pipe_msg,
-                      pipe_msg->size + sizeof(struct pipe_prt_msg) -
-                      sizeof(unsigned char));
+                               struct_size(pipe_msg, data, pipe_msg->size));
                complete(&input_dev->wait_event);
                break;

@@ -359,8 +357,7 @@ static int mousevsc_connect_to_vsp(struct hv_device *device)
        request->request.version_requested.version = SYNTHHID_INPUT_VERSION;
 
        ret = vmbus_sendpacket(device->channel, request,
-                               sizeof(struct pipe_prt_msg) -
-                               sizeof(unsigned char) +
+                               sizeof(struct pipe_prt_msg) +
                                sizeof(struct synthhid_protocol_request),
                                (unsigned long)request,
                                VM_PKT_DATA_INBAND,
--

2.37.3

Paulo Miguel Almeida

author

2022-10-17 09:30 UTC #2 lore raw

Quoted 16 lines · Benjamin Tissoires

Quoted 14 lines · Paulo Miguel Almeida
One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in structs synthhid_msg, synthhid_input_report, pipe_prt_msg and refactor the rest of the code accordingly.
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1].
Link https://github.com/KSPP/linux/issues/79
Link https://github.com/KSPP/linux/issues/210
Link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]

Quoted 6 lines

Quoted 2 lines
Signed-off-by Paulo Miguel Almeida <[email protected]>
---
FWIW, this is
Reviewed-by Benjamin Tissoires <[email protected]>

Thanks for reviewing this patch :-)

Quoted 15 lines

drivers/hid/hid-hyperv.c struct synthhid_msg_hdr { · line 65
Quoted 8 lines
+++ b/drivers/hid/hid-hyperv.c
drivers/hid/hid-hyperv.c +1 -1
@@ -61,7 +61,7 @@ struct synthhid_msg_hdr {
 
 struct synthhid_msg {
        struct synthhid_msg_hdr header;
-       char data[1]; /* Enclosed message */
+       char data[]; /* Enclosed message */
 };
IMO that struct has no real use. We just use it in mousevsc_on_receive() to dereference the first field only, the header. So how about we have a followup cleanup patch that just removes it and in mousevsc_on_receive() we convert those usages directly to struct synthhid_msg_hdr?

Quoted 2 lines

Cheers,
Benjamin

I'm happy to send a followup cleanup patch for that.

Paulo A.

Michael Kelley (LINUX)

Reviewed-by on v1 1/1

2022-10-17 15:15 UTC #3 lore raw

In reply to Paulo Miguel Almeida

From: Paulo Miguel Almeida <[email protected]> Sent: Monday, October 17, 2022 12:51 AM

Quoted 90 lines

This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1].

Link https://github.com/KSPP/linux/issues/79

Link https://github.com/KSPP/linux/issues/210

Link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]

Signed-off-by Paulo Miguel Almeida <[email protected]>

---
 drivers/hid/hid-hyperv.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

drivers/hid/hid-hyperv.c +4 -4

index e0bc73124196..208cf8d981a5 100644
@@ -61,7 +61,7 @@ struct synthhid_msg_hdr {
 
 struct synthhid_msg {
 	struct synthhid_msg_hdr header;
-	char data[1]; /* Enclosed message */
+	char data[]; /* Enclosed message */
 };
 
 union synthhid_version {

@@ -99,7 +99,7 @@ struct synthhid_device_info_ack {
 
 struct synthhid_input_report {
 	struct synthhid_msg_hdr header;
-	char buffer[1];
+	char buffer[];
 };
 
 #pragma pack(pop)

@@ -118,7 +118,7 @@ enum pipe_prot_msg_type {
 struct pipe_prt_msg {
 	enum pipe_prot_msg_type type;
 	u32 size;
-	char data[1];
+	char data[];
 };
 
 struct  mousevsc_prt_msg {

@@ -232,7 +232,7 @@ static void mousevsc_on_receive_device_info(struct
 mousevsc_dev *input_device,
 
 	ret = vmbus_sendpacket(input_device->device->channel,
 			&ack,
-			sizeof(struct pipe_prt_msg) - sizeof(unsigned char) +
+			sizeof(struct pipe_prt_msg) +
 			sizeof(struct synthhid_device_info_ack),
 			(unsigned long)&ack,

drivers/hid/hid-hyperv.c +3 -7

 			VM_PKT_DATA_INBAND,

@@ -271,16 +271,14 @@ static void mousevsc_on_receive(struct hv_device *device,
 		 * malicious/buggy hypervisor/host, add a check here to
 		 * ensure we don't corrupt memory.
 		 */
-		if ((pipe_msg->size + sizeof(struct pipe_prt_msg)
-			- sizeof(unsigned char))
+		if (struct_size(pipe_msg, data, pipe_msg->size)
 			> sizeof(struct mousevsc_prt_msg)) {
 			WARN_ON(1);
 			break;
 		}
 
 		memcpy(&input_dev->protocol_resp, pipe_msg,
-		       pipe_msg->size + sizeof(struct pipe_prt_msg) -
-		       sizeof(unsigned char));
+				struct_size(pipe_msg, data, pipe_msg->size));
 		complete(&input_dev->wait_event);
 		break;

@@ -359,8 +357,7 @@ static int mousevsc_connect_to_vsp(struct hv_device *device)
 	request->request.version_requested.version = SYNTHHID_INPUT_VERSION;
 
 	ret = vmbus_sendpacket(device->channel, request,
-				sizeof(struct pipe_prt_msg) -
-				sizeof(unsigned char) +
+				sizeof(struct pipe_prt_msg) +
 				sizeof(struct synthhid_protocol_request),
 				(unsigned long)request,
 				VM_PKT_DATA_INBAND,
--

Quoted 1 line

2.37.3

Reviewed-by Michael Kelley <[email protected]>

Jiri Kosina

maintainer Applied

2022-10-18 12:56 UTC #4 lore raw

Quoted 81 lines · Michael Kelley (LINUX)

Quoted 81 lines

This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1].

Link https://github.com/KSPP/linux/issues/79

Link https://github.com/KSPP/linux/issues/210

Link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]

Signed-off-by Paulo Miguel Almeida <[email protected]>

---
 drivers/hid/hid-hyperv.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

drivers/hid/hid-hyperv.c +4 -4

index e0bc73124196..208cf8d981a5 100644
@@ -61,7 +61,7 @@ struct synthhid_msg_hdr {
 
 struct synthhid_msg {
 	struct synthhid_msg_hdr header;
-	char data[1]; /* Enclosed message */
+	char data[]; /* Enclosed message */
 };
 
 union synthhid_version {

@@ -99,7 +99,7 @@ struct synthhid_device_info_ack {
 
 struct synthhid_input_report {
 	struct synthhid_msg_hdr header;
-	char buffer[1];
+	char buffer[];
 };
 
 #pragma pack(pop)

@@ -118,7 +118,7 @@ enum pipe_prot_msg_type {
 struct pipe_prt_msg {
 	enum pipe_prot_msg_type type;
 	u32 size;
-	char data[1];
+	char data[];
 };
 
 struct  mousevsc_prt_msg {

@@ -232,7 +232,7 @@ static void mousevsc_on_receive_device_info(struct
 mousevsc_dev *input_device,
 
 	ret = vmbus_sendpacket(input_device->device->channel,
 			&ack,
-			sizeof(struct pipe_prt_msg) - sizeof(unsigned char) +
+			sizeof(struct pipe_prt_msg) +
 			sizeof(struct synthhid_device_info_ack),
 			(unsigned long)&ack,

drivers/hid/hid-hyperv.c +2 -4

 			VM_PKT_DATA_INBAND,

@@ -271,16 +271,14 @@ static void mousevsc_on_receive(struct hv_device *device,
 		 * malicious/buggy hypervisor/host, add a check here to
 		 * ensure we don't corrupt memory.
 		 */
-		if ((pipe_msg->size + sizeof(struct pipe_prt_msg)
-			- sizeof(unsigned char))
+		if (struct_size(pipe_msg, data, pipe_msg->size)
 			> sizeof(struct mousevsc_prt_msg)) {
 			WARN_ON(1);
 			break;
 		}
 
 		memcpy(&input_dev->protocol_resp, pipe_msg,
-		       pipe_msg->size + sizeof(struct pipe_prt_msg) -
-		       sizeof(unsigned char));
+				struct_size(pipe_msg, data, pipe_msg->size));
 		complete(&input_dev->wait_event);
 		break;

@@ -359,8 +357,7 @@ static int mousevsc_connect_to_vsp(struct hv_device *device)
 	request->request.version_requested.version = SYNTHHID_INPUT_VERSION;

Quoted 11 lines

Quoted 9 lines

drivers/hid/hid-hyperv.c +1 -3

 	ret = vmbus_sendpacket(device->channel, request,
-				sizeof(struct pipe_prt_msg) -
-				sizeof(unsigned char) +
+				sizeof(struct pipe_prt_msg) +
 				sizeof(struct synthhid_protocol_request),
 				(unsigned long)request,
 				VM_PKT_DATA_INBAND,
--

2.37.3

Reviewed-by Michael Kelley <[email protected]>

Thanks, applied.

Signature · 2 lines

Jiri Kosina
SUSE Labs