From patchwork Fri Mar 3 16:00:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 735170 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 3vZYzH2Y6nz9s1h for ; Sat, 4 Mar 2017 03:09:31 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="H6kvjRTk"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752037AbdCCQIl (ORCPT ); Fri, 3 Mar 2017 11:08:41 -0500 Received: from mail-pf0-f194.google.com ([209.85.192.194]:33425 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752010AbdCCQIh (ORCPT ); Fri, 3 Mar 2017 11:08:37 -0500 Received: by mail-pf0-f194.google.com with SMTP id e66so4613744pfe.0; Fri, 03 Mar 2017 08:08:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=VTh57OEGbFKzFs7IRdlCo4OzEFtniJBh2xY4OYYXYJo=; b=H6kvjRTkzvDXyhoUW4dGtDXO5xh87Lqj81TT76an/wC4blGyWdD6G8oYwunFhGvI9n Yjjl4AcVushnsCBXB6J0JTtBADy/vV/SImJMVYA08//TeV7LrKoZy2jkcE1mqGDxqj+O /oJDkpF3Ft1TrNTSoQbxAdGOSgXgpCSaxLgxlsEhgYP1F3g3a5qEdtY8k7t8QgLQtSoz cyitr+fRqfnExxSAgcdVoRMi3dDH9w4nVyDft2W+F43Ty8QwehzqaOXihh+xYFIqRucb 8mqDVGPzSRiKe5SxYdWxePx0X8+P7BiC8h7P/09ng1YaswtEECo1uF2TD2g2Dgs1Zy2C e4ag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=VTh57OEGbFKzFs7IRdlCo4OzEFtniJBh2xY4OYYXYJo=; b=QI/ccJOmWw6vPgqiu7OFcnyaLgLH6ZBnU66qwQm6rJlcyMfQyJtdQ9rNGnIpVdwZfr UAYXtWhogntBa1YpdUtb+HeKf7P9hoWpJZCwGn6dOJUgDTz/oWyr+4CEYlacstuK4FlM 1ZdOwUkOfiTVAInWDTzI25k+BZdJXOx/1HQSGlbZlYWj3ExiQQgR2uDQxSWCv17rR7P4 ev9grfVPIL0xs925udsHa4vRKpnsnmZgaEQNsB7c4mafiS60TFk0sSvX8+Q0tL/s/kSn Y8H/jMb8JwUaFUPD2JdJ0yz1uvHQyCq1CO1VpvqVnc6i+EbQGwKJSDftBXq5vpjvmeWl mPWg== X-Gm-Message-State: AMke39lMl6AEnA3jUR6WwBQ4qKbhm9wI4CvK0DNHigTcHPPMAdzFzOa3wKCxXU5i+++acw== X-Received: by 10.98.133.6 with SMTP id u6mr4399270pfd.48.1488556815622; Fri, 03 Mar 2017 08:00:15 -0800 (PST) Received: from ?IPv6:2620:0:1000:1704:3595:8fe7:98b2:1bf6? ([2620:0:1000:1704:3595:8fe7:98b2:1bf6]) by smtp.googlemail.com with ESMTPSA id x10sm9818812pff.72.2017.03.03.08.00.14 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Mar 2017 08:00:15 -0800 (PST) Message-ID: <1488556814.9415.334.camel@edumazet-glaptop3.roam.corp.google.com> Subject: Re: net/dccp: use-after-free in dccp_feat_activate_values From: Eric Dumazet To: Eric Dumazet Cc: Dmitry Vyukov , Cong Wang , Andrey Konovalov , Gerrit Renker , "David S. Miller" , dccp@vger.kernel.org, netdev , LKML Date: Fri, 03 Mar 2017 08:00:14 -0800 In-Reply-To: References: <1488551576.9415.328.camel@edumazet-glaptop3.roam.corp.google.com> <1488552503.9415.330.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 On Fri, 2017-03-03 at 07:22 -0800, Eric Dumazet wrote: > On Fri, Mar 3, 2017 at 7:12 AM, Dmitry Vyukov wrote: > > The first bot that picked this up started spewing: > > > > BUG: spinlock recursion on CPU#1, syz-executor2/9452 > > Yes. The bug is not about locking the listener, but protecting fields > of struct dccp_request_sock > > I will provide a patch, once I reach the office and after the breakfast ;) OK here is what I suggest to fix the races. Tested-by: Dmitry Vyukov diff --git a/include/linux/dccp.h b/include/linux/dccp.h index 61d042bbbf607253033d9948b291cab2322814ba..68449293c4b6233c1a1d4133b1819376a9310225 100644 --- a/include/linux/dccp.h +++ b/include/linux/dccp.h @@ -163,6 +163,7 @@ struct dccp_request_sock { __u64 dreq_isr; __u64 dreq_gsr; __be32 dreq_service; + spinlock_t dreq_lock; struct list_head dreq_featneg; __u32 dreq_timestamp_echo; __u32 dreq_timestamp_time; diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c index e267e6f4c9a5566b369a03a600a408e5bd41cbad..abd07a443219853b022bef41cb072e90ff8f07f0 100644 --- a/net/dccp/minisocks.c +++ b/net/dccp/minisocks.c @@ -142,6 +142,13 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb, struct dccp_request_sock *dreq = dccp_rsk(req); bool own_req; + /* TCP/DCCP listeners became lockless. + * DCCP stores complex state in its request_sock, so we need + * a protection for them, now this code runs without being protected + * by the parent (listener) lock. + */ + spin_lock_bh(&dreq->dreq_lock); + /* Check for retransmitted REQUEST */ if (dccp_hdr(skb)->dccph_type == DCCP_PKT_REQUEST) { @@ -156,7 +163,7 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb, inet_rtx_syn_ack(sk, req); } /* Network Duplicate, discard packet */ - return NULL; + goto out; } DCCP_SKB_CB(skb)->dccpd_reset_code = DCCP_RESET_CODE_PACKET_ERROR; @@ -182,20 +189,20 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb, child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL, req, &own_req); - if (!child) - goto listen_overflow; - - return inet_csk_complete_hashdance(sk, child, req, own_req); + if (child) { + child = inet_csk_complete_hashdance(sk, child, req, own_req); + goto out; + } -listen_overflow: - dccp_pr_debug("listen_overflow!\n"); DCCP_SKB_CB(skb)->dccpd_reset_code = DCCP_RESET_CODE_TOO_BUSY; drop: if (dccp_hdr(skb)->dccph_type != DCCP_PKT_RESET) req->rsk_ops->send_reset(sk, skb); inet_csk_reqsk_queue_drop(sk, req); - return NULL; +out: + spin_unlock_bh(&dreq->dreq_lock); + return child; } EXPORT_SYMBOL_GPL(dccp_check_req); @@ -246,6 +253,7 @@ int dccp_reqsk_init(struct request_sock *req, { struct dccp_request_sock *dreq = dccp_rsk(req); + spin_lock_init(&dreq->dreq_lock); inet_rsk(req)->ir_rmt_port = dccp_hdr(skb)->dccph_sport; inet_rsk(req)->ir_num = ntohs(dccp_hdr(skb)->dccph_dport); inet_rsk(req)->acked = 0;