linux-kernel - Re: [PATCH] ath9k: fix calibration data endianness

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: <6c0c6f91-9b5a-4d0a-b92d-4daf5775f27d@lunn.ch>
Date:   Sun, 16 Apr 2023 16:44:24 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Christian Lamparter <chunkeey@...il.com>
Cc:     Toke Høiland-Jørgensen <toke@...e.dk>,
        Álvaro Fernández Rojas <noltari@...il.com>,
        f.fainelli@...il.com, jonas.gorski@...il.com, nbd@....name,
        kvalo@...nel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ath9k: fix calibration data endianness

> |       if (ah->eep_ops->get_eepmisc(ah) & AR5416_EEPMISC_BIG_ENDIAN) {
> |               *swap_needed = true;
> |               ath_dbg(common, EEPROM,
> |                       "Big Endian EEPROM detected according to EEPMISC register.\n");
> |       } else {
> |               *swap_needed = false;
> |       }
> 
> This doesn't take into consideration that swapping is not needed if
> the data is in big endian format on a big endian device. So, this
> could be changed so that the *swap_needed is only true if the flag and
> device endiannes disagrees?

There are versions of the macro which performs the swap which
understands the CPU endianness and become a NOP when it is not
required. htons()/ntohs() are the classic examples. So you need to
consider:

Despite swap_needed being true, it is possible no swap it actually
happening, because such a macro is being used.

and

Maybe using these variant can make the code simpler, by just doing the
NOP swap when the CPU endianess does not require it.

    Andrew