Fw: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

Fw: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[Stephen Hemminger]

Interesting? Does the kernel ABI include supporting buggy old proprietary
programs?

Begin forwarded message:

Date: Tue, 30 Aug 2011 02:59:32 GMT
From: bugzilla-daemon@bugzilla.kernel.org
To: shemminger@linux-foundation.org
Subject: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4


https://bugzilla.kernel.org/show_bug.cgi?id=42012

           Summary: regression on 2.6.39.3 with socket/bind; still there
                    in 3.0.4
           Product: Networking
           Version: 2.5
    Kernel Version: 2.6.39.3 - 3.0.4
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: IPV4
        AssignedTo: shemminger@linux-foundation.org
        ReportedBy: r_meier@freenet.de
        Regression: Yes


Hi,

the regression has been introduced in 2.6.39.3 with commit
d0733d2e29b652b2e7b1438ececa732e4eed98eb. I experience this with a proprietary
binary program. So I cant give you the source code which fails. This program
used to work before but fails after applying this patch. As far as I understand
the programm is using this code for ipc communication. I have recorded the
strace output of the relevant part of the program.
strace without d0733d2e29b652b2e7b1438ececa732e4eed98eb on kernel 2.6.39.2
---------------
2056  socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 4
2056  bind(4, {sa_family=AF_UNSPEC, sa_data="\0\0\0\0\0\0\0\0\0\0\0\0\0\0"},
16) = 0
2056  getsockname(4, {sa_family=AF_INET, sin_port=htons(33537),
sin_addr=inet_addr("0.0.0.0")}, [16]) = 0
2056  listen(4, 5)                      = 0
2056  setsockopt(4, SOL_TCP, TCP_NODELAY, [1], 4) = 0
---------------

strace with d0733d2e29b652b2e7b1438ececa732e4eed98eb on kernel 2.6.39.2
---------------
6190  socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 4
6190  bind(4, {sa_family=AF_UNSPEC, sa_data="\0\0\0\0\0\0\0\0\0\0\0\0\0\0"},
16) = -1 EINVAL (Invalid argument)
6190  dup(2)                            = 11
6190  fcntl64(11, F_GETFL)              = 0x8002 (flags O_RDWR|O_LARGEFILE)
6190  fstat64(11, {st_mode=S_IFCHR|0600, st_rdev=makedev(136, 3), ...}) = 0
6190  mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
= 0x576ac000
6190  _llseek(11, 0, 0xffa03de4, SEEK_CUR) = -1 ESPIPE (Illegal seek)
6190  write(11, "ERROR: Failed to bind to interne"..., 66) = 66
6190  close(11)                         = 0
---------------

Btw, i have not enough knowledge on this topic to decide whether its the
programs fault or the kernels fault. The binary program is sybyl8.1 from
tripos.

Best regards, Rene

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Re: Fw: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[Eric Dumazet]

