* [PATCH 2.5.69] Bug in sys_accept() module ref counts
@ 2003-05-06 19:28 Sridhar Samudrala
2003-05-07 0:14 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 3+ messages in thread
From: Sridhar Samudrala @ 2003-05-06 19:28 UTC (permalink / raw)
To: davem, acme; +Cc: netdev
I think there is a bug in the recent changes to sys_accept() to implement
module ref counts.
module_put() gets called twice on error. Once via the explicit module_put and
the second via sock_release(). Also i think we should do a __module_get() with
newsock's owner(although same as the original listening sock).
The following patch against 2.5.69 should fix the problem.
Thanks
Sridhar
-------------------------------------------------------------------------------
diff -Nru a/net/socket.c b/net/socket.c
--- a/net/socket.c Tue May 6 12:14:35 2003
+++ b/net/socket.c Tue May 6 12:14:35 2003
@@ -1280,26 +1280,26 @@
* We don't need try_module_get here, as the listening socket (sock)
* has the protocol module (sock->ops->owner) held.
*/
- __module_get(sock->ops->owner);
+ __module_get(newsock->ops->owner);
err = sock->ops->accept(sock, newsock, sock->file->f_flags);
if (err < 0)
- goto out_module_put;
+ goto out_release;
if (upeer_sockaddr) {
if(newsock->ops->getname(newsock, (struct sockaddr *)address, &len, 2)<0) {
err = -ECONNABORTED;
- goto out_module_put;
+ goto out_release;
}
err = move_addr_to_user(address, len, upeer_sockaddr, upeer_addrlen);
if (err < 0)
- goto out_module_put;
+ goto out_release;
}
/* File flags are not inherited via accept() unlike another OSes. */
if ((err = sock_map_fd(newsock)) < 0)
- goto out_module_put;
+ goto out_release;
security_socket_post_accept(sock, newsock);
@@ -1307,8 +1307,6 @@
sockfd_put(sock);
out:
return err;
-out_module_put:
- module_put(sock->ops->owner);
out_release:
sock_release(newsock);
goto out_put;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 2.5.69] Bug in sys_accept() module ref counts 2003-05-06 19:28 [PATCH 2.5.69] Bug in sys_accept() module ref counts Sridhar Samudrala @ 2003-05-07 0:14 ` Arnaldo Carvalho de Melo 2003-05-07 0:43 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 3+ messages in thread From: Arnaldo Carvalho de Melo @ 2003-05-07 0:14 UTC (permalink / raw) To: Sridhar Samudrala; +Cc: davem, netdev Em Tue, May 06, 2003 at 12:28:41PM -0700, Sridhar Samudrala escreveu: > > I think there is a bug in the recent changes to sys_accept() to implement > module ref counts. Yes, well spotted, small comment below, I'll be sending this patch to DaveM, thanks a lot! > module_put() gets called twice on error. Once via the explicit module_put and > the second via sock_release(). Also i think we should do a __module_get() with > newsock's owner(although same as the original listening sock). > > The following patch against 2.5.69 should fix the problem. > > Thanks > Sridhar > ------------------------------------------------------------------------------- > diff -Nru a/net/socket.c b/net/socket.c > --- a/net/socket.c Tue May 6 12:14:35 2003 > +++ b/net/socket.c Tue May 6 12:14:35 2003 > @@ -1280,26 +1280,26 @@ > * We don't need try_module_get here, as the listening socket (sock) > * has the protocol module (sock->ops->owner) held. > */ > - __module_get(sock->ops->owner); > + __module_get(newsock->ops->owner); This one is OK, but the two operations are the same, so the effect, as well, is the same, but for correctness, better have it with newsock. > err = sock->ops->accept(sock, newsock, sock->file->f_flags); > if (err < 0) > - goto out_module_put; > + goto out_release; > > if (upeer_sockaddr) { > if(newsock->ops->getname(newsock, (struct sockaddr *)address, &len, 2)<0) { > err = -ECONNABORTED; > - goto out_module_put; > + goto out_release; > } > err = move_addr_to_user(address, len, upeer_sockaddr, upeer_addrlen); > if (err < 0) > - goto out_module_put; > + goto out_release; > } > > /* File flags are not inherited via accept() unlike another OSes. */ > > if ((err = sock_map_fd(newsock)) < 0) > - goto out_module_put; > + goto out_release; > > security_socket_post_accept(sock, newsock); > > @@ -1307,8 +1307,6 @@ > sockfd_put(sock); > out: > return err; > -out_module_put: > - module_put(sock->ops->owner); > out_release: > sock_release(newsock); > goto out_put; > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2.5.69] Bug in sys_accept() module ref counts 2003-05-07 0:14 ` Arnaldo Carvalho de Melo @ 2003-05-07 0:43 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 3+ messages in thread From: Arnaldo Carvalho de Melo @ 2003-05-07 0:43 UTC (permalink / raw) To: Sridhar Samudrala; +Cc: davem, netdev Em Tue, May 06, 2003 at 09:14:19PM -0300, Arnaldo C. Melo escreveu: > Em Tue, May 06, 2003 at 12:28:41PM -0700, Sridhar Samudrala escreveu: > > Also i think we should do a __module_get() with newsock's owner(although > > same as the original listening sock). Forget about the comment, its what you said above, sorry for answering too fast 8) - Arnaldo ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2003-05-07 0:43 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2003-05-06 19:28 [PATCH 2.5.69] Bug in sys_accept() module ref counts Sridhar Samudrala 2003-05-07 0:14 ` Arnaldo Carvalho de Melo 2003-05-07 0:43 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox