net: Check the argument for listen(2)

PATCH In review Changli Gao 3 participants 4 messages Jun 28, 2013

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Changli Gao

author

2013-06-28 16:24 UTC #0 lore raw

PATCH v2 1/1 net: Check the argument for listen(2)

As we use u16 to save the value of the argument for listen(2), we'd better check if the value is larger than SINT_MAX other than cut it down silently on error.

---
 net/ipv4/af_inet.c |    3 +++
 1 file changed, 3 insertions(+)

net/ipv4/af_inet.c +3 -0

index b4d0be2..35aaf00 100644
@@ -198,6 +198,9 @@ int inet_listen(struct socket *sock, int backlog)
 	unsigned char old_state;
 	int err;
 
+	if (backlog >= (1 << 16))
+		return -EINVAL;
+
 	lock_sock(sk);
 
 	err = -EINVAL;

Signature · 1 line

1.7.9.5

Eric Dumazet

2013-06-28 16:33 UTC #1 lore raw

Quoted 12 lines · Changli Gao

As we use u16 to save the value of the argument for listen(2), we'd better check if the value is larger than SINT_MAX other than cut it down silently on error.
---
 net/ipv4/af_inet.c |    3 +++
 1 file changed, 3 insertions(+)
net/ipv4/af_inet.c +0 -0
index b4d0be2..35aaf00 100644

net/ipv4/af_inet.c int inet_listen(struct socket *sock, int backlog) · line 206

Quoted 9 lines

net/ipv4/af_inet.c +3 -0

@@ -198,6 +198,9 @@ int inet_listen(struct socket *sock, int backlog)
 	unsigned char old_state;
 	int err;
 
+	if (backlog >= (1 << 16))
+		return -EINVAL;
+
 	lock_sock(sk);
 
 	err = -EINVAL;

Well, there is still this possible regression for old applications.

Just use u32 fields instead of u16 ?

Changli Gao

author

2013-06-28 16:47 UTC #2 lore raw

Quoted 5 lines · Eric Dumazet

Well, there is still this possible regression for old applications.
Just use u32 fields instead of u16 ?

I'll look at this. Thanks.

--
Regards,
Changli Gao([email protected])

Rick Jones

2013-06-28 18:01 UTC #3 lore raw

In reply to Eric Dumazet

Quoted 4 lines · Eric Dumazet

Well, there is still this possible regression for old applications.
Just use u32 fields instead of u16 ?

FWIW, the manpage for listen() gives the backlog parameter as an "int"

SYNOPSIS

       #include <sys/types.h>          /* See NOTES */
       #include <sys/socket.h>

       int listen(int sockfd, int backlog);

and mentions no explicit limit beyond 2.4.35, only interaction with the likes of /proc/sys/net/core/somaxconn.

And sys/socket.h has:

/* Prepare to accept connections on socket FD.
N connection requests will be queued before further requests are
refused.
Returns 0 on success, -1 for errors. */
extern int listen (int __fd, int __n) __THROW;

Not sure if it is considered "sane" to try to set the backlog to a negative value of course...

rick jones