From: David Laight <David.Laight@ACULAB.COM>
To: 'Michael Tuexen' <Michael.Tuexen@lurchi.franken.de>
Cc: "Ivan Skytte Jørgensen" <isj-sctp@i1.dk>,
"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: RE: packed structures used in socket options
Date: Mon, 08 Jun 2020 21:13:15 +0000 [thread overview]
Message-ID: <529a772bd3ef40d3a310e78d613339ca@AcuMS.aculab.com> (raw)
In-Reply-To: <7BD347D7-562F-459D-B0CB-0BC798919876@lurchi.franken.de>
RnJvbTogTWljaGFlbCBUdWV4ZW4NCj4gU2VudDogMDggSnVuZSAyMDIwIDE4OjM3DQo+ID4gT24g
OC4gSnVuIDIwMjAsIGF0IDE4OjE4LCBEYXZpZCBMYWlnaHQgPERhdmlkLkxhaWdodEBBQ1VMQUIu
Q09NPiB3cm90ZToNCj4gPg0KPiA+IEZyb206IEl2YW4gU2t5dHRlIErDuHJnZW5zZW4NCj4gPj4g
U2VudDogMDcgSnVuZSAyMDIwIDIyOjM1DQo+ID4gLi4uDQo+ID4+Pj4+Pj4+IGNvbnRhaW5zOg0K
PiA+Pj4+Pj4+Pg0KPiA+Pj4+Pj4+PiBzdHJ1Y3Qgc2N0cF9wYWRkcnBhcmFtcyB7DQo+ID4+Pj4+
Pj4+IAlzY3RwX2Fzc29jX3QJCXNwcF9hc3NvY19pZDsNCj4gPj4+Pj4+Pj4gCXN0cnVjdCBzb2Nr
YWRkcl9zdG9yYWdlCXNwcF9hZGRyZXNzOw0KPiA+Pj4+Pj4+PiAJX191MzIJCQlzcHBfaGJpbnRl
cnZhbDsNCj4gPj4+Pj4+Pj4gCV9fdTE2CQkJc3BwX3BhdGhtYXhyeHQ7DQo+ID4+Pj4+Pj4+IAlf
X3UzMgkJCXNwcF9wYXRobXR1Ow0KPiA+Pj4+Pj4+PiAJX191MzIJCQlzcHBfc2Fja2RlbGF5Ow0K
PiA+Pj4+Pj4+PiAJX191MzIJCQlzcHBfZmxhZ3M7DQo+ID4+Pj4+Pj4+IAlfX3UzMgkJCXNwcF9p
cHY2X2Zsb3dsYWJlbDsNCj4gPj4+Pj4+Pj4gCV9fdTgJCQlzcHBfZHNjcDsNCj4gPj4+Pj4+Pj4g
fSBfX2F0dHJpYnV0ZV9fKChwYWNrZWQsIGFsaWduZWQoNCkpKTsNCj4gPj4+Pj4+Pj4NCj4gPj4+
Pj4+Pj4gVGhpcyBzdHJ1Y3R1cmUgaXMgb25seSB1c2VkIGluIHRoZSBJUFBST1RPX1NDVFAgbGV2
ZWwgc29ja2V0IG9wdGlvbiBTQ1RQX1BFRVJfQUREUl9QQVJBTVMuDQo+ID4+Pj4+Pj4+IFdoeSBp
cyBpdCBwYWNrZWQ/DQo+ID4gLi4uDQo+ID4+IEkgd2FzIGludm9sdmVkLiBBdCB0aGF0IHRpbWUg
KFNlcHRlbWJlciAyMDA1KSB0aGUgU0NUUCBBUEkgd2FzIHN0aWxsIGV2b2x2aW5nIChmaXJzdCBm
aW5hbGl6ZWQgaW4NCj4gPj4gMjAxMSksIGFuZCBvbmUgb2YgdGhlIG1ham9yIHVzZXJzIG9mIHRo
ZSBBUEkgd2FzIDMyLWJpdCBwcm9ncmFtcyBydW5uaW5nIG9uIDY0LWJpdCBrZXJuZWwgKG9uDQo+
IHBvd2VycGMNCj4gPj4gYXMgSSByZWNhbGwpLiBXaGVuIHdlIHJlYWxpemVkIHRoYXQgdGhlIHN0
cnVjdHVyZXMgd2VyZSBkaWZmZXJlbnQgYmV0d2VlbiAzMmJpdCBhbmQgNjRiaXQgd2UgaGFkIHRv
DQo+ID4+IGJyZWFrIHRoZSBsZWFzdCBudW1iZXIgb2YgcHJvZ3JhbXMsIGFuZCB0aGUgcmVzdWx0
IHdlcmUgdGhvc2UgKChwYWNrZWQpKSBzdHJ1Y3RzIHNvIDMyLWJpdCBwcm9ncmFtcw0KPiA+PiB3
b3VsZG4ndCBiZSBicm9rZW4gYW5kIHdlIGRpZG4ndCBuZWVkIGEgeHh4X2NvbXBhdCB0cmFuc2xh
dGlvbiBsYXllciBpbiB0aGUga2VybmVsLg0KPiA+DQo+ID4gSSB3YXMgYWxzbyBsb29raW5nIGF0
IGFsbCB0aGUgX191MTYgaW4gdGhhdCBoZWFkZXIgLSBib3JrZWQuDQo+ID4NCj4gPiBPaywgc28g
dGhlIGludGVudGlvbiB3YXMgdG8gYXZvaWQgcGFkZGluZyBjYXVzZWQgYnkgdGhlIGFsaWdubWVu
dA0KPiA+IG9mIHNvY2thZGRyX3N0b3JhZ2UgcmF0aGVyIHRoYW4gYXJvdW5kIHRoZSAnX191MTYg
c3BwX2ZsYWdzJy4NCj4gPg0KPiA+IEknZCBoYXZlIHRvIGxvb2sgdXAgd2hhdCAocGFja2VkLCBh
bGlnbmVkKDQpKSBhY3R1YWxseSBtZWFucy4NCj4gPiBJdCBjb3VsZCBmb3JjZSB0aGUgc3RydWN0
dXJlIHRvIGJlIGZ1bGx5IHBhY2tlZCAobm8gaG9sZXMpDQo+ID4gYnV0IGFsd2F5cyBoYXZlIGFu
IG92ZXJhbGwgYWxpZ25tZW50IG9mIDQuDQo+ID4NCj4gPiBJdCBtaWdodCBoYXZlIGJlZW4gY2xl
YXJlciB0byBwdXQgYW4gJ2FsaWduZWQoNCknIGF0dHJpYnV0ZQ0KPiA+IG9uIHRoZSBzcHBfYWRk
cmVzcyBmaWVsZCBpdHNlbGYuDQo+ID4gT3IgZXZlbiB3b25kZXIgd2hldGhlciBzb2NrYWRkcl9z
dG9yYWdlIHNob3VsZCBhY3R1YWxseQ0KPiA+IGhhdmUgOCBieXRlIGFsaWdubWVudC4NCj4gPg0K
PiA+IElmIGl0IGhhcyAxNiBieXRlIGFsaWdubWVudCB0aGVuIHlvdSBjYW5ub3QgY2FzdCBhbiBJ
UHY0DQo+ID4gc29ja2V0IGJ1ZmZlciBhZGRyZXNzICh3aGljaCB3aWxsIGJlIGF0IG1vc3QgNCBi
eXRlIGFsaWduZWQpDQo+ID4gdG8gc29ja2FkZHJfc3RvcmFnZSBhbmQgZXhwZWN0IHRoZSBjb21w
aWxlciBub3QgdG8gZ2VuZXJhdGUNCj4gPiBjb2RlIHRoYXQgd2lsbCBjcmFzaCBhbmQgYnVybiBv
biBzcGFyYzY0Lg0KDQpBY3R1YWxseSwgd2hhdCBoYXBwZW5zIHdoZW4gdGhlIG1pc2FsaWduZWQg
J3N0cnVjdCBzb2NrYWRkcicNCihpbiB0aGUgc2N0cCBvcHRpb25zKSBpcyBwYXNzZWQgdGhyb3Vn
aCB0byBhIGZ1bmN0aW9uDQp0aGF0IGV4cGVjdHMgaXQgdG8gYmUgYWxpZ25lZCBhbmQgdGhlbiBh
Y2Nlc3NlcyBwYXJ0IG9mIChzYXkpDQphbiBJUHY2IHN0cnVjdHVyZSB1c2luZyA4IGJ5dGVzIGFj
Y2Vzc2VzLg0KVGhhdCB3aWxsICdjcmFzaCBhbmQgYnVybicgb24gc3BhcmM2NCBhcyB3ZWxsLg0K
DQo+ID4gSVNUUiB0aGF0IHRoZSBOZXRCU0QgdmlldyB3YXMgdGhhdCAnc29ja2FkZHJfc3RvcmFn
ZScgc2hvdWxkDQo+ID4gbmV2ZXIgYWN0dWFsbHkgYmUgaW5zdGFudGlhdGVkIC0gaXQgb25seSBl
eGlzdGVkIGFzIGEgdHlwZWQNCj4gPiBwb2ludGVyLg0KPg0KPiBOb3Qgc3VyZSB0aGlzIGlzIGNv
cnJlY3QuIEkgd291bGQgc2F5IHRoaXMgYXBwbGllcyB0byBzdHVjdCBzb2NrYWRkciAqLg0KPiBJ
IGhhdmUgc2VlbiBpbnN0YW50aWF0ZWQgc29ja2FkZHJfc3RvcmFnZSB2YXJpYWJsZSBpbiBnZW5l
cmljIGNvZGUsDQo+IHdoZXJlIHlvdSBuZWVkIHRvIHByb3ZpZGUgZW5vdWdoIHNwYWNlIHRvIGhv
bGQgYW4gYWRkcmVzcywgbm90IHlldA0KPiBrbm93aW5nIHRoZSBhZGRyZXNzIGZhbWlseS4gSG93
ZXZlciwgSSdtIG5vdCBmYW1pbGlhciB3aXRoIHRoZSBOZXRCU0QNCj4gY29kZSBiYXNlLg0KDQpC
YXNpY2FsbHkgeW91IHNob3VsZCBhbHdheXMgaGF2ZSB0aGUgYWRkcmVzcyBsZW5ndGguDQpJIGp1
c3QgcmVtZW1iZXIgQ2hyaXN0b3MgY29tcGxhaW5pbmcgYWJvdXQgc29tZSBrZXJuZWwgY29kZQ0K
dGhhdCBhbGxvY2F0ZWQgb25lIG9uIHN0YWNrLg0KKE15IE5ldEJTRCAnY29tbWl0IGJpdCcgaGFz
IHJhdGhlciBsYXBzZWQuKQ0KDQoJRGF2aWQNCg0KLQ0KUmVnaXN0ZXJlZCBBZGRyZXNzIExha2Vz
aWRlLCBCcmFtbGV5IFJvYWQsIE1vdW50IEZhcm0sIE1pbHRvbiBLZXluZXMsIE1LMSAxUFQsIFVL
DQpSZWdpc3RyYXRpb24gTm86IDEzOTczODYgKFdhbGVzKQ0K
WARNING: multiple messages have this Message-ID (diff)
From: David Laight <David.Laight@ACULAB.COM>
To: 'Michael Tuexen' <Michael.Tuexen@lurchi.franken.de>
Cc: "Ivan Skytte Jørgensen" <isj-sctp@i1.dk>,
"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: RE: packed structures used in socket options
Date: Mon, 8 Jun 2020 21:13:15 +0000 [thread overview]
Message-ID: <529a772bd3ef40d3a310e78d613339ca@AcuMS.aculab.com> (raw)
In-Reply-To: <7BD347D7-562F-459D-B0CB-0BC798919876@lurchi.franken.de>
From: Michael Tuexen
> Sent: 08 June 2020 18:37
> > On 8. Jun 2020, at 18:18, David Laight <David.Laight@ACULAB.COM> wrote:
> >
> > From: Ivan Skytte Jørgensen
> >> Sent: 07 June 2020 22:35
> > ...
> >>>>>>>> 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 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 was also looking at all the __u16 in that header - borked.
> >
> > Ok, so the intention was to avoid padding caused by the alignment
> > of sockaddr_storage rather than around the '__u16 spp_flags'.
> >
> > I'd have to look up what (packed, aligned(4)) actually means.
> > It could force the structure to be fully packed (no holes)
> > but always have an overall alignment of 4.
> >
> > It might have been clearer to put an 'aligned(4)' attribute
> > on the spp_address field itself.
> > Or even wonder whether sockaddr_storage should actually
> > have 8 byte alignment.
> >
> > If it has 16 byte alignment then you cannot cast an IPv4
> > socket buffer address (which will be at most 4 byte aligned)
> > to sockaddr_storage and expect the compiler not to generate
> > code that will crash and burn on sparc64.
Actually, what happens when the misaligned 'struct sockaddr'
(in the sctp options) is passed through to a function
that expects it to be aligned and then accesses part of (say)
an IPv6 structure using 8 bytes accesses.
That will 'crash and burn' on sparc64 as well.
> > ISTR that the NetBSD view was that 'sockaddr_storage' should
> > never actually be instantiated - it only existed as a typed
> > pointer.
>
> Not sure this is correct. I would say this applies to stuct sockaddr *.
> I have seen instantiated sockaddr_storage variable in generic code,
> where you need to provide enough space to hold an address, not yet
> knowing the address family. However, I'm not familiar with the NetBSD
> code base.
Basically you should always have the address length.
I just remember Christos complaining about some kernel code
that allocated one on stack.
(My NetBSD 'commit bit' has rather lapsed.)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
next prev parent reply other threads:[~2020-06-08 21:13 UTC|newest]
Thread overview: 17+ 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
2020-06-08 16:18 ` David Laight
2020-06-08 16:18 ` David Laight
2020-06-08 17:37 ` Michael Tuexen
2020-06-08 17:37 ` Michael Tuexen
2020-06-08 21:13 ` David Laight [this message]
2020-06-08 21:13 ` David Laight
2020-06-07 21:51 ` Michael Tuexen
2020-06-08 8:17 ` David Laight
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=529a772bd3ef40d3a310e78d613339ca@AcuMS.aculab.com \
--to=david.laight@aculab.com \
--cc=Michael.Tuexen@lurchi.franken.de \
--cc=isj-sctp@i1.dk \
--cc=linux-sctp@vger.kernel.org \
--cc=netdev@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.