From patchwork Thu Oct 20 16:39:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 684697 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3t0F03563sz9ssP for ; Fri, 21 Oct 2016 03:39:47 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=CbwLl6J5; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753684AbcJTQjo (ORCPT ); Thu, 20 Oct 2016 12:39:44 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:34146 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753142AbcJTQjm (ORCPT ); Thu, 20 Oct 2016 12:39:42 -0400 Received: by mail-pf0-f193.google.com with SMTP id 128so6042395pfz.1 for ; Thu, 20 Oct 2016 09:39:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=YPyQE3Ps+aX7/OrYIV8C4wGItHBkDAfqkrd+6G25cVE=; b=CbwLl6J5oIh3cNkGyyvFrXfbDsBvcvu8wUg8qCsTj43hs+QLxebZzGJBxAgNgp53wQ wQ2PqLs0AMKUj5kIURLfYAyL2CTooi+73If3RKeCHzXmdi1qFVh09vYpcrMjJY3dOlbD 7jZdvc31Dri4uWQ135ZLEa6v756kHHYd6r3/USOAkS4p1S6U+w5bmJ+IcrRjIU/o4ual 9YsssRReLuZDxLOzdpvndoJpe9usYQfKZcTQMcC9DRTMy5g+P/p3E4zLn2zt8rNw4lwg LPQnpOeOpT6UNsW5pyLHV+IIgnJzfj0tunXT0D8g8pulH3MmHeIE6fQp28KRI9z7NkvZ Sh0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=YPyQE3Ps+aX7/OrYIV8C4wGItHBkDAfqkrd+6G25cVE=; b=k9FDmK2uytS7UEYSeOdhDRa3w6QvGL7kS3P8g0QL4sy3bBvgYQfUHN6nwSL461uxjV LL/0kxftHwVm3VBf1tG+SLMKJ0buCiLzO32LMgUBMwIRspM6uLQJbF63+mUJYyfkwCK9 vbVQCLdJmRzYMTS7hRHGUdKtnHfTEo9+/cyWiGDLUCEdsiKPpqbaamCnDpZGwuoFp3w5 46/ALujRSWZr47OchvUO4yZuabFMWHDmzc6Xw5lXhN/XVjIyjWuAt4J9e95Iu+5+q+Jf bKI59seqpLAHSnieOzNIz0AqpNqJSVfW+XxPp7lOGOcRL8AySh6WokqXLmmX5uQZjfPR NXQA== X-Gm-Message-State: AA6/9Rn3K6OCaDIw8ViAfmDr1AOD2HS1K2nnT8Y4ClArp2k6iASrGUr1xMmxS6sNJtfR0Q== X-Received: by 10.98.84.131 with SMTP id i125mr2882799pfb.96.1476981581369; Thu, 20 Oct 2016 09:39:41 -0700 (PDT) Received: from ?IPv6:2620:0:1000:1704:8dd9:de29:1ca7:82f1? ([2620:0:1000:1704:8dd9:de29:1ca7:82f1]) by smtp.googlemail.com with ESMTPSA id z87sm22513962pfk.67.2016.10.20.09.39.40 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Oct 2016 09:39:40 -0700 (PDT) Message-ID: <1476981580.7065.15.camel@edumazet-glaptop3.roam.corp.google.com> Subject: [PATCH net] udp: must lock the socket in udp_disconnect() From: Eric Dumazet To: Baozeng Ding , David Miller Cc: network dev Date: Thu, 20 Oct 2016 09:39:40 -0700 In-Reply-To: <1476944754.7065.3.camel@edumazet-glaptop3.roam.corp.google.com> References: <31cb3583-5620-de3f-784d-300df3fda0a4@gmail.com> <05f766db-4a8c-933e-9d73-6daada21f491@gmail.com> <1476944754.7065.3.camel@edumazet-glaptop3.roam.corp.google.com> X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Eric Dumazet Baozeng Ding reported KASAN traces showing uses after free in udp_lib_get_port() and other related UDP functions. A CONFIG_DEBUG_PAGEALLOC=y kernel would eventually crash. I could write a reproducer with two threads doing : static int sock_fd; static void *thr1(void *arg) { for (;;) { connect(sock_fd, (const struct sockaddr *)arg, sizeof(struct sockaddr_in)); } } static void *thr2(void *arg) { struct sockaddr_in unspec; for (;;) { memset(&unspec, 0, sizeof(unspec)); connect(sock_fd, (const struct sockaddr *)&unspec, sizeof(unspec)); } } Problem is that udp_disconnect() could run without holding socket lock, and this was causing list corruptions. Signed-off-by: Eric Dumazet Reported-by: Baozeng Ding --- include/net/udp.h | 1 + net/ipv4/ping.c | 2 +- net/ipv4/raw.c | 2 +- net/ipv4/udp.c | 13 +++++++++++-- net/ipv6/ping.c | 2 +- net/ipv6/raw.c | 2 +- net/l2tp/l2tp_ip.c | 2 +- net/l2tp/l2tp_ip6.c | 2 +- 8 files changed, 18 insertions(+), 8 deletions(-) diff --git a/include/net/udp.h b/include/net/udp.h index ea53a87d880f..4948790d393d 100644 --- a/include/net/udp.h +++ b/include/net/udp.h @@ -258,6 +258,7 @@ void udp_flush_pending_frames(struct sock *sk); void udp4_hwcsum(struct sk_buff *skb, __be32 src, __be32 dst); int udp_rcv(struct sk_buff *skb); int udp_ioctl(struct sock *sk, int cmd, unsigned long arg); +int __udp_disconnect(struct sock *sk, int flags); int udp_disconnect(struct sock *sk, int flags); unsigned int udp_poll(struct file *file, struct socket *sock, poll_table *wait); struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb, diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c index 7cf7d6e380c2..205e2000d395 100644 --- a/net/ipv4/ping.c +++ b/net/ipv4/ping.c @@ -994,7 +994,7 @@ struct proto ping_prot = { .init = ping_init_sock, .close = ping_close, .connect = ip4_datagram_connect, - .disconnect = udp_disconnect, + .disconnect = __udp_disconnect, .setsockopt = ip_setsockopt, .getsockopt = ip_getsockopt, .sendmsg = ping_v4_sendmsg, diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c index 90a85c955872..ecbe5a7c2d6d 100644 --- a/net/ipv4/raw.c +++ b/net/ipv4/raw.c @@ -918,7 +918,7 @@ struct proto raw_prot = { .close = raw_close, .destroy = raw_destroy, .connect = ip4_datagram_connect, - .disconnect = udp_disconnect, + .disconnect = __udp_disconnect, .ioctl = raw_ioctl, .init = raw_init, .setsockopt = raw_setsockopt, diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index 7d96dc2d3d08..311613e413cb 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -1345,7 +1345,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock, goto try_again; } -int udp_disconnect(struct sock *sk, int flags) +int __udp_disconnect(struct sock *sk, int flags) { struct inet_sock *inet = inet_sk(sk); /* @@ -1367,6 +1367,15 @@ int udp_disconnect(struct sock *sk, int flags) sk_dst_reset(sk); return 0; } +EXPORT_SYMBOL(__udp_disconnect); + +int udp_disconnect(struct sock *sk, int flags) +{ + lock_sock(sk); + __udp_disconnect(sk, flags); + release_sock(sk); + return 0; +} EXPORT_SYMBOL(udp_disconnect); void udp_lib_unhash(struct sock *sk) @@ -2193,7 +2202,7 @@ int udp_abort(struct sock *sk, int err) sk->sk_err = err; sk->sk_error_report(sk); - udp_disconnect(sk, 0); + __udp_disconnect(sk, 0); release_sock(sk); diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c index 0e983b694ee8..66e2d9dfc43a 100644 --- a/net/ipv6/ping.c +++ b/net/ipv6/ping.c @@ -180,7 +180,7 @@ struct proto pingv6_prot = { .init = ping_init_sock, .close = ping_close, .connect = ip6_datagram_connect_v6_only, - .disconnect = udp_disconnect, + .disconnect = __udp_disconnect, .setsockopt = ipv6_setsockopt, .getsockopt = ipv6_getsockopt, .sendmsg = ping_v6_sendmsg, diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c index 54404f08efcc..054a1d84fc5e 100644 --- a/net/ipv6/raw.c +++ b/net/ipv6/raw.c @@ -1241,7 +1241,7 @@ struct proto rawv6_prot = { .close = rawv6_close, .destroy = raw6_destroy, .connect = ip6_datagram_connect_v6_only, - .disconnect = udp_disconnect, + .disconnect = __udp_disconnect, .ioctl = rawv6_ioctl, .init = rawv6_init_sk, .setsockopt = rawv6_setsockopt, diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index 42de4ccd159f..fce25afb652a 100644 --- a/net/l2tp/l2tp_ip.c +++ b/net/l2tp/l2tp_ip.c @@ -338,7 +338,7 @@ static int l2tp_ip_disconnect(struct sock *sk, int flags) if (sock_flag(sk, SOCK_ZAPPED)) return 0; - return udp_disconnect(sk, flags); + return __udp_disconnect(sk, flags); } static int l2tp_ip_getname(struct socket *sock, struct sockaddr *uaddr, diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c index ea2ae6664cc8..ad3468c32b53 100644 --- a/net/l2tp/l2tp_ip6.c +++ b/net/l2tp/l2tp_ip6.c @@ -410,7 +410,7 @@ static int l2tp_ip6_disconnect(struct sock *sk, int flags) if (sock_flag(sk, SOCK_ZAPPED)) return 0; - return udp_disconnect(sk, flags); + return __udp_disconnect(sk, flags); } static int l2tp_ip6_getname(struct socket *sock, struct sockaddr *uaddr,