| 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
| ||
|
Message-ID: <Z/e7p53g/e0wNYF+@gmail.com>
Date: Thu, 10 Apr 2025 05:37:59 -0700
From: Breno Leitao <leitao@...ian.org>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: kernel test robot <lkp@...el.com>, sean.anderson@...o.com,
bigeasy@...utronix.de, horia.geanta@....com,
madalin.bucur@....nxp.com, hristophe.leroy@...roup.eu,
oe-kbuild-all@...ts.linux.dev, linux-kernel@...r.kernel.org,
Jakub Kicinski <kuba@...nel.org>
Subject: Re: include/soc/fsl/qman.h:383:4: warning: field context_a within
'struct qm_fqd' is less aligned than 'union (unnamed union at
include/soc/fsl/qman.h:365:2)' and is usually due to 'struct qm_fqd' being
packed, which can lead to unaligned accesses
On Wed, Apr 09, 2025 at 03:01:38PM +0300, Vladimir Oltean wrote:
> Hi Breno,
>
> On Wed, Apr 09, 2025 at 04:18:04AM -0700, Breno Leitao wrote:
> > On Wed, Apr 09, 2025 at 04:55:16PM +0800, kernel test robot wrote:
> > > >> include/soc/fsl/qman.h:383:4: warning: field context_a within 'struct qm_fqd' is less aligned than 'union (unnamed union at include/soc/fsl/qman.h:365:2)' and is usually due to 'struct qm_fqd' being packed, which can lead to unaligned accesses [-Wunaligned-access]
> > > } context_a;
> >
> > <snip>
> >
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 346
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 347 struct qm_fqd {
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 348 /* _res[6-7], orprws[3-5], oa[2], olws[0-1] */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 349 u8 orpc;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 350 u8 cgid;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 351 __be16 fq_ctrl; /* See QM_FQCTRL_<...> */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 352 __be16 dest_wq; /* channel[3-15], wq[0-2] */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 353 __be16 ics_cred; /* 15-bit */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 354 /*
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 355 * For "Initialize Frame Queue" commands, the write-enable mask
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 356 * determines whether 'td' or 'oac_init' is observed. For query
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 357 * commands, this field is always 'td', and 'oac_query' (below) reflects
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 358 * the Overhead ACcounting values.
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 359 */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 360 union {
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 361 __be16 td; /* "Taildrop": _res[13-15], mant[5-12], exp[0-4] */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 362 struct qm_fqd_oac oac_init;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 363 };
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 364 __be32 context_b;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 365 union {
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 366 /* Treat it as 64-bit opaque */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 367 __be64 opaque;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 368 struct {
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 369 __be32 hi;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 370 __be32 lo;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 371 };
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 372 /* Treat it as s/w portal stashing config */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 373 /* see "FQD Context_A field used for [...]" */
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 374 struct {
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 375 struct qm_fqd_stashing stashing;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 376 /*
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 377 * 48-bit address of FQ context to
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 378 * stash, must be cacheline-aligned
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 379 */
> >
> > Looking at pahole, I am not sure this is caacheline-aligned, as
> > suggested above.
> >
> > # pahole -C qm_fqd
> > struct qm_fqd {
> > u8 orpc; /* 0 1 */
> > u8 cgid; /* 1 1 */
> > __be16 fq_ctrl; /* 2 2 */
> > __be16 dest_wq; /* 4 2 */
> > __be16 ics_cred; /* 6 2 */
> > union {
> > __be16 td; /* 8 2 */
> > struct qm_fqd_oac oac_init; /* 8 2 */
> > }; /* 8 2 */
> > __be32 context_b; /* 10 4 */
> > union {
> > __be64 opaque; /* 14 8 */
> > struct {
> > __be32 hi; /* 14 4 */
> > __be32 lo; /* 18 4 */
> > }; /* 14 8 */
> > struct {
> > struct qm_fqd_stashing stashing; /* 14 2 */
> > __be16 context_hi; /* 16 2 */
> > __be32 context_lo; /* 18 4 */
> > }; /* 14 8 */
> > } context_a; /* 14 8 */
> > struct qm_fqd_oac oac_query; /* 22 2 */
> >
> > /* size: 24, cachelines: 1, members: 9 */
> > /* last cacheline: 24 bytes */
> > } __attribute__((__packed__));
> >
> >
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 380 __be16 context_hi;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 381 __be32 context_lo;
> > > c535e923bb97a4 Claudiu Manoil 2016-09-22 382 } __packed;
> >
> > I am wondering if we should use __attribute__((aligned(8))) instead of
> > __packed, according to the document above.
>
> Since you've replied, could you please help me also understand the issue?
> It says "field context_a within 'struct qm_fqd' is less aligned than
> 'union (unnamed union at include/soc/fsl/qman.h:365:2)'", but at line 365,
> the unnamed union _is_ field context_a, no? What is it saying?
>
> I _suspect_, but I'm not sure, that it's complaining about context_a.opaque
> being a 64-bit field which is not aligned to a 64-bit boundary. Though
> I'm not getting that from reading the description.
In fact, the whole union is not aligned, and because "struct qm_fqd" is
also packed, it cannot add a padding before the union (which would be
the compiler decision, since the union contains a packed field).
> But struct qm_fqd is embedded in other structures, and the alignment
> of the fields changes in that case. I think that context_a.opaque is
> 64-bit aligned in all cases.
I don't think it is ever aligned. Check the pahole above, and the union
starts at byte 14th, which is not aligned if my math is correct.
--breno
Powered by blists - more mailing lists