From: Eric Biggers <ebiggers@kernel.org>
To: netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>
Cc: linux-kernel@vger.kernel.org, Mao Wenan <maowenan@huawei.com>,
Cong Wang <xiyou.wangcong@gmail.com>,
Lorenzo Colitti <lorenzo@google.com>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Al Viro <viro@zeniv.linux.org.uk>,
Eric Biggers <ebiggers@kernel.org>
Subject: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
Date: Thu, 21 Feb 2019 14:13:56 -0800 [thread overview]
Message-ID: <20190221221356.173485-1-ebiggers@kernel.org> (raw)
From: Eric Biggers <ebiggers@google.com>
Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
closed concurrently with fchownat(). However, it ignored that many
other proto_ops::release() methods don't set sock->sk to NULL and
therefore allow the same use-after-free:
- base_sock_release
- bnep_sock_release
- cmtp_sock_release
- data_sock_release
- dn_release
- hci_sock_release
- hidp_sock_release
- iucv_sock_release
- l2cap_sock_release
- llcp_sock_release
- llc_ui_release
- rawsock_release
- rfcomm_sock_release
- sco_sock_release
- svc_release
- vcc_release
- x25_release
Rather than fixing all these and relying on every socket type to get
this right forever, just make __sock_release() set sock->sk to NULL
itself after calling proto_ops::release().
Reproducer that produces the KASAN splat when any of these socket types
are configured into the kernel:
#include <pthread.h>
#include <stdlib.h>
#include <sys/socket.h>
#include <unistd.h>
pthread_t t;
volatile int fd;
void *close_thread(void *arg)
{
for (;;) {
usleep(rand() % 100);
close(fd);
}
}
int main()
{
pthread_create(&t, NULL, close_thread, NULL);
for (;;) {
fd = socket(rand() % 50, rand() % 11, 0);
fchownat(fd, "", 1000, 1000, 0x1000);
close(fd);
}
}
Fixes: 86741ec25462 ("net: core: Add a UID field to struct sock.")
Cc: <stable@vger.kernel.org> # v4.10+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
NOTE: I am not an expert in the networking code, so please carefully
check that I haven't missed some reason why this simple fix won't do.
net/socket.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/socket.c b/net/socket.c
index d80d87a395ea..320f51b22b19 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -577,6 +577,7 @@ static void __sock_release(struct socket *sock, struct inode *inode)
if (inode)
inode_lock(inode);
sock->ops->release(sock);
+ sock->sk = NULL;
if (inode)
inode_unlock(inode);
sock->ops = NULL;
--
2.21.0.rc0.258.g878e2cd30e-goog
next reply other threads:[~2019-02-21 22:14 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-21 22:13 Eric Biggers [this message]
2019-02-22 2:51 ` [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release() Al Viro
2019-02-22 17:45 ` Eric Dumazet
2019-02-22 17:57 ` Eric Biggers
2019-02-22 18:25 ` Eric Dumazet
2019-02-22 19:08 ` Al Viro
2019-02-22 18:05 ` Cong Wang
2019-02-25 18:41 ` 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=20190221221356.173485-1-ebiggers@kernel.org \
--to=ebiggers@kernel.org \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=lorenzo@google.com \
--cc=maowenan@huawei.com \
--cc=netdev@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=viro@zeniv.linux.org.uk \
--cc=xiyou.wangcong@gmail.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.