Le mardi 30 août 2011 à 08:47 -0700, Stephen Hemminger a écrit :
> Interesting? Does the kernel ABI include supporting buggy old proprietary
> programs?
> 
> Begin forwarded message:
> 
> Date: Tue, 30 Aug 2011 02:59:32 GMT
> From: bugzilla-daemon@bugzilla.kernel.org
> To: shemminger@linux-foundation.org
> Subject: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4
> 
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=42012
> 
>            Summary: regression on 2.6.39.3 with socket/bind; still there
>                     in 3.0.4
>            Product: Networking
>            Version: 2.5
>     Kernel Version: 2.6.39.3 - 3.0.4
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: IPV4
>         AssignedTo: shemminger@linux-foundation.org
>         ReportedBy: r_meier@freenet.de
>         Regression: Yes
> 
> 
> Hi,
> 
> the regression has been introduced in 2.6.39.3 with commit
> d0733d2e29b652b2e7b1438ececa732e4eed98eb. I experience this with a proprietary
> binary program. So I cant give you the source code which fails. This program
> used to work before but fails after applying this patch. As far as I understand
> the programm is using this code for ipc communication. I have recorded the
> strace output of the relevant part of the program.
> strace without d0733d2e29b652b2e7b1438ececa732e4eed98eb on kernel 2.6.39.2
> ---------------
> 2056  socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 4
> 2056  bind(4, {sa_family=AF_UNSPEC, sa_data="\0\0\0\0\0\0\0\0\0\0\0\0\0\0"},
> 16) = 0
> 2056  getsockname(4, {sa_family=AF_INET, sin_port=htons(33537),
> sin_addr=inet_addr("0.0.0.0")}, [16]) = 0
> 2056  listen(4, 5)                      = 0
> 2056  setsockopt(4, SOL_TCP, TCP_NODELAY, [1], 4) = 0
> ---------------
> 
> strace with d0733d2e29b652b2e7b1438ececa732e4eed98eb on kernel 2.6.39.2
> ---------------
> 6190  socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 4
> 6190  bind(4, {sa_family=AF_UNSPEC, sa_data="\0\0\0\0\0\0\0\0\0\0\0\0\0\0"},
> 16) = -1 EINVAL (Invalid argument)
> 6190  dup(2)                            = 11
> 6190  fcntl64(11, F_GETFL)              = 0x8002 (flags O_RDWR|O_LARGEFILE)
> 6190  fstat64(11, {st_mode=S_IFCHR|0600, st_rdev=makedev(136, 3), ...}) = 0
> 6190  mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0)
> = 0x576ac000
> 6190  _llseek(11, 0, 0xffa03de4, SEEK_CUR) = -1 ESPIPE (Illegal seek)
> 6190  write(11, "ERROR: Failed to bind to interne"..., 66) = 66
> 6190  close(11)                         = 0
> ---------------
> 
> Btw, i have not enough knowledge on this topic to decide whether its the
> programs fault or the kernels fault. The binary program is sybyl8.1 from
> tripos.
> 
> Best regards, Rene
> 

Yep, we should relax the check and accept AF_UNSPEC.

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1b745d4..60fd64e 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -465,7 +465,7 @@ 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) {
+	if (addr->sin_family != AF_INET && addr->sin_family != AF_UNSPEC) {
 		err = -EAFNOSUPPORT;
 		goto out;
 	}

Re: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 30 Aug 2011 18:11:48 +0200

> Yep, we should relax the check and accept AF_UNSPEC.

I guess we'll have to do this, but I just can't bring myself to accept
that we can just do zero validation of what the user is passing us,
see an AF_UNSPEC, and say "yeah it's fine to assume there's an ipv4
address in there."

Re: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[Eric Dumazet]

Le mardi 30 août 2011 à 14:07 -0400, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 30 Aug 2011 18:11:48 +0200
> 
> > Yep, we should relax the check and accept AF_UNSPEC.
> 
> I guess we'll have to do this, but I just can't bring myself to accept
> that we can just do zero validation of what the user is passing us,
> see an AF_UNSPEC, and say "yeah it's fine to assume there's an ipv4
> address in there."

I couldnt accept it either ;)

By the way, if we accept it, strace() will probably still print binary
blob instead of the IP address (not necessarily ANY address ?)

connect ( AF_UNSPEC ) has special semantic, but AFAIK, bind (AF_UNSPEC)
only brings some mixed results : FreeBSD was accepting it in old
versions it seems. I guess I should try current FreeBSD versions.

Re: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[Eric Dumazet]

Le mardi 30 août 2011 à 21:16 +0200, Eric Dumazet a écrit :
> Le mardi 30 août 2011 à 14:07 -0400, David Miller a écrit :
> > From: Eric Dumazet <eric.dumazet@gmail.com>
> > Date: Tue, 30 Aug 2011 18:11:48 +0200
> > 
> > > Yep, we should relax the check and accept AF_UNSPEC.
> > 
> > I guess we'll have to do this, but I just can't bring myself to accept
> > that we can just do zero validation of what the user is passing us,
> > see an AF_UNSPEC, and say "yeah it's fine to assume there's an ipv4
> > address in there."
> 
> I couldnt accept it either ;)
> 
> By the way, if we accept it, strace() will probably still print binary
> blob instead of the IP address (not necessarily ANY address ?)
> 
> connect ( AF_UNSPEC ) has special semantic, but AFAIK, bind (AF_UNSPEC)
> only brings some mixed results : FreeBSD was accepting it in old
> versions it seems. I guess I should try current FreeBSD versions.
> 

Status on FreeBSD 8.1-RELEASE

bind(AF_UNSPEC) is accepted (converted to AF_INET), only with a 0.0.0.0
address.

Following code works :

