Re: [PATCH] ipv4: return early for possible invalid uaddr

[Wen Yang]

在 2021/8/10 上午6:32, Jakub Kicinski 写道:
> On Sun,  8 Aug 2021 01:19:38 +0800 Wen Yang wrote:
> > The inet_dgram_connect() first calls inet_autobind() to select an
> > ephemeral port, then checks uaddr in udp_pre_connect() or
> > __ip4_datagram_connect(), but the port is not released until the socket
> > is closed.
> >
> > We should return early for invalid uaddr to improve performance and
> > simplify the code a bit,
>
> The performance improvement would be if the benchmark is calling
> connect with invalid arguments? That seems like an extremely rare
> scenario in real life.

Thanks for your comments.

On the one hand, it is the performance impact, but we also found that it
may cause DoS: simulate a scenario where udp connect is frequently
performed (illegal addrlen, and the socket is not closed), the local
ports will be exhausted quickly.

> > and also switch from a mix of tabs and spaces to just tabs.
>
> Please never mix unrelated whitespace cleanup into patches making real
> code changes.
OK.

> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index 5464818..97b6fc4 100644
> > --- a/net/ipv4/af_inet.c
> > +++ b/net/ipv4/af_inet.c
> > @@ -569,6 +569,11 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr,
> >   	if (uaddr->sa_family == AF_UNSPEC)
> >   		return sk->sk_prot->disconnect(sk, flags);
> >   
> > +	if (uaddr->sa_family != AF_INET)
> > +		return -EAFNOSUPPORT;
>
> And what about IPv6 which also calls this function?

Sorry that currently only ipv4 has been modified, we will continue to 
improve, and the v2 patch will be submitted later, thank you.


--
Best wishes，
Wen


> > +	if (addr_len < sizeof(struct sockaddr_in))
> > +		return -EINVAL;
> > +
> >   	if (BPF_CGROUP_PRE_CONNECT_ENABLED(sk)) {
> >   		err = sk->sk_prot->pre_connect(sk, uaddr, addr_len);
> >   		if (err)