[PATCH net-next] some mac802154 fixes

[PATCH net-next] some mac802154 fixes

[Phoebe Buckheister]

Recent llsec code introduced a memory leak on decryption failures during rx.
This fixes said leak, and optimizes the receive loop to only deliver skbs to
the (single) device that's actually up.

---

Phoebe Buckheister (2):
      mac802154: properly free incoming skbs on decryption failure
      mac802154: don't deliver packets to devices that are down


 net/mac802154/wpan.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

[PATCH net-next 1/2] mac802154: properly free incoming skbs on decryption failure

[Phoebe Buckheister]

mac802154 RX did not free skbs on decryption failure, assuming that the
caller would when the local rx handler returned _DROP. This was false.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister-mPn0NPGs4xGatNDF+KUbs4QuADTiUCJX@public.gmane.org>
---
 net/mac802154/wpan.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
index 23bc91c..c8cfd54 100644
--- a/net/mac802154/wpan.c
+++ b/net/mac802154/wpan.c
@@ -472,6 +472,7 @@ mac802154_subif_frame(struct mac802154_sub_if_data *sdata, struct sk_buff *skb,
 	rc = mac802154_llsec_decrypt(&sdata->sec, skb);
 	if (rc) {
 		pr_debug("decryption failed: %i\n", rc);
+		kfree_skb(skb);
 		return NET_RX_DROP;
 	}
 
-- 
1.7.9.5


------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems

[PATCH net-next 2/2] mac802154: don't deliver packets to devices that are down

[Phoebe Buckheister]

Only one WPAN devices can be active at any given time, so only deliver
packets to that one interface that is actually up.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister-mPn0NPGs4xGatNDF+KUbs4QuADTiUCJX@public.gmane.org>
---
 net/mac802154/wpan.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
index c8cfd54..1ba648e 100644
--- a/net/mac802154/wpan.c
+++ b/net/mac802154/wpan.c
@@ -567,7 +567,6 @@ static int mac802154_parse_frame_start(struct sk_buff *skb,
 void mac802154_wpans_rx(struct mac802154_priv *priv, struct sk_buff *skb)
 {
 	int ret;
-	struct sk_buff *sskb;
 	struct mac802154_sub_if_data *sdata;
 	struct ieee802154_hdr hdr;
 
@@ -579,12 +578,15 @@ void mac802154_wpans_rx(struct mac802154_priv *priv, struct sk_buff *skb)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(sdata, &priv->slaves, list) {
-		if (sdata->type != IEEE802154_DEV_WPAN)
+		if (sdata->type != IEEE802154_DEV_WPAN ||
+		    !netif_running(sdata->dev))
 			continue;
 
-		sskb = skb_clone(skb, GFP_ATOMIC);
-		if (sskb)
-			mac802154_subif_frame(sdata, sskb, &hdr);
+		skb = skb_clone(skb, GFP_ATOMIC);
+		if (skb)
+			mac802154_subif_frame(sdata, skb, &hdr);
+
+		break;
 	}
 	rcu_read_unlock();
 }
-- 
1.7.9.5


------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems

Re: [PATCH net-next 2/2] mac802154: don't deliver packets to devices that are down

[Eric Dumazet]

On Tue, 2014-06-10 at 17:08 +0200, Phoebe Buckheister wrote:
> Only one WPAN devices can be active at any given time, so only deliver
> packets to that one interface that is actually up.
> 
> Signed-off-by: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>
> ---
>  net/mac802154/wpan.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
> index c8cfd54..1ba648e 100644
> --- a/net/mac802154/wpan.c
> +++ b/net/mac802154/wpan.c
> @@ -567,7 +567,6 @@ static int mac802154_parse_frame_start(struct sk_buff *skb,
>  void mac802154_wpans_rx(struct mac802154_priv *priv, struct sk_buff *skb)
>  {
>  	int ret;
> -	struct sk_buff *sskb;
>  	struct mac802154_sub_if_data *sdata;
>  	struct ieee802154_hdr hdr;
>  
> @@ -579,12 +578,15 @@ void mac802154_wpans_rx(struct mac802154_priv *priv, struct sk_buff *skb)
>  
>  	rcu_read_lock();
>  	list_for_each_entry_rcu(sdata, &priv->slaves, list) {
> -		if (sdata->type != IEEE802154_DEV_WPAN)
> +		if (sdata->type != IEEE802154_DEV_WPAN ||
> +		    !netif_running(sdata->dev))
>  			continue;
>  
> -		sskb = skb_clone(skb, GFP_ATOMIC);
> -		if (sskb)
> -			mac802154_subif_frame(sdata, sskb, &hdr);
> +		skb = skb_clone(skb, GFP_ATOMIC);
> +		if (skb)
> +			mac802154_subif_frame(sdata, skb, &hdr);
> +
> +		break;
>  	}
>  	rcu_read_unlock();
>  }

If this is really the case, do not clone the skb, and fix
mac802154_subif_rx().

(mac802154_wpans_rx() would always consume the skb)

Re: [PATCH net-next 2/2] mac802154: don't deliver packets to devices that are down

[Phoebe Buckheister]

On Tue, 10 Jun 2014 08:26:01 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Tue, 2014-06-10 at 17:08 +0200, Phoebe Buckheister wrote:
> > Only one WPAN devices can be active at any given time, so only
> > deliver packets to that one interface that is actually up.
> > 
> > Signed-off-by: Phoebe Buckheister
> > <phoebe.buckheister@itwm.fraunhofer.de> ---
> >  net/mac802154/wpan.c |   12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
> > index c8cfd54..1ba648e 100644
> > --- a/net/mac802154/wpan.c
> > +++ b/net/mac802154/wpan.c
> > @@ -567,7 +567,6 @@ static int mac802154_parse_frame_start(struct
> > sk_buff *skb, void mac802154_wpans_rx(struct mac802154_priv *priv,
> > struct sk_buff *skb) {
> >  	int ret;
> > -	struct sk_buff *sskb;
> >  	struct mac802154_sub_if_data *sdata;
> >  	struct ieee802154_hdr hdr;
> >  
> > @@ -579,12 +578,15 @@ void mac802154_wpans_rx(struct mac802154_priv
> > *priv, struct sk_buff *skb) 
> >  	rcu_read_lock();
> >  	list_for_each_entry_rcu(sdata, &priv->slaves, list) {
> > -		if (sdata->type != IEEE802154_DEV_WPAN)
> > +		if (sdata->type != IEEE802154_DEV_WPAN ||
> > +		    !netif_running(sdata->dev))
> >  			continue;
> >  
> > -		sskb = skb_clone(skb, GFP_ATOMIC);
> > -		if (sskb)
> > -			mac802154_subif_frame(sdata, sskb, &hdr);
> > +		skb = skb_clone(skb, GFP_ATOMIC);
> > +		if (skb)
> > +			mac802154_subif_frame(sdata, skb, &hdr);
> > +
> > +		break;
> >  	}
> >  	rcu_read_unlock();
> >  }
> 
> If this is really the case, do not clone the skb, and fix
> mac802154_subif_rx().
> 
> (mac802154_wpans_rx() would always consume the skb)

That's also possible, I guess. mac802154_wpans_rx is the last call,
after all, and I didn't want to muck with that part of the stack too
much (it's already convoluted and not entirely clear). Monitor devices
will still have to clone their skb though - and they as well could
benefit from not queueing packets to inactive devices.

I'll change and resend.