UDP does not autobind on recv

Message ID 5d0f78b4-f850-4f73-dbb7-6c3d15d99c86@suse.cz
State New
Headers show

Commit Message

Jiri Slaby Oct. 24, 2016, 12:54 p.m.
Hello,

as per man 7 udp:
  In order to receive packets, the socket can be bound to
  a local  address first  by using bind(2).  Otherwise,
  the socket layer will automatically assign a free local
  port out of the range defined by /proc/sys/net/ipv4
  /ip_local_port_range and bind the socket to INADDR_ANY.

I did not know that bind is unneeded, so I tried that. But it does not
work with this piece of code:
int main()
{
    char buf[128];
    int fd = socket(AF_INET, SOCK_DGRAM, 0);
    recv(fd, buf, sizeof(buf), 0);
}

The recv above never returns (even if I bomb all ports from the range).
ss -ulpan is silent too. As a workaround, I can stick a dummy write/send
before recv:
    write(fd, "", 0);

And it starts working. ss suddenly displays a port which the program
listens on.

I think the UDP recv path should do inet_autobind as I have done in the
attached patch. But my knowledge is very limited in that area, so I have
no idea whether that is correct at all.

thanks,
-- 
js
suse labs

Comments

Eric Dumazet Oct. 24, 2016, 1:03 p.m. | #1
On Mon, 2016-10-24 at 14:54 +0200, Jiri Slaby wrote:
> Hello,

> 

> as per man 7 udp:

>   In order to receive packets, the socket can be bound to

>   a local  address first  by using bind(2).  Otherwise,

>   the socket layer will automatically assign a free local

>   port out of the range defined by /proc/sys/net/ipv4

>   /ip_local_port_range and bind the socket to INADDR_ANY.

> 

> I did not know that bind is unneeded, so I tried that. But it does not

> work with this piece of code:

> int main()

> {

>     char buf[128];

>     int fd = socket(AF_INET, SOCK_DGRAM, 0);

>     recv(fd, buf, sizeof(buf), 0);

> }


autobind makes little sense at recv() time really.

How an application could expect to receive a frame to 'some socket'
without even knowing its port ?

How useful would that be exactly ?

How TCP behaves ?

I would say, fix the documentation if it is not correct.
Jiri Slaby Oct. 24, 2016, 2:23 p.m. | #2
On 10/24/2016, 03:03 PM, Eric Dumazet wrote:
> On Mon, 2016-10-24 at 14:54 +0200, Jiri Slaby wrote:

>> Hello,

>>

>> as per man 7 udp:

>>   In order to receive packets, the socket can be bound to

>>   a local  address first  by using bind(2).  Otherwise,

>>   the socket layer will automatically assign a free local

>>   port out of the range defined by /proc/sys/net/ipv4

>>   /ip_local_port_range and bind the socket to INADDR_ANY.

>>

>> I did not know that bind is unneeded, so I tried that. But it does not

>> work with this piece of code:

>> int main()

>> {

>>     char buf[128];

>>     int fd = socket(AF_INET, SOCK_DGRAM, 0);

>>     recv(fd, buf, sizeof(buf), 0);

>> }

> 

> autobind makes little sense at recv() time really.

> 

> How an application could expect to receive a frame to 'some socket'

> without even knowing its port ?


For example
        struct sockaddr_storage sa;
        socklen_t slen = sizeof(sa);
        recv(fd, buf, sizeof(buf), MSG_DONTWAIT);
        getsockname(fd, (struct sockaddr *)&sa, &slen);
        recv(fd, buf, sizeof(buf), 0);
works.

> How useful would that be exactly ?


No need for finding a free port and checking, for example.

> How TCP behaves ?


TCP is a completely different story. bind is documented to be required
there. (And listen and accept.)

> I would say, fix the documentation if it is not correct.


I don't have a problem with either. I have only found, that the
implementation differs from the documentation :). Is there some
supervisor documentation (like POSIX) which should we be in conformance to?

thanks,
-- 
js
suse labs

Patch

From 57c320998feb2e1e705a4ab6d3bbcb74c6ae65f0 Mon Sep 17 00:00:00 2001
From: Jiri Slaby <jslaby@suse.cz>
Date: Sat, 22 Oct 2016 12:10:53 +0200
Subject: [PATCH] net: autobind UDP on recv

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 include/net/inet_common.h | 1 +
 net/ipv4/af_inet.c        | 3 ++-
 net/ipv4/udp.c            | 5 +++++
 net/ipv6/udp.c            | 5 +++++
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/net/inet_common.h b/include/net/inet_common.h
index 5d683428fced..ba224ed3dd36 100644
--- a/include/net/inet_common.h
+++ b/include/net/inet_common.h
@@ -27,6 +27,7 @@  ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
 int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
 		 int flags);
 int inet_shutdown(struct socket *sock, int how);
+int inet_autobind(struct sock *sk);
 int inet_listen(struct socket *sock, int backlog);
 void inet_sock_destruct(struct sock *sk);
 int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 9648c97e541f..d23acb11cdb0 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -171,7 +171,7 @@  EXPORT_SYMBOL(inet_sock_destruct);
  *	Automatically bind an unbound socket.
  */
 
-static int inet_autobind(struct sock *sk)
+int inet_autobind(struct sock *sk)
 {
 	struct inet_sock *inet;
 	/* We may need to bind the socket. */
@@ -187,6 +187,7 @@  static int inet_autobind(struct sock *sk)
 	release_sock(sk);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(inet_autobind);
 
 /*
  *	Move a socket into listening state.
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 82fb78265f4b..ceb07c83af17 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1360,6 +1360,11 @@  int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
 	if (flags & MSG_ERRQUEUE)
 		return ip_recv_error(sk, msg, len, addr_len);
 
+	/* We may need to bind the socket. */
+	if (!inet_sk(sk)->inet_num && !sk->sk_prot->no_autobind &&
+	    inet_autobind(sk))
+		return -EAGAIN;
+
 try_again:
 	peeking = off = sk_peek_offset(sk, flags);
 	skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 71963b23d5a5..1c3dafc3d91e 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -341,6 +341,11 @@  int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 	if (np->rxpmtu && np->rxopt.bits.rxpmtu)
 		return ipv6_recv_rxpmtu(sk, msg, len, addr_len);
 
+	/* We may need to bind the socket. */
+	if (!inet_sk(sk)->inet_num && !sk->sk_prot->no_autobind &&
+	    inet_autobind(sk))
+		return -EAGAIN;
+
 try_again:
 	peeking = off = sk_peek_offset(sk, flags);
 	skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
-- 
2.10.1