netdev - Re: [PATCH net-next 0/6] net: qualcomm: rmnet: stop using C bit-fields

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]

Date:   Thu, 4 Mar 2021 16:43:33 -0600
From:   Alex Elder <elder@...aro.org>
To:     subashab@...eaurora.org, stranche@...eaurora.org,
        davem@...emloft.net, kuba@...nel.org
Cc:     sharathv@...eaurora.org, bjorn.andersson@...aro.org,
        evgreen@...omium.org, cpratapa@...eaurora.org, elder@...nel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 0/6] net: qualcomm: rmnet: stop using C
 bit-fields

On 3/4/21 4:34 PM, Alex Elder wrote:
> This series converts data structures defined in <linux/if_rmnet.h>
> so they use integral field values with bitfield masks rather than
> rely on C bit-fields.

Whoops!  I forgot to check if net-next was open.  I'm very
sorry about that...

   http://vger.kernel.org/~davem/net-next.html

					-Alex

> I first proposed doing something like this long ago when my confusion
> about this code (and the memory layout it was supposed to represent)
> led me to believe it was erroneous:
>    https://lore.kernel.org/netdev/20190520135354.18628-1-elder@linaro.org/
> 
> It came up again recently, when Sharath Chandra Vurukala proposed
> a new structure in "if_rmnet.h", again using C bit-fields.  I asked
> whether the new structure could use field masks, and Jakub requested
> that this be done.
>    https://lore.kernel.org/netdev/1613079324-20166-1-git-send-email-sharathv@codeaurora.org/
> I volunteered to convert the existing RMNet code to use bitfield
> masks, and that is what I'm doing here.
> 
> The first three patches are more or less preparation work for the
> last three.
>    - The first marks two fields in an existing structure explicitly
>      big endian.  They are unused by current code, so this should
>      have no impact.
>    - The second simplifies some code that computes the value of a
>      field in a header in a somewhat obfuscated way.
>    - The third eliminates some trivial accessor macros, open-coding
>      them instead.  I believe the accessors actually do more harm
>      than good.
>    - The last three convert the structures defined in "if_rmnet.h"
>      so they are defined only with integral fields, each having
>      well-defined byte order.  Where sub-fields are needed, field
>      masks are defined so they can be encoded or extracted using
>      functions like be16_get_bits() or u8_encode_bits(), defined
>      in <linux/bitfield.h>.  The three structures converted are,
>      in order:  rmnet_map_header, rmnet_map_dl_csum_trailer, and
>      rmnet_map_ul_csum_header.
> 
> 					-Alex
> 
> Alex Elder (6):
>    net: qualcomm: rmnet: mark trailer field endianness
>    net: qualcomm: rmnet: simplify some byte order logic
>    net: qualcomm: rmnet: kill RMNET_MAP_GET_*() accessor macros
>    net: qualcomm: rmnet: use field masks instead of C bit-fields
>    net: qualcomm: rmnet: don't use C bit-fields in rmnet checksum trailer
>    net: qualcomm: rmnet: don't use C bit-fields in rmnet checksum header
> 
>   .../ethernet/qualcomm/rmnet/rmnet_handlers.c  | 11 ++--
>   .../net/ethernet/qualcomm/rmnet/rmnet_map.h   | 12 ----
>   .../qualcomm/rmnet/rmnet_map_command.c        | 11 +++-
>   .../ethernet/qualcomm/rmnet/rmnet_map_data.c  | 60 ++++++++---------
>   include/linux/if_rmnet.h                      | 65 +++++++++----------
>   5 files changed, 70 insertions(+), 89 deletions(-)
>