On Fri, Jul 18, 2014 at 04:57:16PM +0200, Phoebe Buckheister wrote:
> 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.
>
I think mode is a masked value with 0x3 and can only between 0 - 3
and "1" is a reserved address type. This is actually the default branch.
Maybe we can add the RESERVED ADDR type and handle this with a warning
and add in default a BUG().
- 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
