From: Michael Tuexen
> Sent: 08 June 2020 18:37
> > On 8. Jun 2020, at 18:18, David Laight <[email protected]> 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)
