On Fri, 18 Jul 2014 16:48:08 +0200
Alexander Aring <alex.ar...@gmail.com> wrote:
> Hi,
>
> On Fri, Jul 18, 2014 at 11:16:17AM +0100, Martin Townsend wrote:
> > For IEEE802154_ADDR_NONE it now checks that the frame type for
> > acknowledgement. For the reserved dest mode it frees the skb and
> > returns.
> >
> > Signed-off-by: Martin Townsend <martin.towns...@xsilon.com>
> > ---
> > net/mac802154/wpan.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
> > index 3c3069f..70d5307 100644
> > --- a/net/mac802154/wpan.c
> > +++ b/net/mac802154/wpan.c
> > @@ -433,7 +433,7 @@ mac802154_subif_frame(struct
> > mac802154_sub_if_data *sdata, struct sk_buff *skb,
> > switch (mac_cb(skb)->dest.mode) {
> > case IEEE802154_ADDR_NONE:
> > - if (mac_cb(skb)->dest.mode != IEEE802154_ADDR_NONE)
>
> Okay, this if branch is dead code, I agree with that.
>
> Another thing is this branch can only occur on acknowledgement frames,
> because this kind of frames doesn't contain addresses and it's the
> only kind which doesn't include any addresses.
>
> But we can't handle acknowledgement frames over software.
> According IEEE 802.15.4-2011 standard page 45 acknowledgement is time
> critical and need in between macSIFSPeriod after last symbol of data
> or mac frame.
>
> macSIFSPeriod is phy depended but we can't handle this time critical
> ack handling in software. Some phy's like the at86rf2xx support ack
> handling via hardware. We should drop any ack frames which we receive
> and don't send any ack frames.
>
> We should do nothing here, only a "break;", but first check on frame
> type and do nothing.
>
> > + if (mac_cb(skb)->type != IEEE802154_FC_TYPE_ACK)
> > /* FIXME: check if we are PAN coordinator
> > */ skb->pkt_type = PACKET_OTHERHOST;
> > else
> > @@ -462,7 +462,9 @@ mac802154_subif_frame(struct
> > mac802154_sub_if_data *sdata, struct sk_buff *skb, skb->pkt_type =
> > PACKET_OTHERHOST; break;
> > default:
>
> this branch can't never happen and should be contain a BUG(); or
> something like that.
No, it should not. The addr mode can be invalid in corrupted frames or
because someone chose to just send an invalid mode to begin with, so
it's not a BUG() to receive one. Emitting a rate-limited warning is
fine, but locking up the machine because someone sent an invalid packet
seems rather harsh.
> > - break;
> > + spin_unlock_bh(&sdata->mib_lock);
> > + kfree_skb(skb);
> > + return NET_RX_DROP;
> > }
>
> - Alex
>
> ------------------------------------------------------------------------------
> Want fast and easy access to all the code in your enterprise? Index
> and search up to 200,000 lines of code with a free copy of Black Duck
> Code Sight - the same software that powers the world's largest code
> search on Ohloh, the Black Duck Open Hub! Try it now.
> http://p.sf.net/sfu/bds
> _______________________________________________
> Linux-zigbee-devel mailing list
> Linux-zigbee-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
