net: fix kernel_accept() error path
Subsystems (3)
PATCH 1/1 net: fix kernel_accept() error path
If accept() returns an error, kernel_accept() releases the new socket but passes a pointer to the released socket back to the caller. Make it pass back NULL instead.
---
net/socket.c renamed from net/socket.c.bak +1 -0
@@ -2230,6 +2230,7 @@ int kernel_accept(struct socket *sock, s err = sock->ops->accept (sock, *newsock, flags);if (err <0 ) {sock_release (*newsock); + *newsock =NULL ; goto done; }
Quoted 9 lines · Tony Battersby
If
accept()returns an error,kernel_accept()releases the new socket but passes a pointer to the released socket back to the caller. Make it pass back NULL instead.Signed-off-by Tony Battersby <[email protected]>---
net/socket.c int kernel_accept(struct socket *sock, s · line 2236
Quoted 7 lines
net/socket.c renamed from net/socket.c.bak +1
-0@@ -2230,6 +2230,7 @@ int kernel_accept(struct socket *sock, s err = sock->ops->accept (sock, *newsock, flags);if (err <0 ) {sock_release (*newsock); + *newsock =NULL ; goto done; }
If you get an error back from kernel_accept, you should not be trying to use newsock.
Signature · 2 lines
James Morris <[email protected]>
David Miller
maintainerFrom: James Morris <[email protected]>
Date: Thu, 4 Oct 2007 14:57:33 -0700 (PDT)
Quoted 6 lines · Tony Battersby
Quoted 5 lines
If
accept()returns an error,kernel_accept()releases the new socket but passes a pointer to the released socket back to the caller. Make it pass back NULL instead.
Quoted 15 lines
net/socket.c int kernel_accept(struct socket *sock, s · line 2236
Quoted 12 lines
Signed-off-by Tony Battersby <[email protected]>---
net/socket.c renamed from net/socket.c.bak +1
-0@@ -2230,6 +2230,7 @@ int kernel_accept(struct socket *sock, s err = sock->ops->accept (sock, *newsock, flags);if (err <0 ) {sock_release (*newsock); + *newsock =NULL ; goto done; }If you get an error back from
kernel_accept, you should not be trying to use newsock.
Agreed, the caller should not try to deref the thing, it's value is undefined.
Quoted 7 lines · James Morris
Quoted 4 lines · Tony Battersby
If
accept()returns an error,kernel_accept()releases the new socket but passes a pointer to the released socket back to the caller. Make it pass back NULL instead.
Quoted 18 lines
net/socket.c int kernel_accept(struct socket *sock, s · line 2236
Quoted 13 lines
Signed-off-by Tony Battersby <[email protected]>---
net/socket.c renamed from net/socket.c.bak +1
-0@@ -2230,6 +2230,7 @@ int kernel_accept(struct socket *sock, s err = sock->ops->accept (sock, *newsock, flags);if (err <0 ) {sock_release (*newsock); + *newsock =NULL ; goto done; }If you get an error back from
kernel_accept, you should not be trying to use newsock.
Here is an example of what I would consider "reasonable code" that would fail:
int example()
{
struct socket *conn_socket = NULL;
int err;
...
if ((err = kernel_accept(sock, &conn_socket, 0)) < 0)
goto out_cleanup;
[do whatever with conn_socket] out_cleanup:
if (conn_socket != NULL)
sock_release(&conn_socket);
return err;
}Without the patch, the double sock_release() will cause a BUG().
Also compare to sock_create_lite(), which sets *res to NULL on error.
Tony
David Miller
maintainerFrom: Tony Battersby <[email protected]>
Date: Thu, 04 Oct 2007 18:12:53 -0400
Quoted 15 lines
Here is an example of what I would consider "reasonable code" that would fail:
int
example(){ struct socket *conn_socket = NULL; int err; ... if ((err = kernel_accept(sock, &conn_socket, 0)) < 0) goto out_cleanup; [do whatever with conn_socket]
Quoted 7 lines
out_cleanup:if (conn_socket != NULL) sock_release(&conn_socket); return err; }
This is a grey area.
I'd say you shouldn't be trying to do cleanups on conn_socket unless kernel_accept() gave you a success return.
However, kernel_accept() is guilty of leaving a stray pointer in conn_socket, in fact a reference to freed memory. So from that perspective we should put your patch in.
Please resubmit, at least to me under seperate cover,thanks.