From: "Ivan Skytte Jørgensen" <isj-sctp@i1.dk>
To: linux-sctp@vger.kernel.org
Subject: Re: packed structures used in socket options
Date: Sun, 07 Jun 2020 21:35:19 +0000 [thread overview]
Message-ID: <2213135.ChUyxVVRYb@isjsys> (raw)
In-Reply-To: <CBFEFEF1-127A-4ADA-B438-B171B9E26282@lurchi.franken.de>
On Sunday, 7 June 2020 22:21:41 CEST you wrote:
> From: Michael Tuexen
> > Sent: 07 June 2020 18:24
> > > On 7. Jun 2020, at 19:14, David Laight <David.Laight@ACULAB.COM> wrote:
> > >
> > > From: Michael Tuexen <Michael.Tuexen@lurchi.franken.de>
> > >> Sent: 07 June 2020 16:15
> > >>> On 7. Jun 2020, at 15:53, David Laight <David.Laight@ACULAB.COM> wrote:
> > >>>
> > >>> From: Michael Tuexen
> > >>>>
> > >>>> since gcc uses -Werrordress-of-packed-member, I get warnings for my variant
> > >>>> of packetdrill, which supports SCTP.
> > >>>>
> > >>>> Here is why:
> > >>>>
> > >>>>
> > >>
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/sctp.h?h=v5
> > >>>> .7
> > >>>> contains:
> > >>>>
> > >>>> struct sctp_paddrparams {
> > >>>> sctp_assoc_t spp_assoc_id;
> > >>>> struct sockaddr_storage spp_address;
> > >>>> __u32 spp_hbinterval;
> > >>>> __u16 spp_pathmaxrxt;
> > >>>> __u32 spp_pathmtu;
> > >>>> __u32 spp_sackdelay;
> > >>>> __u32 spp_flags;
> > >>>> __u32 spp_ipv6_flowlabel;
> > >>>> __u8 spp_dscp;
> > >>>> } __attribute__((packed, aligned(4)));
> > >>>>
> > >>>> This structure is only used in the IPPROTO_SCTP level socket option SCTP_PEER_ADDR_PARAMS.
> > >>>> Why is it packed?
> > >>>
> > >>> I'm guessing 'to remove holes to avoid leaking kernel data'.
> > >>>
> > >>> The sctp socket api defines loads of structures that will have
> > >>> holes in them if not packed.
> > >>
> > >> Hi David,
> > >> I agree that they have holes and we should have done better. The
> > >> kernel definitely should also not leak kernel data. However, the
> > >> way to handle this shouldn't be packing. I guess it is too late
> > >> to change this?
> > >
> > > Probably too late.
> > > I've no idea how it got through the standards body either.
> > > In fact, the standard may actually require the holes.
> >
> > No, it does not. Avoiding holes was not taken into account.
>
> It depends on whether the rfc that describes the sockops says
> the structures 'look like this' or 'contain the following members'.
>
> > It should have been, but this was missed. Authors of all
> > kernel implementation (FreeBSD, Linux, and Solaris) were involved.
>
> Sounds like none of the right people even looked at it.
I was involved. At that time (September 2005) the SCTP API was still evolving (first finalized in 2011), and one of the major users of the API was 32-bit programs running on 64-bit kernel (on powerpc as I recall). When we realized that the structures were different between 32bit and 64bit we had to break the least number of programs, and the result were those ((packed)) structs so 32-bit programs wouldn't be broken and we didn't need a xxx_compat translation layer in the kernel.
I don't have access to TSVWG mailing list archive that far back but I found I wrote this summary here:
On Sunday, 25 September 2005 21:36:05 CEST Ivan Skytte Jørgensen wrote:
> I followed the discussion in tsvwg mailing list. My interpretation of the few
> answers is that this is a "quality of implementation issue" and that padding
> fields are allowed but won't get into the RFC because it is an implementation
> issue.
So RFC6458 allows padding but doesn't list them.
Incidentally, at that time (and perhaps still) sockaddr_storage had different alignement between 32-bit programs and 64-bit programs, and the multicast structures used in setsockopt() (group_req, group_source_req and group_filter) had/has the same problem.
/isj
next prev parent reply other threads:[~2020-06-07 21:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-07 10:49 packed structures used in socket options Michael Tuexen
2020-06-07 13:53 ` David Laight
2020-06-07 15:15 ` Michael Tuexen
2020-06-07 17:14 ` David Laight
2020-06-07 17:23 ` Michael Tuexen
2020-06-07 20:21 ` David Laight
2020-06-07 21:35 ` Ivan Skytte Jørgensen [this message]
2020-06-08 16:18 ` David Laight
2020-06-08 17:37 ` Michael Tuexen
2020-06-08 21:13 ` David Laight
2020-06-07 21:51 ` Michael Tuexen
2020-06-08 8:17 ` David Laight
2020-06-07 21:55 ` Michael Tuexen
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=2213135.ChUyxVVRYb@isjsys \
--to=isj-sctp@i1.dk \
--cc=linux-sctp@vger.kernel.org \
/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).