* [PATCH] [next] HID: hyperv: Replace one-element array with flexible-array member
@ 2022-10-17 7:51 Paulo Miguel Almeida
2022-10-17 8:30 ` Benjamin Tissoires
2022-10-17 15:15 ` Michael Kelley (LINUX)
0 siblings, 2 replies; 5+ messages in thread
From: Paulo Miguel Almeida @ 2022-10-17 7:51 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Jiri Kosina, Benjamin Tissoires, linux-hyperv,
linux-input
Cc: linux-kernel, linux-hardening, paulo.miguel.almeida.rodenas
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 <paulo.miguel.almeida.rodenas@gmail.com>
---
drivers/hid/hid-hyperv.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index e0bc73124196..208cf8d981a5 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -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,
--
2.37.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] [next] HID: hyperv: Replace one-element array with flexible-array member
2022-10-17 7:51 [PATCH] [next] HID: hyperv: Replace one-element array with flexible-array member Paulo Miguel Almeida
@ 2022-10-17 8:30 ` Benjamin Tissoires
2022-10-17 9:30 ` Paulo Miguel Almeida
2022-10-17 15:15 ` Michael Kelley (LINUX)
1 sibling, 1 reply; 5+ messages in thread
From: Benjamin Tissoires @ 2022-10-17 8:30 UTC (permalink / raw)
To: Paulo Miguel Almeida
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Jiri Kosina, linux-hyperv, linux-input, linux-kernel,
linux-hardening
On Mon, Oct 17, 2022 at 9:51 AM Paulo Miguel Almeida
<paulo.miguel.almeida.rodenas@gmail.com> wrote:
>
> 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 <paulo.miguel.almeida.rodenas@gmail.com>
> ---
FWIW, this is
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
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:
> drivers/hid/hid-hyperv.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> index e0bc73124196..208cf8d981a5 100644
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -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
>
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] [next] HID: hyperv: Replace one-element array with flexible-array member
2022-10-17 8:30 ` Benjamin Tissoires
@ 2022-10-17 9:30 ` Paulo Miguel Almeida
0 siblings, 0 replies; 5+ messages in thread
From: Paulo Miguel Almeida @ 2022-10-17 9:30 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Jiri Kosina, linux-hyperv, linux-input, linux-kernel,
linux-hardening
On Mon, Oct 17, 2022 at 10:30:54AM +0200, Benjamin Tissoires wrote:
> On Mon, Oct 17, 2022 at 9:51 AM Paulo Miguel Almeida
> <paulo.miguel.almeida.rodenas@gmail.com> wrote:
> >
> > 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 <paulo.miguel.almeida.rodenas@gmail.com>
> > ---
>
> FWIW, this is
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
Thanks for reviewing this patch :-)
> > +++ b/drivers/hid/hid-hyperv.c
> > @@ -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
I'm happy to send a followup cleanup patch for that.
Paulo A.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] [next] HID: hyperv: Replace one-element array with flexible-array member
2022-10-17 7:51 [PATCH] [next] HID: hyperv: Replace one-element array with flexible-array member Paulo Miguel Almeida
2022-10-17 8:30 ` Benjamin Tissoires
@ 2022-10-17 15:15 ` Michael Kelley (LINUX)
2022-10-18 12:56 ` Jiri Kosina
1 sibling, 1 reply; 5+ messages in thread
From: Michael Kelley (LINUX) @ 2022-10-17 15:15 UTC (permalink / raw)
To: Paulo Miguel Almeida, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, Dexuan Cui, Jiri Kosina,
Benjamin Tissoires, linux-hyperv@vger.kernel.org,
linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
From: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com> Sent: Monday, October 17, 2022 12:51 AM
>
> 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 <paulo.miguel.almeida.rodenas@gmail.com>
> ---
> drivers/hid/hid-hyperv.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> index e0bc73124196..208cf8d981a5 100644
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -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,
> --
> 2.37.3
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] [next] HID: hyperv: Replace one-element array with flexible-array member
2022-10-17 15:15 ` Michael Kelley (LINUX)
@ 2022-10-18 12:56 ` Jiri Kosina
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Kosina @ 2022-10-18 12:56 UTC (permalink / raw)
To: Michael Kelley (LINUX)
Cc: Paulo Miguel Almeida, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, Dexuan Cui, Benjamin Tissoires,
linux-hyperv@vger.kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
On Mon, 17 Oct 2022, Michael Kelley (LINUX) wrote:
> > 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 <paulo.miguel.almeida.rodenas@gmail.com>
> > ---
> > drivers/hid/hid-hyperv.c | 17 +++++++----------
> > 1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> > index e0bc73124196..208cf8d981a5 100644
> > --- a/drivers/hid/hid-hyperv.c
> > +++ b/drivers/hid/hid-hyperv.c
> > @@ -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,
> > --
> > 2.37.3
>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Thanks, applied.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-18 12:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-17 7:51 [PATCH] [next] HID: hyperv: Replace one-element array with flexible-array member Paulo Miguel Almeida
2022-10-17 8:30 ` Benjamin Tissoires
2022-10-17 9:30 ` Paulo Miguel Almeida
2022-10-17 15:15 ` Michael Kelley (LINUX)
2022-10-18 12:56 ` Jiri Kosina
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).