netdev - Re: [PATCH wpan-next] mac802154: Avoid superfluous endianness handling

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: <20230131120346.65d42f25@xps-13>
Date:   Tue, 31 Jan 2023 12:03:46 +0100
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>,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH wpan-next] mac802154: Avoid superfluous endianness
 handling

Hi Alexander,

aahringo@...hat.com wrote on Mon, 30 Jan 2023 11:41:20 -0500:

> Hi,
> 
> On Mon, Jan 30, 2023 at 11:34 AM Stefan Schmidt
> <stefan@...enfreihafen.org> wrote:
> >
> > Hello.
> >
> > On 30.01.23 16:43, Miquel Raynal wrote:  
> > > When compiling scan.c with C=1, Sparse complains with:
> > >
> > >     sparse:     expected unsigned short [usertype] val
> > >     sparse:     got restricted __le16 [usertype] pan_id
> > >     sparse: sparse: cast from restricted __le16
> > >
> > >     sparse:     expected unsigned long long [usertype] val
> > >     sparse:     got restricted __le64 [usertype] extended_addr
> > >     sparse: sparse: cast from restricted __le64
> > >
> > > The tool is right, both pan_id and extended_addr already are rightfully
> > > defined as being __le16 and __le64 on both sides of the operations and
> > > do not require extra endianness handling.
> > >
> > > Fixes: 3accf4762734 ("mac802154: Handle basic beaconing")
> > > Reported-by: kernel test robot <lkp@...el.com>
> > > Signed-off-by: Miquel Raynal <miquel.raynal@...tlin.com>
> > > ---
> > >   net/mac802154/scan.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
> > > index cfbe20b1ec5e..8f98efec7753 100644
> > > --- a/net/mac802154/scan.c
> > > +++ b/net/mac802154/scan.c
> > > @@ -419,8 +419,8 @@ int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata,
> > >       local->beacon.mhr.fc.source_addr_mode = IEEE802154_EXTENDED_ADDRESSING;
> > >       atomic_set(&request->wpan_dev->bsn, -1);
> > >       local->beacon.mhr.source.mode = IEEE802154_ADDR_LONG;
> > > -     local->beacon.mhr.source.pan_id = cpu_to_le16(request->wpan_dev->pan_id);
> > > -     local->beacon.mhr.source.extended_addr = cpu_to_le64(request->wpan_dev->extended_addr);
> > > +     local->beacon.mhr.source.pan_id = request->wpan_dev->pan_id;
> > > +     local->beacon.mhr.source.extended_addr = request->wpan_dev->extended_addr;
> > >       local->beacon.mac_pl.beacon_order = request->interval;
> > >       local->beacon.mac_pl.superframe_order = request->interval;
> > >       local->beacon.mac_pl.final_cap_slot = 0xf;  
> >
> > This patch has been applied to the wpan-next tree and will be
> > part of the next pull request to net-next. Thanks!  
> 
> fyi: in my opinion, depending on system endianness this is actually a bug.

Actually there are many uses of __le16 and __le64 for PAN IDs, short
and extended addresses. I did follow the existing patterns, I think
they are legitimate. Can you clarify what you think is a bug in the
current state? I always feel a bit flaky when it comes to properly
handling endianness, so all feedback welcome, if you have any hints
of what should be fixed after this patch, I'll do it. 

Thanks,
Miquèl