[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20251022102624.18d2c6f7@pumpkin>
Date: Wed, 22 Oct 2025 10:26:24 +0100
From: David Laight <david.laight.linux@...il.com>
To: Kees Cook <kees@...nel.org>
Cc: Jakub Kicinski <kuba@...nel.org>, "Gustavo A. R. Silva"
<gustavo@...eddedor.com>, Alexei Starovoitov <ast@...nel.org>, Daniel
Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
<horms@...nel.org>, Kuniyuki Iwashima <kuniyu@...gle.com>, Willem de Bruijn
<willemb@...gle.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
bpf@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v3 1/9] net: Add struct sockaddr_unspec for sockaddr of
unknown length
On Tue, 21 Oct 2025 12:42:10 -0700
Kees Cook <kees@...nel.org> wrote:
> On Tue, Oct 21, 2025 at 10:26:00AM +0100, David Laight wrote:
> > On Mon, 20 Oct 2025 14:26:30 -0700
> > Kees Cook <kees@...nel.org> wrote:
> >
> > > Add flexible sockaddr structure to support addresses longer than the
> > > traditional 14-byte struct sockaddr::sa_data limitation without
> > > requiring the full 128-byte sa_data of struct sockaddr_storage. This
> > > allows the network APIs to pass around a pointer to an object that
> > > isn't lying to the compiler about how big it is, but must be accompanied
> > > by its actual size as an additional parameter.
> > >
> > > It's possible we may way to migrate to including the size with the
> > > struct in the future, e.g.:
> > >
> > > struct sockaddr_unspec {
> > > u16 sa_data_len;
> > > u16 sa_family;
> > > u8 sa_data[] __counted_by(sa_data_len);
> > > };
> >
> > One on the historic Unix implementations split the 'sa_family'
> > field into two single byte fields - the second one containing the length.
> > That might work - although care would be needed not to pass a length
> > back to userspace.
>
> I think this is just asking for trouble -- leaving that inline could
> be hard to track down places that needed filtering out.
>
> It might be easier to move to a separate struct like I suggest above,
> though maybe as:
>
> struct sockaddr_sized {
> u16 sa_data_len;
> struct {
> u16 sa_family;
> u8 sa_data[] __counted_by(sa_data_len);
> } sa_unspec;
> };
>
> (So it's easier to cast between implementation-specific sockaddr and the
> "sa_unspec" member.)
>
> And then pass that around. But I think that'll require a LOT of
> refactoring. But that could happen separately from this change, which is
> to just get us back to the existing state of passing around an unknown
> sized object but now we're not lying to the compiler about its size.
The problem with that is it misaligns things later on.
And you really want to be using the length from the user,
which includes sa_family.
I'm not sure it even works if the extra sa_data_len is u32 (or even u64).
I did wonder if 'union casts' would help.
They would require a union of pointers to all possible sockaddr formats.
Annoyingly I don't think you can avoid the need for the cast on the
call sites for a lot of the functions - unless they are renamed to have
unique names (so a #define can add the cast).
If you do add a #define, it can generate the size from that of the
supplied address structure (and error things that are stupidly short).
David
Powered by blists - more mailing lists