From: Dave Jones <davej@redhat.com>
To: Jacek Luczak <difrost.kernel@gmail.com>
Cc: David Miller <davem@davemloft.net>,
vladislav.yasevich@hp.com, eric.dumazet@gmail.com,
netdev@vger.kernel.org
Subject: Re: [PATCH FINAL] SCTP: fix race between sctp_bind_addr_free() and sctp_bind_addr_conflict()
Date: Fri, 20 May 2011 19:27:17 -0400 [thread overview]
Message-ID: <20110520232717.GA5038@redhat.com> (raw)
In-Reply-To: <4DD575A1.4080405@gmail.com>
On Thu, May 19, 2011 at 09:55:13PM +0200, Jacek Luczak wrote:
> During the sctp_close() call, we do not use rcu primitives to
> destroy the address list attached to the endpoint. At the same
> time, we do the removal of addresses from this list before
> attempting to remove the socket from the port hash
>
> As a result, it is possible for another process to find the socket
> in the port hash that is in the process of being closed. It then
> proceeds to traverse the address list to find the conflict, only
> to have that address list suddenly disappear without rcu() critical
> section.
>
> Fix issue by closing address list removal inside RCU critical
> section.
>
> Race can result in a kernel crash with general protection fault or
> kernel NULL pointer dereference:
>
> kernel: general protection fault: 0000 [#1] SMP
> kernel: RIP: 0010:[<ffffffffa02f3dde>] [<ffffffffa02f3dde>] sctp_bind_addr_conflict+0x64/0x82 [sctp]
> kernel: Call Trace:
> kernel: [<ffffffffa02f415f>] ? sctp_get_port_local+0x17b/0x2a3 [sctp]
> kernel: [<ffffffffa02f3d45>] ? sctp_bind_addr_match+0x33/0x68 [sctp]
> kernel: [<ffffffffa02f4416>] ? sctp_do_bind+0xd3/0x141 [sctp]
> kernel: [<ffffffffa02f5030>] ? sctp_bindx_add+0x4d/0x8e [sctp]
> kernel: [<ffffffffa02f5183>] ? sctp_setsockopt_bindx+0x112/0x4a4 [sctp]
> kernel: [<ffffffff81089e82>] ? generic_file_aio_write+0x7f/0x9b
> kernel: [<ffffffffa02f763e>] ? sctp_setsockopt+0x14f/0xfee [sctp]
> kernel: [<ffffffff810c11fb>] ? do_sync_write+0xab/0xeb
> kernel: [<ffffffff810e82ab>] ? fsnotify+0x239/0x282
> kernel: [<ffffffff810c2462>] ? alloc_file+0x18/0xb1
> kernel: [<ffffffff8134a0b1>] ? compat_sys_setsockopt+0x1a5/0x1d9
> kernel: [<ffffffff8134aaf1>] ? compat_sys_socketcall+0x143/0x1a4
> kernel: [<ffffffff810467dc>] ? sysenter_dispatch+0x7/0x32
>
> Signed-off-by: Jacek Luczak <luczak.jacek@gmail.com>
> Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com>
> CC: Eric Dumazet <eric.dumazet@gmail.com>
>
> ---
> net/sctp/bind_addr.c | 10 ++++------
> 1 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
> index faf71d1..6150ac5 100644
> --- a/net/sctp/bind_addr.c
> +++ b/net/sctp/bind_addr.c
> @@ -140,14 +140,12 @@ void sctp_bind_addr_init(struct sctp_bind_addr *bp, __u16 port)
> /* Dispose of the address list. */
> static void sctp_bind_addr_clean(struct sctp_bind_addr *bp)
> {
> - struct sctp_sockaddr_entry *addr;
> - struct list_head *pos, *temp;
> + struct sctp_sockaddr_entry *addr, *temp;
>
> /* Empty the bind address list. */
> - list_for_each_safe(pos, temp, &bp->address_list) {
> - addr = list_entry(pos, struct sctp_sockaddr_entry, list);
> - list_del(pos);
> - kfree(addr);
> + list_for_each_entry_safe(addr, temp, &bp->address_list, list) {
> + list_del_rcu(&addr->list);
> + call_rcu(&addr->rcu, sctp_local_addr_free);
> SCTP_DBG_OBJCNT_DEC(addr);
Just saw this land in Linus tree, and it broke the build for me..
net/sctp/bind_addr.c: In function ‘sctp_bind_addr_clean’:
net/sctp/bind_addr.c:148:24: error: ‘sctp_local_addr_free’ undeclared (first use in this function)
net/sctp/bind_addr.c:148:24: note: each undeclared identifier is reported only once for each function it appears in
make[2]: *** [net/sctp/bind_addr.o] Error 1
Dave
next prev parent reply other threads:[~2011-05-20 23:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-19 19:55 [PATCH FINAL] SCTP: fix race between sctp_bind_addr_free() and sctp_bind_addr_conflict() Jacek Luczak
2011-05-19 19:58 ` Eric Dumazet
2011-05-19 20:19 ` David Miller
2011-05-20 23:27 ` Dave Jones [this message]
2011-05-21 6:05 ` David Miller
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=20110520232717.GA5038@redhat.com \
--to=davej@redhat.com \
--cc=davem@davemloft.net \
--cc=difrost.kernel@gmail.com \
--cc=eric.dumazet@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=vladislav.yasevich@hp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.