linux-kernel - 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, 04 Mar 2021 20:44:54 -0700
From:   subashab@...eaurora.org
To:     Alex Elder <elder@...aro.org>
Cc:     stranche@...eaurora.org, davem@...emloft.net, kuba@...nel.org,
        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 2021-03-04 15:34, 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.
> 
> 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(-)

Can you share what all tests have been done with these patches