net/ipv4: commit d0733d2e29b breaks rtorrent

net/ipv4: commit d0733d2e29b breaks rtorrent

[Fabienne Ducroquet]

Hi,

With the latest versions of the Linux kernel (I've just tried 
3.0.0-rc4-00034-g2992c4b) rtorrent (version 0.8.7 with libtorrent 
0.12.7, from Debian unstable) fails to start with the error message:

	rtorrent: Could not open/bind port for listening: Invalid argument

I bisected it down to:

	commit d0733d2e29b652b2e7b1438ececa732e4eed98eb
	Author: Marcus Meissner <meissner@suse.de>
	Date:   Wed Jun 1 21:05:22 2011 -0700

	    net/ipv4: Check for mistakenly passed in non-IPv4 address

	    Check against mistakenly passing in IPv6 addresses (which would result
	    in an INADDR_ANY bind) or similar incompatible sockaddrs.

	    Signed-off-by: Marcus Meissner <meissner@suse.de>
	    Cc: Reinhard Max <max@suse.de>
	    Signed-off-by: David S. Miller <davem@davemloft.net>
	---
	 net/ipv4/af_inet.c |    3 +++
	 1 files changed, 3 insertions(+), 0 deletions(-)

	diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
	index cc14631..9c19260 100644
	--- a/net/ipv4/af_inet.c
	+++ b/net/ipv4/af_inet.c
	@@ -465,6 +465,9 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
		if (addr_len < sizeof(struct sockaddr_in))
			goto out;

	+       if (addr->sin_family != AF_INET)
	+               goto out;
	+
		chk_addr_ret = inet_addr_type(sock_net(sk), addr->sin_addr.s_addr);

		/* Not specified by any standard per-se, however it breaks too

With this commit reverted rtorrent works again.

Fabienne

Re: net/ipv4: commit d0733d2e29b breaks rtorrent

[Marcus Meissner]

On Wed, Jun 22, 2011 at 02:40:45PM +0200, Fabienne Ducroquet wrote:
> Hi,
> 
> With the latest versions of the Linux kernel (I've just tried 
> 3.0.0-rc4-00034-g2992c4b) rtorrent (version 0.8.7 with libtorrent 
> 0.12.7, from Debian unstable) fails to start with the error message:
> 
> 	rtorrent: Could not open/bind port for listening: Invalid argument
> 
> I bisected it down to:
> 
> 	commit d0733d2e29b652b2e7b1438ececa732e4eed98eb
> 	Author: Marcus Meissner <meissner@suse.de>
> 	Date:   Wed Jun 1 21:05:22 2011 -0700
> 
> 	    net/ipv4: Check for mistakenly passed in non-IPv4 address
> 
> 	    Check against mistakenly passing in IPv6 addresses (which would result
> 	    in an INADDR_ANY bind) or similar incompatible sockaddrs.
> 
> 	    Signed-off-by: Marcus Meissner <meissner@suse.de>
> 	    Cc: Reinhard Max <max@suse.de>
> 	    Signed-off-by: David S. Miller <davem@davemloft.net>
> 	---
> 	 net/ipv4/af_inet.c |    3 +++
> 	 1 files changed, 3 insertions(+), 0 deletions(-)
> 
> 	diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> 	index cc14631..9c19260 100644
> 	--- a/net/ipv4/af_inet.c
> 	+++ b/net/ipv4/af_inet.c
> 	@@ -465,6 +465,9 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> 		if (addr_len < sizeof(struct sockaddr_in))
> 			goto out;
> 
> 	+       if (addr->sin_family != AF_INET)
> 	+               goto out;
> 	+
> 		chk_addr_ret = inet_addr_type(sock_net(sk), addr->sin_addr.s_addr);
> 
> 		/* Not specified by any standard per-se, however it breaks too
> 
> With this commit reverted rtorrent works again.

Can you show us the code surrounding the bind in rtorrent?

You either do not set the sin_family member, or pass in a ipv6 address.

Ciao, Marcus

Re: net/ipv4: commit d0733d2e29b breaks rtorrent

[Reinhard Max]

Hi,

On Wed, 22 Jun 2011 at 14:47, Marcus Meissner wrote:

> Can you show us the code surrounding the bind in rtorrent?

... or short of that, the output of "strace -e socket,bind" when 
starting rtorrent with the kernel patch not reverted?


cu
 	Reinhard

Re: net/ipv4: commit d0733d2e29b breaks rtorrent

[Fabienne Ducroquet]

On Wed, Jun 22, 2011 at 03:24:53PM +0200, Reinhard Max wrote:
> On Wed, 22 Jun 2011 at 14:47, Marcus Meissner wrote:
> 
> >Can you show us the code surrounding the bind in rtorrent?

Since I'm not a developer of rtorrent I'm not sure what the right code 
to show is, so I leave that to the developer of libtorrent/rtorrent, he 
is CC'ed.

> ... or short of that, the output of "strace -e socket,bind" when
> starting rtorrent with the kernel patch not reverted?

Here it is :

socket(PF_INET6, SOCK_STREAM, IPPROTO_TCP) = -1 EAFNOSUPPORT (Address family not supported by protocol)
socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 5
bind(5, {sa_family=AF_INET6, sin6_port=htons(6903), inet_pton(AF_INET6, "::", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = -1 EINVAL (Invalid argument)

[ 95 lines with only the number in parentheses after htons changing ]

bind(5, {sa_family=AF_INET6, sin6_port=htons(6999), inet_pton(AF_INET6, "::", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = -1 EINVAL (Invalid argument)
socket(PF_INET6, SOCK_STREAM, IPPROTO_TCP) = -1 EAFNOSUPPORT (Address family not supported by protocol)
socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 5
bind(5, {sa_family=AF_INET6, sin6_port=htons(6881), inet_pton(AF_INET6, "::", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = -1 EINVAL (Invalid argument)

[ 21 lines with only the number in parentheses after htons changing ]

bind(5, {sa_family=AF_INET6, sin6_port=htons(6903), inet_pton(AF_INET6, "::", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = -1 EINVAL (Invalid argument)


If I understand correctly, it is trying to bind an IPv6 address to a socket 
expecting an IPv4 address, so it's a bug in libtorrent/rtorrent, but it was not 
visible before because an INADDR_ANY bind is not a problem for rtorrent?

Fabienne

Re: net/ipv4: commit d0733d2e29b breaks rtorrent

[Reinhard Max]

On Wed, 22 Jun 2011 at 16:51, Fabienne Ducroquet wrote:

> If I understand correctly, it is trying to bind an IPv6 address to a 
> socket expecting an IPv4 address, so it's a bug in 
> libtorrent/rtorrent, but it was not visible before because an 
> INADDR_ANY bind is not a problem for rtorrent?

Yes, exactly.


cu
 	Reinhard

Re: net/ipv4: commit d0733d2e29b breaks rtorrent

[Stephen Hemminger]

On Wed, 22 Jun 2011 16:58:24 +0200 (CEST)
Reinhard Max <max@suse.de> wrote:

> 
> On Wed, 22 Jun 2011 at 16:51, Fabienne Ducroquet wrote:
> 
> > If I understand correctly, it is trying to bind an IPv6 address to a 
> > socket expecting an IPv4 address, so it's a bug in 
> > libtorrent/rtorrent, but it was not visible before because an 
> > INADDR_ANY bind is not a problem for rtorrent?
> 
> Yes, exactly.
> 
> 
> cu
>  	Reinhard

You can find code here http://libtorrent.rakshasa.no/
but it is in C++ so there is an abnormally high indirection factor
between reality of the bug and the code as written.

Did you try contacting the developer?

Re: net/ipv4: commit d0733d2e29b breaks rtorrent

[Fabienne Ducroquet]

On Wed, Jun 22, 2011 at 08:53:29AM -0700, Stephen Hemminger wrote:
> You can find code here http://libtorrent.rakshasa.no/
> but it is in C++ so there is an abnormally high indirection factor
> between reality of the bug and the code as written.

I had a look at the code, I found where bind is called, but it does not 
help: it is in libtorrent/src/net/socket_fd.cc:

	bool
	SocketFd::bind(const rak::socket_address& sa) {
	  check_valid();
	
	  return !::bind(m_fd, sa.c_sockaddr(), sa.length());
	}

check_valid() checks the validity of the socket FD.
It is called by the function:

	bool
	Listen::open(uint16_t first, uint16_t last, const rak::socket_address* bindAddress) {
	  close();

	  [ Checks that the range of the ports is OK, that the address 
	  is an inet or inet6 address, that the socket can be 
	  allocated.]

	  rak::socket_address sa;
	  sa.copy(*bindAddress, bindAddress->length());

	  for (uint16_t i = first; i <= last; ++i) {
	  sa.set_port(i);

	  if (get_fd().bind(sa) && get_fd().listen(50)) {
	     ...

in libtorrent/src/net/listen.cc. That function was called by:

	bool
	ConnectionManager::listen_open(port_type begin, port_type end) {
	  if (!m_listen->open(begin, end, rak::socket_address::cast_from(m_bindAddress)))
	    return false;
	
	  m_listenPort = m_listen->port();
	
	  return true;
	}

in libtorrent/src/torrent/connection_manager.cc, and it was itself 
called by Manager::listen_open() in rtorrent/core/manager.cc, which 
throws the error message I got because the call to

	torrent::connection_manager()->listen_open(portFirst, portLast)

fails.

So the question now is to find how the m_bindAddress used in 
ConnectionManager::listen_open is set, but I prefer not to do that right 
now.

> Did you try contacting the developer?

I've put him in the Cc: since the first message.

Fabienne