netdev - Re: [PATCH wpan-next 1/6] ieee802154: Add support for user scanning requests

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20230213183535.05e62c1c@xps-13>
Date:   Mon, 13 Feb 2023 18:35:35 +0100
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Alexander Aring <aahringo@...hat.com>
Cc:     Alexander Aring <alex.aring@...il.com>,
        Stefan Schmidt <stefan@...enfreihafen.org>,
        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 1/6] ieee802154: Add support for user scanning
 requests

Hi Alexander,

> > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > +{
> > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > +       struct cfg802154_scan_request *request;
> > > > > +       u8 type;
> > > > > +       int err;
> > > > > +
> > > > > +       /* Monitors are not allowed to perform scans */
> > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > +               return -EPERM;  
> > > >
> > > > btw: why are monitors not allowed?  
> > >
> > > I guess I had the "active scan" use case in mind which of course does
> > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > right now I don't remember why I strongly refused scans on monitors.  
> >
> > Isn't it that scans really work close to phy level? Means in this case
> > we disable mostly everything of MAC filtering on the transceiver side.
> > Then I don't see any reasons why even monitors can't do anything, they
> > also can send something. But they really don't have any specific
> > source address set, so long addresses are none for source addresses, I
> > don't see any problem here. They also don't have AACK handling, but
> > it's not required for scan anyway...
> >
> > If this gets too complicated right now, then I am also fine with
> > returning an error here, we can enable it later but would it be better
> > to use ENOTSUPP or something like that in this case? EPERM sounds like
> > you can do that, but you don't have the permissions.
> >  
> 
> For me a scan should also be possible from iwpan phy $PHY scan (or
> whatever the scan command is, or just enable beacon)... to go over the
> dev is just a shortcut for "I mean whatever the phy is under this dev"
> ?

Actually only coordinators (in a specific state) should be able to send
beacons, so I am kind of against allowing that shortcut, because there
are usually two dev interfaces on top of the phy's, a regular "NODE"
and a "COORD", so I don't think we should go that way.

For scans however it makes sense, I've added the necessary changes in
wpan-tools. The TOP_LEVEL(scan) macro however does not support using
the same command name twice because it creates a macro, so this one
only supports a device name (the interface command has kind of the same
situation and uses a HIDDEN() macro which cannot be used here).

So in summary here is what is supported:
- dev <dev> beacon
- dev <dev> scan trigger|abort
- phy <phy> scan trigger|abort
- dev <dev> scan (the blocking one, which triggers, listens and returns)

Do you agree?

Thanks,
Miquèl