2012-10-25 19:56:19

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] wireless: drop invalid mesh address extension frames

From: Johannes Berg <[email protected]>

The mesh header can have address extension by a 4th
or a 5th and 6th address, but never both. Drop such
frames in 802.11 -> 802.3 conversion.

Cc: [email protected]
Signed-off-by: Johannes Berg <[email protected]>
---
net/wireless/util.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/wireless/util.c b/net/wireless/util.c
index ef35f4e..7296ec0 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -312,18 +312,15 @@ EXPORT_SYMBOL(ieee80211_get_hdrlen_from_skb);
static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr)
{
int ae = meshhdr->flags & MESH_FLAGS_AE;
- /* 7.1.3.5a.2 */
+ /* IEEE 802.11 - 8.2.4.7.3 */
switch (ae) {
+ default:
case 0:
return 6;
case MESH_FLAGS_AE_A4:
return 12;
case MESH_FLAGS_AE_A5_A6:
return 18;
- case (MESH_FLAGS_AE_A4 | MESH_FLAGS_AE_A5_A6):
- return 24;
- default:
- return 6;
}
}

@@ -373,6 +370,11 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
/* make sure meshdr->flags is on the linear part */
if (!pskb_may_pull(skb, hdrlen + 1))
return -1;
+ /* reserved */
+ if ((meshdr->flags & MESH_FLAGS_AE) ==
+ (MESH_FLAGS_AE_A4 |
+ MESH_FLAGS_AE_A5_A6))
+ return -1;
if (meshdr->flags & MESH_FLAGS_AE_A5_A6) {
skb_copy_bits(skb, hdrlen +
offsetof(struct ieee80211s_hdr, eaddr1),
@@ -397,6 +399,11 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
/* make sure meshdr->flags is on the linear part */
if (!pskb_may_pull(skb, hdrlen + 1))
return -1;
+ /* reserved */
+ if ((meshdr->flags & MESH_FLAGS_AE) ==
+ (MESH_FLAGS_AE_A4 |
+ MESH_FLAGS_AE_A5_A6))
+ return -1;
if (meshdr->flags & MESH_FLAGS_AE_A4)
skb_copy_bits(skb, hdrlen +
offsetof(struct ieee80211s_hdr, eaddr1),
--
1.7.10.4



2012-10-29 09:09:32

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wireless: drop invalid mesh address extension frames

On Thu, 2012-10-25 at 21:56 +0200, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> The mesh header can have address extension by a 4th
> or a 5th and 6th address, but never both. Drop such
> frames in 802.11 -> 802.3 conversion.

Applied.

johannes


2012-10-25 20:16:56

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wireless: drop invalid mesh address extension frames

On Thu, 2012-10-25 at 13:14 -0700, Javier Cardona wrote:

> > @@ -373,6 +370,11 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
> > /* make sure meshdr->flags is on the linear part */
> > if (!pskb_may_pull(skb, hdrlen + 1))
> > return -1;
> > + /* reserved */
> > + if ((meshdr->flags & MESH_FLAGS_AE) ==
> > + (MESH_FLAGS_AE_A4 |
> > + MESH_FLAGS_AE_A5_A6))
> > + return -1;
>
> You can filter more aggressively: If we are here we know that it is a
> unicast data frame (because FromDS == ToDS == 1). In that case, the
> only valid address extension modes are 0 or MESH_FLAGS_AE_A5_A6 (see
> Table 8-14). IOW, you could do:
>
> > + if ((meshdr->flags & MESH_FLAGS_AE_A4)))
> > + return -1

Ok.

> > @@ -397,6 +399,11 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
> > /* make sure meshdr->flags is on the linear part */
> > if (!pskb_may_pull(skb, hdrlen + 1))
> > return -1;
> > + /* reserved */
> > + if ((meshdr->flags & MESH_FLAGS_AE) ==
> > + (MESH_FLAGS_AE_A4 |
> > + MESH_FLAGS_AE_A5_A6))
> > + return -1;
>
> And a similar comment here: only valid address extension modes are 0
> or MESH_FLAGS_AE_A4, i.e.
>
> > + if ((meshdr->flags & MESH_FLAGS_AE_A5_A6)))
> > + return -1

Ok good, I'll make these changes, makes the code formatting nicer too :)

johannes


2012-10-25 20:15:13

by Javier Cardona

[permalink] [raw]
Subject: Re: [PATCH] wireless: drop invalid mesh address extension frames

Johannes,

Thanks for removing those legacy formats. See inline:

On Thu, Oct 25, 2012 at 12:56 PM, Johannes Berg
<[email protected]> wrote:
> From: Johannes Berg <[email protected]>
>
> The mesh header can have address extension by a 4th
> or a 5th and 6th address, but never both. Drop such
> frames in 802.11 -> 802.3 conversion.
>
> Cc: [email protected]
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> net/wireless/util.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index ef35f4e..7296ec0 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -312,18 +312,15 @@ EXPORT_SYMBOL(ieee80211_get_hdrlen_from_skb);
> static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr)
> {
> int ae = meshhdr->flags & MESH_FLAGS_AE;
> - /* 7.1.3.5a.2 */
> + /* IEEE 802.11 - 8.2.4.7.3 */
> switch (ae) {
> + default:
> case 0:
> return 6;
> case MESH_FLAGS_AE_A4:
> return 12;
> case MESH_FLAGS_AE_A5_A6:
> return 18;
> - case (MESH_FLAGS_AE_A4 | MESH_FLAGS_AE_A5_A6):
> - return 24;
> - default:
> - return 6;
> }
> }
>
> @@ -373,6 +370,11 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
> /* make sure meshdr->flags is on the linear part */
> if (!pskb_may_pull(skb, hdrlen + 1))
> return -1;
> + /* reserved */
> + if ((meshdr->flags & MESH_FLAGS_AE) ==
> + (MESH_FLAGS_AE_A4 |
> + MESH_FLAGS_AE_A5_A6))
> + return -1;

You can filter more aggressively: If we are here we know that it is a
unicast data frame (because FromDS == ToDS == 1). In that case, the
only valid address extension modes are 0 or MESH_FLAGS_AE_A5_A6 (see
Table 8-14). IOW, you could do:

> + if ((meshdr->flags & MESH_FLAGS_AE_A4)))
> + return -1

> if (meshdr->flags & MESH_FLAGS_AE_A5_A6) {
> skb_copy_bits(skb, hdrlen +
> offsetof(struct ieee80211s_hdr, eaddr1),
> @@ -397,6 +399,11 @@ int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr,
> /* make sure meshdr->flags is on the linear part */
> if (!pskb_may_pull(skb, hdrlen + 1))
> return -1;
> + /* reserved */
> + if ((meshdr->flags & MESH_FLAGS_AE) ==
> + (MESH_FLAGS_AE_A4 |
> + MESH_FLAGS_AE_A5_A6))
> + return -1;

And a similar comment here: only valid address extension modes are 0
or MESH_FLAGS_AE_A4, i.e.

> + if ((meshdr->flags & MESH_FLAGS_AE_A5_A6)))
> + return -1

> if (meshdr->flags & MESH_FLAGS_AE_A4)
> skb_copy_bits(skb, hdrlen +
> offsetof(struct ieee80211s_hdr, eaddr1),
> --
> 1.7.10.4
>


Cheers,

Javier


--
Javier Cardona
cozybit Inc.
http://www.cozybit.com