linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Qian Cai' <cai@lca.pw>, "davem@davemloft.net" <davem@davemloft.net>
Cc: "vyasevich@gmail.com" <vyasevich@gmail.com>,
	"nhorman@tuxdriver.com" <nhorman@tuxdriver.com>,
	"marcelo.leitner@gmail.com" <marcelo.leitner@gmail.com>,
	"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] net/socket: fix GCC8+ Wpacked-not-aligned warnings
Date: Tue, 30 Jul 2019 13:23:14 +0000	[thread overview]
Message-ID: <a4b03ad38f104bc0b2f9e256a9cade00@AcuMS.aculab.com> (raw)
In-Reply-To: <1564492704.11067.28.camel@lca.pw>

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)

      reply	other threads:[~2019-07-30 13:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-29 20:23 [PATCH] net/socket: fix GCC8+ Wpacked-not-aligned warnings Qian Cai
2019-07-30  9:01 ` David Laight
2019-07-30 13:18   ` Qian Cai
2019-07-30 13:23     ` David Laight [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a4b03ad38f104bc0b2f9e256a9cade00@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=cai@lca.pw \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=vyasevich@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).