linux-kernel - RE: [PATCH] net/socket: fix GCC8+ Wpacked-not-aligned warnings

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] [day] [month] [year] [list]

Date:   Tue, 30 Jul 2019 13:23:14 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Qian Cai' <cai@....pw>,
        "davem@...emloft.net" <davem@...emloft.net>
CC:     "vyasevich@...il.com" <vyasevich@...il.com>,
        "nhorman@...driver.com" <nhorman@...driver.com>,
        "marcelo.leitner@...il.com" <marcelo.leitner@...il.com>,
        "linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] net/socket: fix GCC8+ Wpacked-not-aligned warnings

From: Qian Cai 
> Sent: 30 July 2019 14:18
> On Tue, 2019-07-30 at 09:01 +0000, David Laight wrote:
> > From: Qian Cai
> > > Sent: 29 July 2019 21:24
> >
> > ..
> > > To fix this, "struct sockaddr_storage" needs to be aligned to 4-byte as
> > > it is only used in those packed sctp structure which is part of UAPI,
> > > and "struct __kernel_sockaddr_storage" is used in some other
> > > places of UAPI that need not to change alignments in order to not
> > > breaking userspace.
> > >
> > > One option is use typedef between "sockaddr_storage" and
> > > "__kernel_sockaddr_storage" but it needs to change 35 or 370 lines of
> > > codes. The other option is to duplicate this simple 2-field structure to
> > > have a separate "struct sockaddr_storage" so it can use a different
> > > alignment than "__kernel_sockaddr_storage". Also the structure seems
> > > stable enough, so it will be low-maintenance to update two structures in
> > > the future in case of changes.
> >
> > Does it all work if the 8 byte alignment is implicit, not explicit?
> > For instance if unnamed union and struct are used eg:
> >
> > struct sockaddr_storage {
> > 	union {
> > 		void * __ptr;  /* Force alignment */
> > 		struct {
> > 			__kernel_sa_family_t	ss_family;		/* address family */
> > 			/* Following field(s) are implementation specific */
> > 			char	__data[_K_SS_MAXSIZE - sizeof(unsigned short)];
> > 					/* space to achieve desired size, */
> > 					/* _SS_MAXSIZE value minus size of ss_family */
> > 		};
> > 	};
> > };
> >
> > I suspect unnamed unions and structs have to be allowed by the compiler.
> 
> I believe this will suffer the same problem in that will break UAPI,
> 
> https://lore.kernel.org/lkml/20190726213045.GL6204@localhost.localdomain/

You are missing the bit where the UAPI structure is packed.
If the compiler won't let you 'pack' a structure that contains structures
(rather than just integers) then the compiler is broken!

The hope here was that it would be ok is the alignment was implicit not explicit.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)