netdev - RE: Misaligned IPv6 addresses is SCTP socket options.

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, 21 Jul 2020 08:32:45 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Marcelo Ricardo Leitner' <marcelo.leitner@...il.com>
CC:     "linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Neil Horman <nhorman@...driver.com>
Subject: RE: Misaligned IPv6 addresses is SCTP socket options.

From: Marcelo Ricardo Leitner
> Sent: 21 July 2020 03:55
> 
> On Mon, Jul 20, 2020 at 03:50:16PM +0000, David Laight wrote:
> > Several of the structures in linux/uapi/linux/sctp.h are
> > marked __attribute__((packed, aligned(4))).
> 
> I don't think we can change that by now. It's bad, yes, but it's
> exposed and, well, for a long time (since 2005).
> 
> >
> > I believe this was done so that the UAPI structure was the
> > same on both 32 and 64bit systems.
> > The 'natural' alignment is that of 'u64' - so would differ
> > between 32 and 64 bit x86 cpus.
> >
> > There are two horrible issues here:
> >
> > 1) I believe the natural alignment of u64 is actually 8
> >    bytes on some 32bit architectures.
> 
> Not sure which?

Try arm for starters.

> >    So the change would have broken binary compatibility
> >    for 32bit applications compiled before the alignment
> >    was added.
> 
> If nobody complained in 15 years, that's probably not a problem. ;-)
> 
> >
> > 2) Inside the kernel the address of the structure member
> >    is 'blindly' passed through as if it were an aligned
> >    pointer.
> >    For instance I'm pretty sure is can get passed to
> >    inet_addr_is_any() (in net/core/utils.).
> >    Here it gets passed to memcmp().
> >    gcc will inline the memcmp() and almost certainly use 64bit
> >    accesses.
> >    These will fault on architectures (like sparc64).
> 
> For 2) here we should fix it by copying the data into a different
> buffer, or something like that.

At least on some architectures.
I did wonder if the buffer could be read to 8n+4 aligned memory,
but there are aligned 64bit items elsewhere.

> That is happening on structs sctp_setpeerprim sctp_prim
> sctp_paddrparams sctp_paddrinfo, right?
> As they all use the pattern of having a sockaddr_storage after a s32.

Not no mention sctp_assoc_stats....
Which is broken for 32bit binaries on x86 and sparc 64bit kernels.
I think there is (there should be) a kernel type on 64bit
systems that is 8 bytes with the alignment it would have
on the corresponding 32bit architecture.
If nothing else using alignof() on a structure containing
a member of that type will give the 4 or 8 required to fix
the code.

	David

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