struct sockaddr_in addr;
fd = socket(PF_INET, SOCK_STREAM, 0);
memset(&addr, 0, sizeof(addr))
bind(fd, (struct sockaddr *)&addr, sizeof(addr)); -> 0

If tried on ipv6 sockets, it doesnt work : bind() returns -1, errno=47

struct sockaddr_in6 addr;
fd = socket(PF_INET6, SOCK_STREAM, 0);
memset(&addr, 0, sizeof(addr))
bind(fd, (struct sockaddr *)&addr, sizeof(addr)); -> -1 errno=47

Re: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 30 Aug 2011 21:44:19 +0200

> Status on FreeBSD 8.1-RELEASE
> 
> bind(AF_UNSPEC) is accepted (converted to AF_INET), only with a 0.0.0.0
> address.
> 
> Following code works :
> 
> struct sockaddr_in addr;
> fd = socket(PF_INET, SOCK_STREAM, 0);
> memset(&addr, 0, sizeof(addr))
> bind(fd, (struct sockaddr *)&addr, sizeof(addr)); -> 0
> 
> If tried on ipv6 sockets, it doesnt work : bind() returns -1, errno=47

Ok if we could add that 0.0.0.0 check too that would make me feel
better about this change.

Re: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[Eric Dumazet]

Le mardi 30 août 2011 à 15:52 -0400, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 30 Aug 2011 21:44:19 +0200
> 
> > Status on FreeBSD 8.1-RELEASE
> > 
> > bind(AF_UNSPEC) is accepted (converted to AF_INET), only with a 0.0.0.0
> > address.
> > 
> > Following code works :
> > 
> > struct sockaddr_in addr;
> > fd = socket(PF_INET, SOCK_STREAM, 0);
> > memset(&addr, 0, sizeof(addr))
> > bind(fd, (struct sockaddr *)&addr, sizeof(addr)); -> 0
> > 
> > If tried on ipv6 sockets, it doesnt work : bind() returns -1, errno=47
> 
> Ok if we could add that 0.0.0.0 check too that would make me feel
> better about this change.

Sure, I will send a patch in a couple of minutes.

Re: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[Eric Dumazet]

Le mardi 30 août 2011 à 22:00 +0200, Eric Dumazet a écrit :

> Sure, I will send a patch in a couple of minutes.
> 
> 

[PATCH] net: ipv4: relax AF_INET check in bind()

commit d0733d2e29b65 (Check for mistakenly passed in non-IPv4 address)
added regression on legacy apps that use bind() with AF_UNSPEC family.

Relax the check, but make sure the bind() is done on INADDR_ANY
addresses, as AF_UNSPEC has probably no sane meaning for other
addresses.

Bugzilla reference : https://bugzilla.kernel.org/show_bug.cgi?id=42012

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Reported-and-bisected-by: Rene Meier <r_meier@freenet.de>
CC: Marcus Meissner <meissner@suse.de>
---
 net/ipv4/af_inet.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1b745d4..dd2b947 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -466,8 +466,13 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		goto out;
 
 	if (addr->sin_family != AF_INET) {
+		/* Compatibility games : accept AF_UNSPEC (mapped to AF_INET)
+		 * only if s_addr is INADDR_ANY.
+		 */
 		err = -EAFNOSUPPORT;
-		goto out;
+		if (addr->sin_family != AF_UNSPEC ||
+		    addr->sin_addr.s_addr != htonl(INADDR_ANY))
+			goto out;
 	}
 
 	chk_addr_ret = inet_addr_type(sock_net(sk), addr->sin_addr.s_addr);

Re: [Bug 42012] New: regression on 2.6.39.3 with socket/bind; still there in 3.0.4

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 30 Aug 2011 22:19:22 +0200

> [PATCH] net: ipv4: relax AF_INET check in bind()
> 
> commit d0733d2e29b65 (Check for mistakenly passed in non-IPv4 address)
> added regression on legacy apps that use bind() with AF_UNSPEC family.
> 
> Relax the check, but make sure the bind() is done on INADDR_ANY
> addresses, as AF_UNSPEC has probably no sane meaning for other
> addresses.
> 
> Bugzilla reference : https://bugzilla.kernel.org/show_bug.cgi?id=42012
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Reported-and-bisected-by: Rene Meier <r_meier@freenet.de>
> CC: Marcus Meissner <meissner@suse.de>

Applied, thanks everyone.