netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Kees Cook <kees@kernel.org>
Cc: Jakub Kicinski <kuba@kernel.org>,
	"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	Kuniyuki Iwashima <kuniyu@google.com>,
	Willem de Bruijn <willemb@google.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH v3 1/9] net: Add struct sockaddr_unspec for sockaddr of unknown length
Date: Wed, 22 Oct 2025 10:26:24 +0100	[thread overview]
Message-ID: <20251022102624.18d2c6f7@pumpkin> (raw)
In-Reply-To: <202510211232.77A0F8A@keescook>

On Tue, 21 Oct 2025 12:42:10 -0700
Kees Cook <kees@kernel.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@kernel.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



  reply	other threads:[~2025-10-22  9:26 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-20 21:26 [PATCH v3 0/9] net: Introduce struct sockaddr_unspec Kees Cook
2025-10-20 21:26 ` [PATCH v3 1/9] net: Add struct sockaddr_unspec for sockaddr of unknown length Kees Cook
2025-10-21  9:26   ` David Laight
2025-10-21 19:42     ` Kees Cook
2025-10-22  9:26       ` David Laight [this message]
2025-10-23 16:33         ` Kees Cook
2025-10-23 10:43   ` Paolo Abeni
2025-10-23 11:40     ` David Laight
2025-10-23 16:31     ` Kees Cook
2025-10-23 10:59   ` Paolo Abeni
2025-10-23 16:20     ` Kees Cook
2025-10-20 21:26 ` [PATCH v3 2/9] net/l2tp: Add missing sa_family validation in pppol2tp_sockaddr_get_info Kees Cook
2025-10-23 10:47   ` Paolo Abeni
2025-10-23 16:01     ` Kees Cook
2025-10-20 21:26 ` [PATCH v3 3/9] net: Convert proto_ops bind() callbacks to use sockaddr_unspec Kees Cook
2025-10-20 21:26 ` [PATCH v3 4/9] net: Convert proto_ops connect() " Kees Cook
2025-10-20 21:26 ` [PATCH v3 5/9] net: Remove struct sockaddr from net.h Kees Cook
2025-10-20 21:26 ` [PATCH v3 6/9] net: Convert proto callbacks from sockaddr to sockaddr_unspec Kees Cook
2025-10-20 21:26 ` [PATCH v3 7/9] bpf: Convert cgroup sockaddr filters to use sockaddr_unspec consistently Kees Cook
2025-10-20 21:26 ` [PATCH v3 8/9] bpf: Convert bpf_sock_addr_kern "uaddr" to sockaddr_unspec Kees Cook
2025-10-20 21:26 ` [PATCH v3 9/9] net: Convert struct sockaddr to fixed-size "sa_data[14]" Kees Cook

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=20251022102624.18d2c6f7@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gustavo@embeddedor.com \
    --cc=horms@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=kees@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=willemb@google.com \
    /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).