linux-hardening.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Johnson <quic_jjohnson@quicinc.com>
To: "Gustavo A. R. Silva" <gustavoars@kernel.org>,
	<linux-wireless@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: Kalle Valo <kvalo@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, <netdev@vger.kernel.org>,
	<linux-hardening@vger.kernel.org>
Subject: Re: [PATCH 5/6][next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_disconnect_event
Date: Wed, 23 Feb 2022 16:53:45 -0800	[thread overview]
Message-ID: <03cee2a7-1455-b788-e1f0-5fb48db3478c@quicinc.com> (raw)
In-Reply-To: <4a42b591109202589cb1cf87df13daef02eb75f9.1645583264.git.gustavoars@kernel.org>

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;
>   
>   /*


  reply	other threads:[~2022-02-24  2:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-23  2:38 [PATCH 0/6][next] ath6kl: wmi: Replace one-element arrays with flexible-array members Gustavo A. R. Silva
2022-02-23  2:38 ` [PATCH 1/6][next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_begin_scan_cmd Gustavo A. R. Silva
2022-02-24  0:48   ` Jeff Johnson
2022-02-23  2:38 ` [PATCH 2/6][next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_start_scan_cmd Gustavo A. R. Silva
2022-02-23  2:38 ` [PATCH 3/6][next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_channel_list_reply Gustavo A. R. Silva
2022-02-24  0:49   ` Jeff Johnson
2022-02-23  2:38 ` [PATCH 4/6][next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_connect_event Gustavo A. R. Silva
2022-02-24  0:50   ` Jeff Johnson
2022-02-24 20:08     ` Gustavo A. R. Silva
2022-02-23  2:39 ` [PATCH 5/6][next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_disconnect_event Gustavo A. R. Silva
2022-02-24  0:53   ` Jeff Johnson [this message]
2022-02-23  2:39 ` [PATCH 6/6][next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_aplist_event Gustavo A. R. Silva
2022-02-24  1:01   ` Jeff Johnson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=03cee2a7-1455-b788-e1f0-5fb48db3478c@quicinc.com \
    --to=quic_jjohnson@quicinc.com \
    --cc=davem@davemloft.net \
    --cc=gustavoars@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kvalo@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH 5/6][next] ath6kl: wmi: Replace one-element array with flexible-array member in struct wmi_disconnect_event' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).