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 Aug 2022 09:43:02 +0000
From:   "Drewek, Wojciech" <>
To:     "Torvalds, Linus" <>,
        Paolo Abeni <>,
        "Gustavo A. R. Silva" <>,
        "Nguyen, Anthony L" <>
CC:     "" <>,
        "" <>,
        "" <>,
        "" <>
Subject: RE: [GIT PULL] Networking for 6.0

> -----Original Message-----
> From: Linus Torvalds <>
> Sent: czwartek, 4 sierpnia 2022 01:36
> To: Paolo Abeni <>; Gustavo A. R. Silva <>; Drewek, Wojciech
> <>; Nguyen, Anthony L <>
> Cc:;;;
> Subject: Re: [GIT PULL] Networking for 6.0
> On Wed, Aug 3, 2022 at 3:15 AM Paolo Abeni <> wrote:
> >
> > At the time of writing we have two known conflicts, one with arm-soc:
> Hmm. There's actually a third one, this one semantic (but mostly
> harmless). I wonder how it was overlooked.
> It causes an odd gcc "note" report:
>   net/core/flow_dissector.c: In function ‘is_pppoe_ses_hdr_valid’:
>   net/core/flow_dissector.c:898:13: note: the ABI of passing struct
> with a flexible array member has changed in GCC 4.4
>   898 | static bool is_pppoe_ses_hdr_valid(struct pppoe_hdr hdr)
>       |             ^~~~~~~~~~~~~~~~~~~~~~
> and it looks like a semantic merge conflict between commits
>   94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with
> flexible-array members")
>   46126db9c861 ("flow_dissector: Add PPPoE dissectors")
> where that first commit makes 'struct pppoe_hdr' have a flexible array
> member at the end, and the second second commit passes said pppoe_hdr
> by value as an argument.
> I don't think there is any reason to pass that 'struct pppoe_hdr' by
> value in the first place, and that is not a normal pattern for the
> kernel. Sure, we sometimes do use opaque types that may be structures
> (eg 'pte_t') by value as arguments, but that is not how that code is
> written.
> Any sane compiler will inline that thing anyway, so the end result
> ends up being the same, but passing a structure with an array at the
> end (whether zero-sized or flexible) by value is just cray-cray, to
> use the technical term.
> So I resolved this semantic conflict by simply making that function
> take a 'const struct pppoe_hdr *hdr' argument instead. That's the
> proper way.
> Why was this not noticed in linux-next? Is it because nobody actually
> *looks* at the output? Because it's a "note" and not a "warning", it
> ends up not aborting the build, but I do think the compiler is
> pointing out a very real issue.
> It would be perhaps worthwhile looking at code that passes structures
> by value as arguments (or as return values). It can generate truly
> horrendously bad code, and even when said structures are small, it's
> uisually not the right thing to do.
> And yes, as noted, we sometimes do have that pattern very much on
> purpose, sometimes because of abstraction reasons (pte_t) and
> sometimes because we explicitly want the magic "two words of result"
> ('struct fd' and fdget()).
> So it's not a strict no-no, but it's not generally a good idea unless
> you have a very good reason for it (and it's particularly not a good
> idea when there's an array at the end).
> I've fixed this up in my tree, and it's all fine (and while I'm not
> *happy* with the fact that apparently nobody looks at linux-next
> output, I guess it is what it is).
>               Linus

Thanks for fixing that.
I'll pay more attention in the future when passing structures
by value.


Powered by blists - more mailing lists