netdev - RE: Non-packed structures in IP headers

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: <DB9PR05MB7898339C06B9317EBAB13437E7AE9@DB9PR05MB7898.eurprd05.prod.outlook.com>
Date:   Mon, 4 Oct 2021 10:30:34 +0000
From:   "Cufi, Carles" <Carles.Cufi@...dicsemi.no>
To:     Florian Weimer <fweimer@...hat.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "jukka.rissanen@...ux.intel.com" <jukka.rissanen@...ux.intel.com>,
        "johan.hedberg@...el.com" <johan.hedberg@...el.com>,
        "Lubos, Robert" <Robert.Lubos@...dicsemi.no>,
        "Bursztyka, Tomasz" <tomasz.bursztyka@...el.com>,
        "linux-toolchains@...r.kernel.org" <linux-toolchains@...r.kernel.org>
Subject: RE: Non-packed structures in IP headers

Hi Florian,

Thanks for your response.

> 
> * Carles Cufi:
> 
> > I was looking through the structures for IPv{4,6} packet headers and
> > noticed that several of those that seem to be used to parse a packet
> > directly from the wire are not declared as packed. This surprised me
> > because, although I did find that provisions are made so that the
> > alignment of the structure, it is still technically possible for the
> > compiler to inject padding bytes inside those structures, since AFAIK
> > the C standard makes no guarantees about padding unless it's
> > instructed to pack the structure.
> 
> The C standards do not make such guarantees, but the platform ABI
> standards describe struct layout and ensure that there is no padding.
> Linux relies on that not just for networking, but also for the userspace
> ABI, support for separately compiled kernel modules, and in other places.

That makes sense, but aren't ABI standards different for every architecture? For example, I checked the Arm AAPCS[1] and it states:

"The size of an aggregate shall be the smallest multiple of its alignment that is sufficient to hold all of its
members."

Which, unless I am reading this wrong, means that the compiler would indeed insert padding if the size of the IP headers structs was not a multiple of 4. In this particular case, the struct sizes for the IP headers are 20 and 40 bytes respectively, so there will be no padding inserted. But I only checked a single architecture's ABI (or Procedure Call Standard) documentation, is this true for all archs? 

> Sometimes there are alignment concerns in the way these structs are used,
> but I believe the kernel generally controls placement of the data that is
> being worked on, so that does not matter, either.

I did see those when browsing the code, thanks for confirming this. It is really padding that I am concerned about, and not alignment.

> Therefore, I do not believe this is an actual problem.

Would the static assert still make sense in order to check this for all architectures?

Thanks,

Carles

[1] https://github.com/ARM-software/abi-aa/blob/2bcab1e3b22d55170c563c3c7940134089176746/aapcs64/aapcs64.rst#aggregates