On Thu, Oct 23, 2025 at 12:47:32PM +0200, Paolo Abeni wrote: > On 10/20/25 11:26 PM, Kees Cook wrote: > > While reviewing the struct proto_ops connect() and bind() callback > > implementations, I noticed that there doesn't appear to be any > > validation that AF_PPPOX sockaddr structures actually have sa_family set > > to AF_PPPOX. The pppol2tp_sockaddr_get_info() checks only look at the > > sizes. > > > > I don't see any way that this might actually cause problems as specific > > info fields are being populated, for which the existing size checks are > > correct, but it stood out as a missing address family check. > > > > Add the check and return -EAFNOSUPPORT on mismatch. > > > > Signed-off-by: Kees Cook <[email protected]> > > --- > > net/l2tp/l2tp_ppp.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c > > index 5e12e7ce17d8..b7a9c224520f 100644 > > --- a/net/l2tp/l2tp_ppp.c > > +++ b/net/l2tp/l2tp_ppp.c > > @@ -535,6 +535,13 @@ struct l2tp_connect_info { > > static int pppol2tp_sockaddr_get_info(const void *sa, int sa_len, > > struct l2tp_connect_info *info) > > { > > + const struct sockaddr_unspec *sockaddr = sa; > > + > > + if (sa_len < offsetofend(struct sockaddr, sa_family)) > > + return -EINVAL; > > + if (sockaddr->sa_family != AF_PPPOX) > > + return -EAFNOSUPPORT; > > I fear we can't introduce this check, as it could break existing > user-space application currently passing random data into sa_family but > still able to connect successfully.
Isn't sa_family kind of the critical determining factor on how the network stack decides to handle sockaddr stuff? I'll drop it for now, I guess, but that's surprising to me. -Kees -- Kees Cook
