diff mbox

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

Message ID 1314720708.2935.29.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet Aug. 30, 2011, 4:11 p.m. UTC 
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.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller Aug. 30, 2011, 6:07 p.m. UTC | #1 
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."
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Aug. 30, 2011, 7:16 p.m. UTC | #2 
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.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Aug. 30, 2011, 7:44 p.m. UTC | #3 
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


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Aug. 30, 2011, 7:52 p.m. UTC | #4 
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Aug. 30, 2011, 8 p.m. UTC | #5 
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.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;
 	}