diff mbox series

[5/6,next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_disconnect_event

Message ID 4a42b591109202589cb1cf87df13daef02eb75f9.1645583264.git.gustavoars@kernel.org
State New
Headers show
Series ath6kl: wmi: Replace one-element arrays with flexible-array members | expand

Commit Message

Gustavo A. R. Silva Feb. 23, 2022, 2:39 a.m. UTC
Replace one-element array with flexible-array member in struct
wmi_disconnect_event.

It's also worth noting that due to the flexible array transformation,
the size of struct wmi_disconnect_event changed (now the size is 1 byte
smaller), and in order to preserve the logic of before the transformation,
the following change is needed:

        -       if (len < sizeof(struct wmi_disconnect_event))
        +       if (len <= sizeof(struct wmi_disconnect_event))

This issue was found with the help of Coccinelle and audited and fixed,
manually.

Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
Link: https://github.com/KSPP/linux/issues/79
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Hi!

It'd be great if someone can confirm or comment on the following
changes described in the changelog text:

        -       if (len < sizeof(struct wmi_disconnect_event))
        +       if (len <= sizeof(struct wmi_disconnect_event))

Thanks

 drivers/net/wireless/ath/ath6kl/wmi.c | 2 +-
 drivers/net/wireless/ath/ath6kl/wmi.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Jeff Johnson Feb. 24, 2022, 12:53 a.m. UTC | #1
On 2/22/2022 6:39 PM, Gustavo A. R. Silva wrote:
> Replace one-element array with flexible-array member in struct
> wmi_disconnect_event.
> 
> It's also worth noting that due to the flexible array transformation,
> the size of struct wmi_disconnect_event changed (now the size is 1 byte
> smaller), and in order to preserve the logic of before the transformation,
> the following change is needed:
> 
>          -       if (len < sizeof(struct wmi_disconnect_event))
>          +       if (len <= sizeof(struct wmi_disconnect_event))
> 
> This issue was found with the help of Coccinelle and audited and fixed,
> manually.
> 
> Link: https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays
> Link: https://github.com/KSPP/linux/issues/79
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> Hi!
> 
> It'd be great if someone can confirm or comment on the following
> changes described in the changelog text:
> 
>          -       if (len < sizeof(struct wmi_disconnect_event))
>          +       if (len <= sizeof(struct wmi_disconnect_event))
> 
> Thanks
> 
>   drivers/net/wireless/ath/ath6kl/wmi.c | 2 +-
>   drivers/net/wireless/ath/ath6kl/wmi.h | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
> index ccdccead688e..645fb6cae3be 100644
> --- a/drivers/net/wireless/ath/ath6kl/wmi.c
> +++ b/drivers/net/wireless/ath/ath6kl/wmi.c
> @@ -1023,7 +1023,7 @@ static int ath6kl_wmi_disconnect_event_rx(struct wmi *wmi, u8 *datap, int len,
>   	struct wmi_disconnect_event *ev;
>   	wmi->traffic_class = 100;
>   
> -	if (len < sizeof(struct wmi_disconnect_event))
> +	if (len <= sizeof(struct wmi_disconnect_event))

this is another case where I believe the original code should remain 
since that is checking that the "fixed" portion is present.

and here again what is missing in the original code is logic to verify 
that the provide len is large enough to handle the advertised assoc_resp_len

>   		return -EINVAL;
>   
>   	ev = (struct wmi_disconnect_event *) datap;
> diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
> index 6b064e669d87..6a7fc07cd9aa 100644
> --- a/drivers/net/wireless/ath/ath6kl/wmi.h
> +++ b/drivers/net/wireless/ath/ath6kl/wmi.h
> @@ -1596,7 +1596,7 @@ struct wmi_disconnect_event {
>   	u8 disconn_reason;
>   
>   	u8 assoc_resp_len;
> -	u8 assoc_info[1];
> +	u8 assoc_info[];
>   } __packed;
>   
>   /*
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index ccdccead688e..645fb6cae3be 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -1023,7 +1023,7 @@  static int ath6kl_wmi_disconnect_event_rx(struct wmi *wmi, u8 *datap, int len,
 	struct wmi_disconnect_event *ev;
 	wmi->traffic_class = 100;
 
-	if (len < sizeof(struct wmi_disconnect_event))
+	if (len <= sizeof(struct wmi_disconnect_event))
 		return -EINVAL;
 
 	ev = (struct wmi_disconnect_event *) datap;
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index 6b064e669d87..6a7fc07cd9aa 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -1596,7 +1596,7 @@  struct wmi_disconnect_event {
 	u8 disconn_reason;
 
 	u8 assoc_resp_len;
-	u8 assoc_info[1];
+	u8 assoc_info[];
 } __packed;
 
 /*