| 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
| ||
|
Date: Sun, 14 Apr 2024 08:20:44 +0000
From: Danielle Ratson <danieller@...dia.com>
To: Russell King <linux@...linux.org.uk>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "davem@...emloft.net"
<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
"corbet@....net" <corbet@....net>, "sdf@...gle.com" <sdf@...gle.com>,
"kory.maincent@...tlin.com" <kory.maincent@...tlin.com>,
"maxime.chevallier@...tlin.com" <maxime.chevallier@...tlin.com>,
"vladimir.oltean@....com" <vladimir.oltean@....com>,
"przemyslaw.kitszel@...el.com" <przemyslaw.kitszel@...el.com>,
"ahmed.zaki@...el.com" <ahmed.zaki@...el.com>, "richardcochran@...il.com"
<richardcochran@...il.com>, "shayagr@...zon.com" <shayagr@...zon.com>,
"paul.greenwalt@...el.com" <paul.greenwalt@...el.com>, "jiri@...nulli.us"
<jiri@...nulli.us>, "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, mlxsw
<mlxsw@...dia.com>, Petr Machata <petrm@...dia.com>, Ido Schimmel
<idosch@...dia.com>
Subject: RE: [PATCH net-next 07/10] ethtool: cmis_cdb: Add a layer for
supporting CDB commands
> -----Original Message-----
> From: Danielle Ratson <danieller@...dia.com>
> Sent: Tuesday, 9 April 2024 16:34
> To: Russell King <linux@...linux.org.uk>
> Cc: netdev@...r.kernel.org; davem@...emloft.net; edumazet@...gle.com;
> kuba@...nel.org; pabeni@...hat.com; corbet@....net; sdf@...gle.com;
> kory.maincent@...tlin.com; maxime.chevallier@...tlin.com;
> vladimir.oltean@....com; przemyslaw.kitszel@...el.com;
> ahmed.zaki@...el.com; richardcochran@...il.com; shayagr@...zon.com;
> paul.greenwalt@...el.com; jiri@...nulli.us; linux-doc@...r.kernel.org; linux-
> kernel@...r.kernel.org; mlxsw <mlxsw@...dia.com>; Petr Machata
> <petrm@...dia.com>; Ido Schimmel <idosch@...dia.com>
> Subject: RE: [PATCH net-next 07/10] ethtool: cmis_cdb: Add a layer for
> supporting CDB commands
>
> > -----Original Message-----
> > From: Russell King <linux@...linux.org.uk>
> > Sent: Monday, 8 April 2024 17:55
> > To: Danielle Ratson <danieller@...dia.com>
> > Cc: netdev@...r.kernel.org; davem@...emloft.net;
> edumazet@...gle.com;
> > kuba@...nel.org; pabeni@...hat.com; corbet@....net; sdf@...gle.com;
> > kory.maincent@...tlin.com; maxime.chevallier@...tlin.com;
> > vladimir.oltean@....com; przemyslaw.kitszel@...el.com;
> > ahmed.zaki@...el.com; richardcochran@...il.com; shayagr@...zon.com;
> > paul.greenwalt@...el.com; jiri@...nulli.us; linux-doc@...r.kernel.org;
> > linux- kernel@...r.kernel.org; mlxsw <mlxsw@...dia.com>; Petr Machata
> > <petrm@...dia.com>; Ido Schimmel <idosch@...dia.com>
> > Subject: Re: [PATCH net-next 07/10] ethtool: cmis_cdb: Add a layer for
> > supporting CDB commands
> >
> > On Mon, Apr 08, 2024 at 03:53:37PM +0300, Danielle Ratson wrote:
> > > +/**
> > > + * struct ethtool_cmis_cdb_request - CDB commands request fields as
> > decribed in
> > > + * the CMIS standard
> > > + * @id: Command ID.
> > > + * @epl_len: EPL memory length.
> > > + * @lpl_len: LPL memory length.
> > > + * @chk_code: Check code for the previous field and the payload.
> > > + * @resv1: Added to match the CMIS standard request continuity.
> > > + * @resv2: Added to match the CMIS standard request continuity.
> > > + * @payload: Payload for the CDB commands.
> > > + */
> > > +struct ethtool_cmis_cdb_request {
> > > + __be16 id;
> > > + struct_group(body,
> > > + u16 epl_len;
> >
> > u16 with a struct that also uses __be16 looks suspicious.
> >
> > > + u8 lpl_len;
> > > + u8 chk_code;
> > > + u8 resv1;
> > > + u8 resv2;
> > > + u8 payload[ETHTOOL_CMIS_CDB_LPL_MAX_PL_LENGTH];
> > > + );
> >
> > Does it matter if the compiler inserts some padding before this struct group?
>
> Yes it should matter since I copy this struct into a payload and it should be
> transferred like that to match the CMIS specification, but if I use __be16 for
> both id and epl_len it might resolve the issue isn’t it?
>
> >
> > > +/**
> > > + * struct ethtool_cmis_cdb_rpl_hdr - CDB commands reply header
> > > +arguments
> > > + * @rpl_len: Reply length.
> > > + * @rpl_chk_code: Reply check code.
> > > + */
> > > +struct ethtool_cmis_cdb_rpl_hdr {
> > > + u8 rpl_len;
> > > + u8 rpl_chk_code;
> >
> > Does it matter if the compiler adds some padding here?
>
> Kind of the same idea, it is matter if the compiler adds padding since the reply
> is read and extracted into this struct like it is written in the CMIS specification.
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
Please see below pahole's output after changing ethtool_cmis_cdb_request ::epl_len from u16 to __be16.
$ pahole -C 'ethtool_cmis_cdb_request' ./vmlinux
struct ethtool_cmis_cdb_request {
__be16 id; /* 0 2 */
union {
struct {
__be16 epl_len; /* 2 2 */
u8 lpl_len; /* 4 1 */
u8 chk_code; /* 5 1 */
u8 resv1; /* 6 1 */
u8 resv2; /* 7 1 */
u8 payload[120]; /* 8 120 */
}; /* 2 126 */
struct {
__be16 epl_len; /* 2 2 */
u8 lpl_len; /* 4 1 */
u8 chk_code; /* 5 1 */
u8 resv1; /* 6 1 */
u8 resv2; /* 7 1 */
u8 payload[120]; /* 8 120 */
} body; /* 2 126 */
}; /* 2 126 */
/* size: 128, cachelines: 2, members: 2 */
};
[danieller@...-r-vrt-155 linux_mlxsw]$ pahole -C 'ethtool_cmis_cdb_rpl' ./vmlinux
struct ethtool_cmis_cdb_rpl {
struct ethtool_cmis_cdb_rpl_hdr hdr; /* 0 2 */
u8 payload[120]; /* 2 120 */
/* size: 122, cachelines: 2, members: 2 */
/* last cacheline: 58 bytes */
};
[danieller@...-r-vrt-155 linux_mlxsw]$ pahole -C 'ethtool_cmis_cdb_rpl_hdr' ./vmlinux
struct ethtool_cmis_cdb_rpl_hdr {
u8 rpl_len; /* 0 1 */
u8 rpl_chk_code; /* 1 1 */
/* size: 2, cachelines: 1, members: 2 */
/* last cacheline: 2 bytes */
};
If a further fix is needed, you are welcome to let me know.
Thanks,
Danielle
Powered by blists - more mailing lists