netdev - Re: [PATCH wpan-next v2 02/11] ieee802154: Internal PAN management

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230918161502.69818794@xps-13>
Date: Mon, 18 Sep 2023 16:15:02 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Alexander Aring <aahringo@...hat.com>
Cc: Stefan Schmidt <stefan@...enfreihafen.org>, Alexander Aring
 <alex.aring@...il.com>, linux-wpan@...r.kernel.org, "David S. Miller"
 <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
 <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
 netdev@...r.kernel.org, David Girault <david.girault@...vo.com>, Romuald
 Despres <romuald.despres@...vo.com>, Frederic Blain
 <frederic.blain@...vo.com>, Nicolas Schodet <nico@...fr.eu.org>, Guilhem
 Imberton <guilhem.imberton@...vo.com>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH wpan-next v2 02/11] ieee802154: Internal PAN management

Hi Alexander,

> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * IEEE 802.15.4 PAN management
> > > > > + *
> > > > > + * Copyright (C) 2021 Qorvo US, Inc
> > > > > + * Authors:
> > > > > + *   - David Girault <david.girault@...vo.com>
> > > > > + *   - Miquel Raynal <miquel.raynal@...tlin.com>
> > > > > + */
> > > > > +
> > > > > +#include <linux/kernel.h>
> > > > > +#include <net/cfg802154.h>
> > > > > +#include <net/af_ieee802154.h>
> > > > > +
> > > > > +static bool cfg802154_same_addr(struct ieee802154_pan_device *a,
> > > > > +                             struct ieee802154_addr *b)
> > > > > +{
> > > > > +     if (!a || !b)
> > > > > +             return false;
> > > > > +
> > > > > +     switch (b->mode) {
> > > > > +     case IEEE802154_ADDR_SHORT:
> > > > > +             return a->short_addr == b->short_addr;
> > > > > +     case IEEE802154_ADDR_LONG:
> > > > > +             return a->extended_addr == b->extended_addr;
> > > > > +     default:
> > > > > +             return false;
> > > > > +     }
> > > > > +}  
> > > >
> > > > Don't we already have such a helper already?  
> > >
> > > There must also be a check on (a->mode != b->mode) because short_addr
> > > and extended_addr share memory in this struct.  
> >
> > True.
> >
> > Actually the ieee802154_addr structure uses an enum to store either
> > the short address or the extended addres, while at the MAC level I'd
> > like to compare with what I call a ieee802154_pan_device: the PAN
> > device is part of a list defining the associated neighbors and contains
> > both an extended address and a short address once associated.
> >
> > I do not want to compare the PAN ID here and I do not need to compare
> > if the modes are different because the device the code is running on
> > is known to have both an extended address and a short address field
> > which have been initialized.
> >  
> 
> I see, so it is guaranteed that the mode value is the same?

I looked more carefully at the code of the association section,
we will always know the extended address of the devices which are
associated to us, however there may be situations where the second
device to compare with this list only comes with a short address and pan
ID, so your initial comment needs to be addressed.

> > With all these constraints, I think it would require more code to
> > re-use that small function than just writing a slightly different one
> > here which fully covers the "under association/disassociation" case, no?
> >  
> 
> I am questioning here currently myself if it's enough to uniquely
> identify devices with only short or extended. For extended I would say
> yes, for short I would say no.

As long as we know the PAN ID, it should be fine.

> Somehow I don't get it, maybe because
> it's on the setup to be associated and we know the panid already but
> it is not being set at this point?

Yes, this code is used *when* we associate or disassociate.

Thanks,
Miquèl