* [PATCH 0/3] mac80211: audit for access to skb data
@ 2012-10-25 22:46 Johannes Berg
2012-10-25 22:46 ` [PATCH 1/3] mac80211: check management frame header length Johannes Berg
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Johannes Berg @ 2012-10-25 22:46 UTC (permalink / raw)
To: linux-wireless
While looking at Javier's patch and some other things I found
a number of places that don't make sure the skb data is there
when it's accessed, fix them.
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] mac80211: check management frame header length 2012-10-25 22:46 [PATCH 0/3] mac80211: audit for access to skb data Johannes Berg @ 2012-10-25 22:46 ` Johannes Berg 2012-10-25 22:46 ` [PATCH 2/3] mac80211: verify that skb data is present Johannes Berg ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Johannes Berg @ 2012-10-25 22:46 UTC (permalink / raw) To: linux-wireless; +Cc: Johannes Berg From: Johannes Berg <johannes.berg@intel.com> Due to pskb_may_pull() checking the skb length, all non-management frames are checked on input whether their 802.11 header is fully present. Also add that check for management frames and remove a check that is now duplicate. This prevents accessing skb data beyond the frame end. Cc: stable@vger.kernel.org Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- net/mac80211/rx.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index c6865f2..99cdee1 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -1470,7 +1470,6 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx) frag = sc & IEEE80211_SCTL_FRAG; if (likely((!ieee80211_has_morefrags(fc) && frag == 0) || - (rx->skb)->len < 24 || is_multicast_ether_addr(hdr->addr1))) { /* not fragmented */ goto out; @@ -2915,10 +2914,15 @@ static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw, if (ieee80211_is_data(fc) || ieee80211_is_mgmt(fc)) local->dot11ReceivedFragmentCount++; - if (ieee80211_is_mgmt(fc)) - err = skb_linearize(skb); - else + if (ieee80211_is_mgmt(fc)) { + /* drop frame if too short for header */ + if (skb->len < ieee80211_hdrlen(fc)) + err = -ENOBUFS; + else + err = skb_linearize(skb); + } else { err = !pskb_may_pull(skb, ieee80211_hdrlen(fc)); + } if (err) { dev_kfree_skb(skb); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] mac80211: verify that skb data is present 2012-10-25 22:46 [PATCH 0/3] mac80211: audit for access to skb data Johannes Berg 2012-10-25 22:46 ` [PATCH 1/3] mac80211: check management frame header length Johannes Berg @ 2012-10-25 22:46 ` Johannes Berg 2012-10-25 23:05 ` Thomas Pedersen 2012-10-26 11:00 ` [PATCH v2 " Johannes Berg 2012-10-25 22:46 ` [PATCH 3/3] mac80211: make sure data is accessible in EAPOL check Johannes Berg 2012-10-29 9:09 ` [PATCH 0/3] mac80211: audit for access to skb data Johannes Berg 3 siblings, 2 replies; 13+ messages in thread From: Johannes Berg @ 2012-10-25 22:46 UTC (permalink / raw) To: linux-wireless; +Cc: Johannes Berg From: Johannes Berg <johannes.berg@intel.com> A number of places in the mesh code don't check that the frame data is present and in the skb header when trying to access. Add those checks and the necessary pskb_may_pull() calls. This prevents accessing data that doesn't actually exist. To do this, export ieee80211_get_mesh_hdrlen() to be able to use it in mac80211. Cc: stable@vger.kernel.org Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- include/net/cfg80211.h | 9 +++++++++ net/mac80211/rx.c | 43 +++++++++++++++++++++++++++++++++++++++---- net/wireless/util.c | 3 ++- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index f8cd4cf..7d5b600 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -2652,6 +2652,15 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb); unsigned int __attribute_const__ ieee80211_hdrlen(__le16 fc); /** + * ieee80211_get_mesh_hdrlen - get mesh extension header length + * @meshhdr: the mesh extension header, only the flags field + * (first byte) will be accessed + * Returns the length of the extension header, which is always at + * least 6 bytes and at most 18 if address 5 and 6 are present. + */ +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr); + +/** * DOC: Data path helpers * * In addition to generic utilities, cfg80211 also offers diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 99cdee1..6d1fd39 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -531,6 +531,11 @@ ieee80211_rx_mesh_check(struct ieee80211_rx_data *rx) if (ieee80211_is_action(hdr->frame_control)) { u8 category; + + /* make sure category field is present */ + if (rx->skb->len < IEEE80211_MIN_ACTION_SIZE) + return RX_DROP_MONITOR; + mgmt = (struct ieee80211_mgmt *)hdr; category = mgmt->u.action.category; if (category != WLAN_CATEGORY_MESH_ACTION && @@ -1892,6 +1897,20 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) hdr = (struct ieee80211_hdr *) skb->data; hdrlen = ieee80211_hdrlen(hdr->frame_control); + + /* make sure fixed part of mesh header is there, also checks skb len */ + if (!pskb_may_pull(rx->skb, hdrlen + 6)) + return RX_DROP_MONITOR; + + mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); + + /* make sure full mesh header is there, also checks skb len */ + if (!pskb_may_pull(rx->skb, + hdrlen + ieee80211_get_mesh_hdrlen(mesh_hdr))) + return RX_DROP_MONITOR; + + /* reload pointers */ + hdr = (struct ieee80211_hdr *) skb->data; mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); /* frame is in RMC, don't forward */ @@ -1913,11 +1932,19 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) char *mpp_addr; if (is_multicast_ether_addr(hdr->addr1)) { - mpp_addr = hdr->addr3; - proxied_addr = mesh_hdr->eaddr1; + if (mesh_hdr->flags & MESH_FLAGS_AE_A4) { + mpp_addr = hdr->addr3; + proxied_addr = mesh_hdr->eaddr1; + } else { + return RX_DROP_MONITOR; + } } else { - mpp_addr = hdr->addr4; - proxied_addr = mesh_hdr->eaddr2; + if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { + mpp_addr = hdr->addr4; + proxied_addr = mesh_hdr->eaddr2; + } else { + return RX_DROP_MONITOR; + } } rcu_read_lock(); @@ -2354,6 +2381,10 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) } break; case WLAN_CATEGORY_SELF_PROTECTED: + if (len < (IEEE80211_MIN_ACTION_SIZE + + sizeof(mgmt->u.action.u.self_prot.action_code))) + break; + switch (mgmt->u.action.u.self_prot.action_code) { case WLAN_SP_MESH_PEERING_OPEN: case WLAN_SP_MESH_PEERING_CLOSE: @@ -2372,6 +2403,10 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) } break; case WLAN_CATEGORY_MESH_ACTION: + if (len < (IEEE80211_MIN_ACTION_SIZE + + sizeof(mgmt->u.action.u.mesh_action.action_code))) + break; + if (!ieee80211_vif_is_mesh(&sdata->vif)) break; if (mesh_action_is_path_sel(mgmt) && diff --git a/net/wireless/util.c b/net/wireless/util.c index 45a09de..2762e83 100644 --- a/net/wireless/util.c +++ b/net/wireless/util.c @@ -309,7 +309,7 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb) } EXPORT_SYMBOL(ieee80211_get_hdrlen_from_skb); -static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) { int ae = meshhdr->flags & MESH_FLAGS_AE; /* 802.11-2012, 8.2.4.7.3 */ @@ -323,6 +323,7 @@ static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) return 18; } } +EXPORT_SYMBOL(ieee80211_get_mesh_hdrlen); int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr, enum nl80211_iftype iftype) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] mac80211: verify that skb data is present 2012-10-25 22:46 ` [PATCH 2/3] mac80211: verify that skb data is present Johannes Berg @ 2012-10-25 23:05 ` Thomas Pedersen 2012-10-25 23:58 ` Thomas Pedersen 2012-10-26 11:00 ` [PATCH v2 " Johannes Berg 1 sibling, 1 reply; 13+ messages in thread From: Thomas Pedersen @ 2012-10-25 23:05 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Johannes Berg On Fri, Oct 26, 2012 at 12:46:39AM +0200, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > A number of places in the mesh code don't check that > the frame data is present and in the skb header when > trying to access. Add those checks and the necessary > pskb_may_pull() calls. This prevents accessing data > that doesn't actually exist. > > To do this, export ieee80211_get_mesh_hdrlen() to be > able to use it in mac80211. > > Cc: stable@vger.kernel.org > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > include/net/cfg80211.h | 9 +++++++++ > net/mac80211/rx.c | 43 +++++++++++++++++++++++++++++++++++++++---- > net/wireless/util.c | 3 ++- > 3 files changed, 50 insertions(+), 5 deletions(-) > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > index f8cd4cf..7d5b600 100644 > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -2652,6 +2652,15 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb); > unsigned int __attribute_const__ ieee80211_hdrlen(__le16 fc); > > /** > + * ieee80211_get_mesh_hdrlen - get mesh extension header length > + * @meshhdr: the mesh extension header, only the flags field > + * (first byte) will be accessed > + * Returns the length of the extension header, which is always at > + * least 6 bytes and at most 18 if address 5 and 6 are present. > + */ > +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr); > + > +/** > * DOC: Data path helpers > * > * In addition to generic utilities, cfg80211 also offers > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > index 99cdee1..6d1fd39 100644 > --- a/net/mac80211/rx.c > +++ b/net/mac80211/rx.c > @@ -531,6 +531,11 @@ ieee80211_rx_mesh_check(struct ieee80211_rx_data *rx) > > if (ieee80211_is_action(hdr->frame_control)) { > u8 category; > + > + /* make sure category field is present */ > + if (rx->skb->len < IEEE80211_MIN_ACTION_SIZE) > + return RX_DROP_MONITOR; > + > mgmt = (struct ieee80211_mgmt *)hdr; > category = mgmt->u.action.category; > if (category != WLAN_CATEGORY_MESH_ACTION && > @@ -1892,6 +1897,20 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > > hdr = (struct ieee80211_hdr *) skb->data; > hdrlen = ieee80211_hdrlen(hdr->frame_control); > + > + /* make sure fixed part of mesh header is there, also checks skb len */ > + if (!pskb_may_pull(rx->skb, hdrlen + 6)) > + return RX_DROP_MONITOR; > + > + mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); > + > + /* make sure full mesh header is there, also checks skb len */ > + if (!pskb_may_pull(rx->skb, > + hdrlen + ieee80211_get_mesh_hdrlen(mesh_hdr))) > + return RX_DROP_MONITOR; > + > + /* reload pointers */ > + hdr = (struct ieee80211_hdr *) skb->data; > mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); > > /* frame is in RMC, don't forward */ > @@ -1913,11 +1932,19 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > char *mpp_addr; > > if (is_multicast_ether_addr(hdr->addr1)) { > - mpp_addr = hdr->addr3; > - proxied_addr = mesh_hdr->eaddr1; > + if (mesh_hdr->flags & MESH_FLAGS_AE_A4) { > + mpp_addr = hdr->addr3; > + proxied_addr = mesh_hdr->eaddr1; > + } else { > + return RX_DROP_MONITOR; > + } > } else { > - mpp_addr = hdr->addr4; > - proxied_addr = mesh_hdr->eaddr2; > + if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { > + mpp_addr = hdr->addr4; > + proxied_addr = mesh_hdr->eaddr2; > + } else { > + return RX_DROP_MONITOR; > + } This will drop all non-proxied mcast or unicast data frames :) It looks like this was already incorrect, but what if you only read the eaddrs if the right flag is set? Like: if (is_multicast_ether_addr(hdr->addr1) && mesh_hdr->flags & MESH_FLAGS_AE_A4) { mpp_addr = hdr->addr3; proxied_addr = mesh_hdr->eaddr1; } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { mpp_addr = hdr->addr4; proxied_addr = mesh_hdr->eaddr2; } ^ any data frames that get there should only be data anyway, not sure if you have Javier's patch adding that yet. > } > > rcu_read_lock(); > @@ -2354,6 +2381,10 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) > } > break; > case WLAN_CATEGORY_SELF_PROTECTED: > + if (len < (IEEE80211_MIN_ACTION_SIZE + > + sizeof(mgmt->u.action.u.self_prot.action_code))) > + break; > + > switch (mgmt->u.action.u.self_prot.action_code) { > case WLAN_SP_MESH_PEERING_OPEN: > case WLAN_SP_MESH_PEERING_CLOSE: > @@ -2372,6 +2403,10 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) > } > break; > case WLAN_CATEGORY_MESH_ACTION: > + if (len < (IEEE80211_MIN_ACTION_SIZE + > + sizeof(mgmt->u.action.u.mesh_action.action_code))) > + break; > + > if (!ieee80211_vif_is_mesh(&sdata->vif)) > break; > if (mesh_action_is_path_sel(mgmt) && > diff --git a/net/wireless/util.c b/net/wireless/util.c > index 45a09de..2762e83 100644 > --- a/net/wireless/util.c > +++ b/net/wireless/util.c > @@ -309,7 +309,7 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb) > } > EXPORT_SYMBOL(ieee80211_get_hdrlen_from_skb); > > -static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) > +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) > { > int ae = meshhdr->flags & MESH_FLAGS_AE; > /* 802.11-2012, 8.2.4.7.3 */ > @@ -323,6 +323,7 @@ static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) > return 18; > } > } > +EXPORT_SYMBOL(ieee80211_get_mesh_hdrlen); > > int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr, > enum nl80211_iftype iftype) > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] mac80211: verify that skb data is present 2012-10-25 23:05 ` Thomas Pedersen @ 2012-10-25 23:58 ` Thomas Pedersen 2012-10-26 0:57 ` Thomas Pedersen 0 siblings, 1 reply; 13+ messages in thread From: Thomas Pedersen @ 2012-10-25 23:58 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Johannes Berg On Thu, Oct 25, 2012 at 04:05:05PM -0700, Thomas Pedersen wrote: > On Fri, Oct 26, 2012 at 12:46:39AM +0200, Johannes Berg wrote: > > From: Johannes Berg <johannes.berg@intel.com> > > > > A number of places in the mesh code don't check that > > the frame data is present and in the skb header when > > trying to access. Add those checks and the necessary > > pskb_may_pull() calls. This prevents accessing data > > that doesn't actually exist. > > > > To do this, export ieee80211_get_mesh_hdrlen() to be > > able to use it in mac80211. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > > --- > > include/net/cfg80211.h | 9 +++++++++ > > net/mac80211/rx.c | 43 +++++++++++++++++++++++++++++++++++++++---- > > net/wireless/util.c | 3 ++- > > 3 files changed, 50 insertions(+), 5 deletions(-) > > > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > > index f8cd4cf..7d5b600 100644 > > --- a/include/net/cfg80211.h > > +++ b/include/net/cfg80211.h > > @@ -2652,6 +2652,15 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb); > > unsigned int __attribute_const__ ieee80211_hdrlen(__le16 fc); > > > > /** > > + * ieee80211_get_mesh_hdrlen - get mesh extension header length > > + * @meshhdr: the mesh extension header, only the flags field > > + * (first byte) will be accessed > > + * Returns the length of the extension header, which is always at > > + * least 6 bytes and at most 18 if address 5 and 6 are present. > > + */ > > +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr); > > + > > +/** > > * DOC: Data path helpers > > * > > * In addition to generic utilities, cfg80211 also offers > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > > index 99cdee1..6d1fd39 100644 > > --- a/net/mac80211/rx.c > > +++ b/net/mac80211/rx.c > > @@ -531,6 +531,11 @@ ieee80211_rx_mesh_check(struct ieee80211_rx_data *rx) > > > > if (ieee80211_is_action(hdr->frame_control)) { > > u8 category; > > + > > + /* make sure category field is present */ > > + if (rx->skb->len < IEEE80211_MIN_ACTION_SIZE) > > + return RX_DROP_MONITOR; > > + > > mgmt = (struct ieee80211_mgmt *)hdr; > > category = mgmt->u.action.category; > > if (category != WLAN_CATEGORY_MESH_ACTION && > > @@ -1892,6 +1897,20 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > > > > hdr = (struct ieee80211_hdr *) skb->data; > > hdrlen = ieee80211_hdrlen(hdr->frame_control); > > + > > + /* make sure fixed part of mesh header is there, also checks skb len */ > > + if (!pskb_may_pull(rx->skb, hdrlen + 6)) > > + return RX_DROP_MONITOR; > > + > > + mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); > > + > > + /* make sure full mesh header is there, also checks skb len */ > > + if (!pskb_may_pull(rx->skb, > > + hdrlen + ieee80211_get_mesh_hdrlen(mesh_hdr))) > > + return RX_DROP_MONITOR; > > + > > + /* reload pointers */ > > + hdr = (struct ieee80211_hdr *) skb->data; > > mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); > > > > /* frame is in RMC, don't forward */ > > @@ -1913,11 +1932,19 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > > char *mpp_addr; > > > > if (is_multicast_ether_addr(hdr->addr1)) { > > - mpp_addr = hdr->addr3; > > - proxied_addr = mesh_hdr->eaddr1; > > + if (mesh_hdr->flags & MESH_FLAGS_AE_A4) { > > + mpp_addr = hdr->addr3; > > + proxied_addr = mesh_hdr->eaddr1; > > + } else { > > + return RX_DROP_MONITOR; > > + } > > } else { > > - mpp_addr = hdr->addr4; > > - proxied_addr = mesh_hdr->eaddr2; > > + if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { > > + mpp_addr = hdr->addr4; > > + proxied_addr = mesh_hdr->eaddr2; > > + } else { > > + return RX_DROP_MONITOR; > > + } > > This will drop all non-proxied mcast or unicast data frames :) > > It looks like this was already incorrect, but what if you only read the > eaddrs if the right flag is set? Like: > > if (is_multicast_ether_addr(hdr->addr1) && > mesh_hdr->flags & MESH_FLAGS_AE_A4) { > mpp_addr = hdr->addr3; > proxied_addr = mesh_hdr->eaddr1; > } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { > mpp_addr = hdr->addr4; > proxied_addr = mesh_hdr->eaddr2; > } Or even just: if (mesh_hdr->flags & MESH_FLAGS_AE_A4) { mpp_addr = hdr->addr3; proxied_addr = mesh_hdr->eaddr1; } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { mpp_addr = hdr->addr4; proxied_addr = mesh_hdr->eaddr2; } Should be sufficient since according to table 8-14, those bits are only used for proxied mcast and unicast data frames, respectively. Thomas ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] mac80211: verify that skb data is present 2012-10-25 23:58 ` Thomas Pedersen @ 2012-10-26 0:57 ` Thomas Pedersen 2012-10-26 10:19 ` Johannes Berg 0 siblings, 1 reply; 13+ messages in thread From: Thomas Pedersen @ 2012-10-26 0:57 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless, Johannes Berg On Thu, Oct 25, 2012 at 04:58:16PM -0700, Thomas Pedersen wrote: > On Thu, Oct 25, 2012 at 04:05:05PM -0700, Thomas Pedersen wrote: > > On Fri, Oct 26, 2012 at 12:46:39AM +0200, Johannes Berg wrote: > > > From: Johannes Berg <johannes.berg@intel.com> > > > > > > A number of places in the mesh code don't check that > > > the frame data is present and in the skb header when > > > trying to access. Add those checks and the necessary > > > pskb_may_pull() calls. This prevents accessing data > > > that doesn't actually exist. > > > > > > To do this, export ieee80211_get_mesh_hdrlen() to be > > > able to use it in mac80211. > > > > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > > > --- > > > include/net/cfg80211.h | 9 +++++++++ > > > net/mac80211/rx.c | 43 +++++++++++++++++++++++++++++++++++++++---- > > > net/wireless/util.c | 3 ++- > > > 3 files changed, 50 insertions(+), 5 deletions(-) > > > > > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > > > index f8cd4cf..7d5b600 100644 > > > --- a/include/net/cfg80211.h > > > +++ b/include/net/cfg80211.h > > > @@ -2652,6 +2652,15 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb); > > > unsigned int __attribute_const__ ieee80211_hdrlen(__le16 fc); > > > > > > /** > > > + * ieee80211_get_mesh_hdrlen - get mesh extension header length > > > + * @meshhdr: the mesh extension header, only the flags field > > > + * (first byte) will be accessed > > > + * Returns the length of the extension header, which is always at > > > + * least 6 bytes and at most 18 if address 5 and 6 are present. > > > + */ > > > +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr); > > > + > > > +/** > > > * DOC: Data path helpers > > > * > > > * In addition to generic utilities, cfg80211 also offers > > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > > > index 99cdee1..6d1fd39 100644 > > > --- a/net/mac80211/rx.c > > > +++ b/net/mac80211/rx.c > > > @@ -531,6 +531,11 @@ ieee80211_rx_mesh_check(struct ieee80211_rx_data *rx) > > > > > > if (ieee80211_is_action(hdr->frame_control)) { > > > u8 category; > > > + > > > + /* make sure category field is present */ > > > + if (rx->skb->len < IEEE80211_MIN_ACTION_SIZE) > > > + return RX_DROP_MONITOR; > > > + > > > mgmt = (struct ieee80211_mgmt *)hdr; > > > category = mgmt->u.action.category; > > > if (category != WLAN_CATEGORY_MESH_ACTION && > > > @@ -1892,6 +1897,20 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > > > > > > hdr = (struct ieee80211_hdr *) skb->data; > > > hdrlen = ieee80211_hdrlen(hdr->frame_control); > > > + > > > + /* make sure fixed part of mesh header is there, also checks skb len */ > > > + if (!pskb_may_pull(rx->skb, hdrlen + 6)) > > > + return RX_DROP_MONITOR; > > > + > > > + mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); > > > + > > > + /* make sure full mesh header is there, also checks skb len */ > > > + if (!pskb_may_pull(rx->skb, > > > + hdrlen + ieee80211_get_mesh_hdrlen(mesh_hdr))) > > > + return RX_DROP_MONITOR; > > > + > > > + /* reload pointers */ > > > + hdr = (struct ieee80211_hdr *) skb->data; > > > mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); > > > > > > /* frame is in RMC, don't forward */ > > > @@ -1913,11 +1932,19 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > > > char *mpp_addr; > > > > > > if (is_multicast_ether_addr(hdr->addr1)) { > > > - mpp_addr = hdr->addr3; > > > - proxied_addr = mesh_hdr->eaddr1; > > > + if (mesh_hdr->flags & MESH_FLAGS_AE_A4) { > > > + mpp_addr = hdr->addr3; > > > + proxied_addr = mesh_hdr->eaddr1; > > > + } else { > > > + return RX_DROP_MONITOR; > > > + } > > > } else { > > > - mpp_addr = hdr->addr4; > > > - proxied_addr = mesh_hdr->eaddr2; > > > + if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { > > > + mpp_addr = hdr->addr4; > > > + proxied_addr = mesh_hdr->eaddr2; > > > + } else { > > > + return RX_DROP_MONITOR; > > > + } > > > > This will drop all non-proxied mcast or unicast data frames :) > > > > It looks like this was already incorrect, but what if you only read the > > eaddrs if the right flag is set? Like: > > > > if (is_multicast_ether_addr(hdr->addr1) && > > mesh_hdr->flags & MESH_FLAGS_AE_A4) { > > mpp_addr = hdr->addr3; > > proxied_addr = mesh_hdr->eaddr1; > > } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { > > mpp_addr = hdr->addr4; > > proxied_addr = mesh_hdr->eaddr2; > > } > > Or even just: > > if (mesh_hdr->flags & MESH_FLAGS_AE_A4) { > mpp_addr = hdr->addr3; > proxied_addr = mesh_hdr->eaddr1; > } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { > mpp_addr = hdr->addr4; > proxied_addr = mesh_hdr->eaddr2; > } > > Should be sufficient since according to table 8-14, those bits are only > used for proxied mcast and unicast data frames, respectively. Oh, that hunk is already inside the MESH_FLAGS_AE check... so: if (mesh_hdr->flags & MESH_FLAGS_AE_A4) { mpp_addr = hdr->addr3; proxied_addr = mesh_hdr->eaddr1; } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { mpp_addr = hdr->addr4; proxied_addr = mesh_hdr->eaddr2; } else return RX_DROP_MONITOR; Should be ok since we already made sure to have enough data for each address extension size. Sorry for the noise, Thomas ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] mac80211: verify that skb data is present 2012-10-26 0:57 ` Thomas Pedersen @ 2012-10-26 10:19 ` Johannes Berg 2012-10-26 10:21 ` Johannes Berg 0 siblings, 1 reply; 13+ messages in thread From: Johannes Berg @ 2012-10-26 10:19 UTC (permalink / raw) To: Thomas Pedersen; +Cc: linux-wireless On Thu, 2012-10-25 at 17:57 -0700, Thomas Pedersen wrote: > Oh, that hunk is already inside the MESH_FLAGS_AE check... so: > > > if (mesh_hdr->flags & MESH_FLAGS_AE_A4) { > mpp_addr = hdr->addr3; > proxied_addr = mesh_hdr->eaddr1; > } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { > mpp_addr = hdr->addr4; > proxied_addr = mesh_hdr->eaddr2; > } else > return RX_DROP_MONITOR; > > Should be ok since we already made sure to have enough data for each > address extension size. But that loses the is_multicast_ether_addr() check now, no? I mean, we wouldn't want to accept an AE_A4 frame unless A1 was multicast, at least not according to the original code? All I was trying to make sure is that eaddr1/2 exist when they're used, and that's the length check, but is equivalent (due to earlier checks) to testing AE_A4/AE_A5_A6, I think? None of your versions seem to be equivalent of the original code, which says that * multicast -> use addr3/eaddr1 * unicast -> use addr4/eaddr2 and I'm just adding checks that eaddr1/eaddr2 actually exist. Ohh, ok, no I see, I'm not doing that. What we really need to do is something entirely different: if (is_multicast_ether_addr(hdr->addr1)) { mpp_addr = hdr->addr3; proxied_addr = mesh_hdr->eaddr1; } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { /* has_a4 already checked in ieee80211_rx_mesh_check */ mpp_addr = hdr->addr4; proxied_addr = mesh_hdr->eaddr2; } else { return RX_DROP_MONITOR; } johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] mac80211: verify that skb data is present 2012-10-26 10:19 ` Johannes Berg @ 2012-10-26 10:21 ` Johannes Berg 0 siblings, 0 replies; 13+ messages in thread From: Johannes Berg @ 2012-10-26 10:21 UTC (permalink / raw) To: Thomas Pedersen; +Cc: linux-wireless On Fri, 2012-10-26 at 12:19 +0200, Johannes Berg wrote: > Ohh, ok, no I see, I'm not doing that. What we really need to do is > something entirely different: > > if (is_multicast_ether_addr(hdr->addr1)) { > mpp_addr = hdr->addr3; > proxied_addr = mesh_hdr->eaddr1; where this part is fine because the entire code is inside the AE check already, so the eaddr1 must exist. johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] mac80211: verify that skb data is present 2012-10-25 22:46 ` [PATCH 2/3] mac80211: verify that skb data is present Johannes Berg 2012-10-25 23:05 ` Thomas Pedersen @ 2012-10-26 11:00 ` Johannes Berg 2012-10-26 16:29 ` Thomas Pedersen 1 sibling, 1 reply; 13+ messages in thread From: Johannes Berg @ 2012-10-26 11:00 UTC (permalink / raw) To: linux-wireless From: Johannes Berg <johannes.berg@intel.com> A number of places in the mesh code don't check that the frame data is present and in the skb header when trying to access. Add those checks and the necessary pskb_may_pull() calls. This prevents accessing data that doesn't actually exist. To do this, export ieee80211_get_mesh_hdrlen() to be able to use it in mac80211. Cc: stable@vger.kernel.org Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- include/net/cfg80211.h | 9 +++++++++ net/mac80211/rx.c | 32 +++++++++++++++++++++++++++++++- net/wireless/util.c | 3 ++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index f8cd4cf..7d5b600 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -2652,6 +2652,15 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb); unsigned int __attribute_const__ ieee80211_hdrlen(__le16 fc); /** + * ieee80211_get_mesh_hdrlen - get mesh extension header length + * @meshhdr: the mesh extension header, only the flags field + * (first byte) will be accessed + * Returns the length of the extension header, which is always at + * least 6 bytes and at most 18 if address 5 and 6 are present. + */ +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr); + +/** * DOC: Data path helpers * * In addition to generic utilities, cfg80211 also offers diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 99cdee1..265a032d 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -531,6 +531,11 @@ ieee80211_rx_mesh_check(struct ieee80211_rx_data *rx) if (ieee80211_is_action(hdr->frame_control)) { u8 category; + + /* make sure category field is present */ + if (rx->skb->len < IEEE80211_MIN_ACTION_SIZE) + return RX_DROP_MONITOR; + mgmt = (struct ieee80211_mgmt *)hdr; category = mgmt->u.action.category; if (category != WLAN_CATEGORY_MESH_ACTION && @@ -1892,6 +1897,20 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) hdr = (struct ieee80211_hdr *) skb->data; hdrlen = ieee80211_hdrlen(hdr->frame_control); + + /* make sure fixed part of mesh header is there, also checks skb len */ + if (!pskb_may_pull(rx->skb, hdrlen + 6)) + return RX_DROP_MONITOR; + + mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); + + /* make sure full mesh header is there, also checks skb len */ + if (!pskb_may_pull(rx->skb, + hdrlen + ieee80211_get_mesh_hdrlen(mesh_hdr))) + return RX_DROP_MONITOR; + + /* reload pointers */ + hdr = (struct ieee80211_hdr *) skb->data; mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); /* frame is in RMC, don't forward */ @@ -1915,9 +1934,12 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) if (is_multicast_ether_addr(hdr->addr1)) { mpp_addr = hdr->addr3; proxied_addr = mesh_hdr->eaddr1; - } else { + } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { + /* has_a4 already checked in ieee80211_rx_mesh_check */ mpp_addr = hdr->addr4; proxied_addr = mesh_hdr->eaddr2; + } else { + return RX_DROP_MONITOR; } rcu_read_lock(); @@ -2354,6 +2376,10 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) } break; case WLAN_CATEGORY_SELF_PROTECTED: + if (len < (IEEE80211_MIN_ACTION_SIZE + + sizeof(mgmt->u.action.u.self_prot.action_code))) + break; + switch (mgmt->u.action.u.self_prot.action_code) { case WLAN_SP_MESH_PEERING_OPEN: case WLAN_SP_MESH_PEERING_CLOSE: @@ -2372,6 +2398,10 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) } break; case WLAN_CATEGORY_MESH_ACTION: + if (len < (IEEE80211_MIN_ACTION_SIZE + + sizeof(mgmt->u.action.u.mesh_action.action_code))) + break; + if (!ieee80211_vif_is_mesh(&sdata->vif)) break; if (mesh_action_is_path_sel(mgmt) && diff --git a/net/wireless/util.c b/net/wireless/util.c index 45a09de..2762e83 100644 --- a/net/wireless/util.c +++ b/net/wireless/util.c @@ -309,7 +309,7 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb) } EXPORT_SYMBOL(ieee80211_get_hdrlen_from_skb); -static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) { int ae = meshhdr->flags & MESH_FLAGS_AE; /* 802.11-2012, 8.2.4.7.3 */ @@ -323,6 +323,7 @@ static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) return 18; } } +EXPORT_SYMBOL(ieee80211_get_mesh_hdrlen); int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr, enum nl80211_iftype iftype) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] mac80211: verify that skb data is present 2012-10-26 11:00 ` [PATCH v2 " Johannes Berg @ 2012-10-26 16:29 ` Thomas Pedersen 2012-10-26 16:35 ` Johannes Berg 0 siblings, 1 reply; 13+ messages in thread From: Thomas Pedersen @ 2012-10-26 16:29 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless On Fri, Oct 26, 2012 at 4:00 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > From: Johannes Berg <johannes.berg@intel.com> > > A number of places in the mesh code don't check that > the frame data is present and in the skb header when > trying to access. Add those checks and the necessary > pskb_may_pull() calls. This prevents accessing data > that doesn't actually exist. > > To do this, export ieee80211_get_mesh_hdrlen() to be > able to use it in mac80211. > > Cc: stable@vger.kernel.org > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > include/net/cfg80211.h | 9 +++++++++ > net/mac80211/rx.c | 32 +++++++++++++++++++++++++++++++- > net/wireless/util.c | 3 ++- > 3 files changed, 42 insertions(+), 2 deletions(-) > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > index f8cd4cf..7d5b600 100644 > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -2652,6 +2652,15 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb); > unsigned int __attribute_const__ ieee80211_hdrlen(__le16 fc); > > /** > + * ieee80211_get_mesh_hdrlen - get mesh extension header length > + * @meshhdr: the mesh extension header, only the flags field > + * (first byte) will be accessed > + * Returns the length of the extension header, which is always at > + * least 6 bytes and at most 18 if address 5 and 6 are present. > + */ > +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr); > + > +/** > * DOC: Data path helpers > * > * In addition to generic utilities, cfg80211 also offers > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > index 99cdee1..265a032d 100644 > --- a/net/mac80211/rx.c > +++ b/net/mac80211/rx.c > @@ -531,6 +531,11 @@ ieee80211_rx_mesh_check(struct ieee80211_rx_data *rx) > > if (ieee80211_is_action(hdr->frame_control)) { > u8 category; > + > + /* make sure category field is present */ > + if (rx->skb->len < IEEE80211_MIN_ACTION_SIZE) > + return RX_DROP_MONITOR; > + > mgmt = (struct ieee80211_mgmt *)hdr; > category = mgmt->u.action.category; > if (category != WLAN_CATEGORY_MESH_ACTION && > @@ -1892,6 +1897,20 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > > hdr = (struct ieee80211_hdr *) skb->data; > hdrlen = ieee80211_hdrlen(hdr->frame_control); > + > + /* make sure fixed part of mesh header is there, also checks skb len */ > + if (!pskb_may_pull(rx->skb, hdrlen + 6)) > + return RX_DROP_MONITOR; > + > + mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); > + > + /* make sure full mesh header is there, also checks skb len */ > + if (!pskb_may_pull(rx->skb, > + hdrlen + ieee80211_get_mesh_hdrlen(mesh_hdr))) > + return RX_DROP_MONITOR; > + > + /* reload pointers */ > + hdr = (struct ieee80211_hdr *) skb->data; > mesh_hdr = (struct ieee80211s_hdr *) (skb->data + hdrlen); > > /* frame is in RMC, don't forward */ > @@ -1915,9 +1934,12 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > if (is_multicast_ether_addr(hdr->addr1)) { > mpp_addr = hdr->addr3; > proxied_addr = mesh_hdr->eaddr1; > - } else { > + } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { OK, but now the first check (is_multicast_ether_addr()) implies AE_A4 flag. It seems cleaner to just be explicit about what you're asking for both cases. > + /* has_a4 already checked in ieee80211_rx_mesh_check */ > mpp_addr = hdr->addr4; > proxied_addr = mesh_hdr->eaddr2; > + } else { > + return RX_DROP_MONITOR; > } > > rcu_read_lock(); > @@ -2354,6 +2376,10 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) > } > break; > case WLAN_CATEGORY_SELF_PROTECTED: > + if (len < (IEEE80211_MIN_ACTION_SIZE + > + sizeof(mgmt->u.action.u.self_prot.action_code))) > + break; > + > switch (mgmt->u.action.u.self_prot.action_code) { > case WLAN_SP_MESH_PEERING_OPEN: > case WLAN_SP_MESH_PEERING_CLOSE: > @@ -2372,6 +2398,10 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) > } > break; > case WLAN_CATEGORY_MESH_ACTION: > + if (len < (IEEE80211_MIN_ACTION_SIZE + > + sizeof(mgmt->u.action.u.mesh_action.action_code))) > + break; > + > if (!ieee80211_vif_is_mesh(&sdata->vif)) > break; > if (mesh_action_is_path_sel(mgmt) && > diff --git a/net/wireless/util.c b/net/wireless/util.c > index 45a09de..2762e83 100644 > --- a/net/wireless/util.c > +++ b/net/wireless/util.c > @@ -309,7 +309,7 @@ unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb) > } > EXPORT_SYMBOL(ieee80211_get_hdrlen_from_skb); > > -static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) > +unsigned int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) > { > int ae = meshhdr->flags & MESH_FLAGS_AE; > /* 802.11-2012, 8.2.4.7.3 */ > @@ -323,6 +323,7 @@ static int ieee80211_get_mesh_hdrlen(struct ieee80211s_hdr *meshhdr) > return 18; > } > } > +EXPORT_SYMBOL(ieee80211_get_mesh_hdrlen); > > int ieee80211_data_to_8023(struct sk_buff *skb, const u8 *addr, > enum nl80211_iftype iftype) > -- > 1.7.10.4 > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/3] mac80211: verify that skb data is present 2012-10-26 16:29 ` Thomas Pedersen @ 2012-10-26 16:35 ` Johannes Berg 0 siblings, 0 replies; 13+ messages in thread From: Johannes Berg @ 2012-10-26 16:35 UTC (permalink / raw) To: Thomas Pedersen; +Cc: linux-wireless On Fri, 2012-10-26 at 09:29 -0700, Thomas Pedersen wrote: > > @@ -1915,9 +1934,12 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx) > > if (is_multicast_ether_addr(hdr->addr1)) { > > mpp_addr = hdr->addr3; > > proxied_addr = mesh_hdr->eaddr1; > > - } else { > > + } else if (mesh_hdr->flags & MESH_FLAGS_AE_A5_A6) { > > OK, but now the first check (is_multicast_ether_addr()) implies AE_A4 > flag. It seems cleaner to just be explicit about what you're asking > for both cases. Well, I dunno. That seems like a change that might be correct in the semantics of mesh, but it's not the minimal technical change that I'm after to make sure we have all the data we access. This may not be valid in mesh, but if we were to receive a multicast frame with AE_A5_A5 instead of AE_A4 then we could still use eaddr1, though we'd probably drop the frame later or something. johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] mac80211: make sure data is accessible in EAPOL check 2012-10-25 22:46 [PATCH 0/3] mac80211: audit for access to skb data Johannes Berg 2012-10-25 22:46 ` [PATCH 1/3] mac80211: check management frame header length Johannes Berg 2012-10-25 22:46 ` [PATCH 2/3] mac80211: verify that skb data is present Johannes Berg @ 2012-10-25 22:46 ` Johannes Berg 2012-10-29 9:09 ` [PATCH 0/3] mac80211: audit for access to skb data Johannes Berg 3 siblings, 0 replies; 13+ messages in thread From: Johannes Berg @ 2012-10-25 22:46 UTC (permalink / raw) To: linux-wireless; +Cc: Johannes Berg From: Johannes Berg <johannes.berg@intel.com> The code to allow EAPOL frames even when the station isn't yet marked associated needs to check that the incoming frame is long enough and due to paged RX it also can't assume skb->data contains the right data, it must use skb_copy_bits(). Fix this to avoid using data that doesn't really exist. Cc: stable@vger.kernel.org Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- net/mac80211/rx.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 6d1fd39..a5a3db8 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -888,14 +888,16 @@ ieee80211_rx_h_check(struct ieee80211_rx_data *rx) */ if (rx->sta && rx->sdata->vif.type == NL80211_IFTYPE_STATION && ieee80211_is_data_present(hdr->frame_control)) { - u16 ethertype; - u8 *payload; - - payload = rx->skb->data + - ieee80211_hdrlen(hdr->frame_control); - ethertype = (payload[6] << 8) | payload[7]; - if (cpu_to_be16(ethertype) == - rx->sdata->control_port_protocol) + unsigned int hdrlen; + __be16 ethertype; + + hdrlen = ieee80211_hdrlen(hdr->frame_control); + + if (rx->skb->len < hdrlen + 8) + return RX_DROP_MONITOR; + + skb_copy_bits(rx->skb, hdrlen + 6, ðertype, 2); + if (ethertype == rx->sdata->control_port_protocol) return RX_CONTINUE; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] mac80211: audit for access to skb data 2012-10-25 22:46 [PATCH 0/3] mac80211: audit for access to skb data Johannes Berg ` (2 preceding siblings ...) 2012-10-25 22:46 ` [PATCH 3/3] mac80211: make sure data is accessible in EAPOL check Johannes Berg @ 2012-10-29 9:09 ` Johannes Berg 3 siblings, 0 replies; 13+ messages in thread From: Johannes Berg @ 2012-10-29 9:09 UTC (permalink / raw) To: linux-wireless On Fri, 2012-10-26 at 00:46 +0200, Johannes Berg wrote: > While looking at Javier's patch and some other things I found > a number of places that don't make sure the skb data is there > when it's accessed, fix them. Applied all three to mac80211.git. johannes ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-10-29 9:09 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-25 22:46 [PATCH 0/3] mac80211: audit for access to skb data Johannes Berg 2012-10-25 22:46 ` [PATCH 1/3] mac80211: check management frame header length Johannes Berg 2012-10-25 22:46 ` [PATCH 2/3] mac80211: verify that skb data is present Johannes Berg 2012-10-25 23:05 ` Thomas Pedersen 2012-10-25 23:58 ` Thomas Pedersen 2012-10-26 0:57 ` Thomas Pedersen 2012-10-26 10:19 ` Johannes Berg 2012-10-26 10:21 ` Johannes Berg 2012-10-26 11:00 ` [PATCH v2 " Johannes Berg 2012-10-26 16:29 ` Thomas Pedersen 2012-10-26 16:35 ` Johannes Berg 2012-10-25 22:46 ` [PATCH 3/3] mac80211: make sure data is accessible in EAPOL check Johannes Berg 2012-10-29 9:09 ` [PATCH 0/3] mac80211: audit for access to skb data Johannes Berg
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).