All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org,
	Johannes Berg <johannes.berg@intel.com>,
	stable@vger.kernel.org, Sid Hayn <sidhayn@gmail.com>
Subject: Re: [PATCH] mac80211: fix radiotap header generation
Date: Tue, 9 Nov 2021 09:17:15 -0800	[thread overview]
Message-ID: <202111090915.F881B406B@keescook> (raw)
In-Reply-To: <20211109100203.c61007433ed6.I1dade57aba7de9c4f48d68249adbae62636fd98c@changeid>

On Tue, Nov 09, 2021 at 10:02:04AM +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> In commit 8c89f7b3d3f2 ("mac80211: Use flex-array for radiotap header
> bitmap") we accidentally pointed the position to the wrong place, so
> we overwrite a present bitmap, and thus cause all kinds of trouble.
> 
> To see the issue, note that the previous code read:
> 
>   pos = (void *)(it_present + 1);
> 
> The requirement now is that we need to calculate pos via it_optional,
> to not trigger the compiler hardening checks, as:
> 
>   pos = (void *)&rthdr->it_optional[...];
> 
> Rewriting the original expression, we get (obviously, since that just
> adds "+ x - x" terms):
> 
>   pos = (void *)(it_present + 1 + rthdr->it_optional - rthdr->it_optional)
> 
> and moving the "+ rthdr->it_optional" outside to be used as an array:
> 
>   pos = (void *)&rthdr->it_optional[it_present + 1 - rthdr->it_optional];
> 
> The original is off by one, fix it.
> 
> Cc: stable@vger.kernel.org
> Fixes: 8c89f7b3d3f2 ("mac80211: Use flex-array for radiotap header bitmap")
> Reported-by: Sid Hayn <sidhayn@gmail.com>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Argh! Thank you for the fix and sorry for the trouble the earlier patch
created!

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  net/mac80211/rx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index fc5c608d02e2..3562730ea0f8 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -364,7 +364,7 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
>  	 * the compiler to think we have walked past the end of the
>  	 * struct member.
>  	 */
> -	pos = (void *)&rthdr->it_optional[it_present - rthdr->it_optional];
> +	pos = (void *)&rthdr->it_optional[it_present + 1 - rthdr->it_optional];
>  
>  	/* the order of the following fields is important */
>  
> -- 
> 2.31.1
> 

-- 
Kees Cook

  parent reply	other threads:[~2021-11-09 17:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-09  9:02 [PATCH] mac80211: fix radiotap header generation Johannes Berg
2021-11-09 16:13 ` Sid Hayn
2021-11-09 17:17 ` Kees Cook [this message]
  -- strict thread matches above, loose matches on Subject: below --
2009-10-28  8:58 Johannes Berg

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=202111090915.F881B406B@keescook \
    --to=keescook@chromium.org \
    --cc=johannes.berg@intel.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=sidhayn@gmail.com \
    --cc=stable@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.