netdev - Re: [PATCH net-next resend 1/2] enetc: Fix endianness issues for enetc

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: <20201119095403.63325987@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Thu, 19 Nov 2020 09:54:03 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Denis Kirjanov <kda@...ux-powerpc.org>
Cc:     Claudiu Manoil <claudiu.manoil@....com>, netdev@...r.kernel.org,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next resend 1/2] enetc: Fix endianness issues for
 enetc_ethtool

On Thu, 19 Nov 2020 13:37:21 +0300 Denis Kirjanov wrote:
> On 11/19/20, Claudiu Manoil <claudiu.manoil@....com> wrote:
> > These particular fields are specified in the H/W reference
> > manual as having network byte order format, so enforce big
> > endian annotation for them and clear the related sparse
> > warnings in the process.
> >
> > Signed-off-by: Claudiu Manoil <claudiu.manoil@....com>
> > ---
> >  drivers/net/ethernet/freescale/enetc/enetc_hw.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > index 68ef4f959982..04efccd11162 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > @@ -472,10 +472,10 @@ struct enetc_cmd_rfse {
> >  	u8 smac_m[6];
> >  	u8 dmac_h[6];
> >  	u8 dmac_m[6];
> > -	u32 sip_h[4];
> > -	u32 sip_m[4];
> > -	u32 dip_h[4];
> > -	u32 dip_m[4];
> > +	__be32 sip_h[4];
> > +	__be32 sip_m[4];
> > +	__be32 dip_h[4];
> > +	__be32 dip_m[4];
> >  	u16 ethtype_h;
> >  	u16 ethtype_m;
> >  	u16 ethtype4_h;  
> 
> Hi Claudiu,
> 
> Why the struct is declared without packed?
> I'm seeing that the structure is used in dma transfers in the driver

We prefer not to pack structs unnecessarily in netdev, because it
forces compilers to do inefficient loads on some arches. If the
structure is laid out correctly according to normal C data layout rules
it should be left alone. 

You can add compile times assertions on the size of the structures
to make double-sure things don't break.