From: Arnaldo Carvalho de Melo <acme@conectiva•com.br>
To: Sridhar Samudrala <sri@us•ibm.com>
Cc: davem@redhat•com, netdev@oss•sgi.com
Subject: Re: [PATCH 2.5.69] Bug in sys_accept() module ref counts
Date: Tue, 6 May 2003 21:14:19 -0300 [thread overview]
Message-ID: <20030507001418.GA27162@conectiva.com.br> (raw)
In-Reply-To: <Pine.LNX.4.44.0305061223090.1348-100000@dyn9-47-18-140.beaverton.ibm.com>
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;
>
next prev parent reply other threads:[~2003-05-07 0:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2003-05-07 0:43 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20030507001418.GA27162@conectiva.com.br \
--to=acme@conectiva$(echo .)com.br \
--cc=davem@redhat$(echo .)com \
--cc=netdev@oss$(echo .)sgi.com \
--cc=sri@us$(echo .)ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox