Received: by 10.223.176.5 with SMTP id f5csp702868wra; Fri, 9 Feb 2018 05:58:25 -0800 (PST) X-Google-Smtp-Source: AH8x227GbsUAT2g+mLAjZjUJf4bdzd1+GYpzybTKnYxJnLTX5v8VP5K+bVTzYr8arv03JCWpZbAQ X-Received: by 2002:a17:902:b413:: with SMTP id x19-v6mr2629689plr.420.1518184705714; Fri, 09 Feb 2018 05:58:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1518184705; cv=none; d=google.com; s=arc-20160816; b=G6MEdyXCXXuTR88a4OtklMmvUoAeThUDT6WV+n0E5rZmRHqdW8lVSUWiRHl5QW1xnS JpDZ+8Ua+5N9u+pOKB2UmbLBRYlSZl4k/LvVllxTZssTZTUuyYE3t/nEls7KebLrxmKM rwxQ7CUan4GspaKe72lyncHV2jyMF8xk8TtGFfFw9vYxZTo2Hbm0yxz9X0N+lsKv26Ca IsPktfhrydddprrFxGNHWOPeGHNecuFUSbTiVd1i6xNu9LeooBKEKE2Q4QPpy+kF9a26 GnGvkyc8gEPeDQxJ4umK6Rghbc/d0qUldxSxv1Tyknjnpb8zX6MewQLsM4n220RHOcMX sHJg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:user-agent:references :in-reply-to:message-id:date:subject:cc:to:from :arc-authentication-results; bh=ryzw77gUy7qK1slvZS1QRO5u252KmL0UFw9dqvbiinU=; b=Ff/srUDbYXZFosnSoUBdnCxEyyt6ye9vlZ/j/Vd8SDlYWaTVcfGpfyPd0fIY5p6/MJ 1h8NlF/3ej5V/R5Cf3g6mAAv7+PIxiga1CvDbmpdMs9t4bqmLALFOM5132xVbFrqCtlo SvPeXVEmBhkNwOoOOwimKf6BZUuGvizNeCFre/Bt3qvwaq2QES+FtLnwWKibpUqIUqON +2rtLbHncP+Io9gdBqiF4AGOr5/djjY0DH8JtpCoTdSyPnTpmm+thfDPnBPPnw+eXTlU eQr+GALvO7j4GkqufuyLCOPO1ixSmgdGVgbcPCw7NICDJvb/43WlFsh2Q124Fx6Pqb1k +5vg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 99-v6si1607300plf.440.2018.02.09.05.58.11; Fri, 09 Feb 2018 05:58:25 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753554AbeBINpH (ORCPT + 99 others); Fri, 9 Feb 2018 08:45:07 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:51692 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753475AbeBINpE (ORCPT ); Fri, 9 Feb 2018 08:45:04 -0500 Received: from localhost (LFbn-1-12258-90.w90-92.abo.wanadoo.fr [90.92.71.90]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id 7E54910C4; Fri, 9 Feb 2018 13:45:03 +0000 (UTC) From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Roman Gushchin , Eric Dumazet , "David S. Miller" , Johannes Weiner , Tejun Heo Subject: [PATCH 4.14 11/22] Revert "defer call to mem_cgroup_sk_alloc()" Date: Fri, 9 Feb 2018 14:40:00 +0100 Message-Id: <20180209133934.875219838@linuxfoundation.org> X-Mailer: git-send-email 2.16.1 In-Reply-To: <20180209133934.024795822@linuxfoundation.org> References: <20180209133934.024795822@linuxfoundation.org> User-Agent: quilt/0.65 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.14-stable review patch. If anyone has any objections, please let me know. ------------------ From: Roman Gushchin [ Upstream commit edbe69ef2c90fc86998a74b08319a01c508bd497 ] This patch effectively reverts commit 9f1c2674b328 ("net: memcontrol: defer call to mem_cgroup_sk_alloc()"). Moving mem_cgroup_sk_alloc() to the inet_csk_accept() completely breaks memcg socket memory accounting, as packets received before memcg pointer initialization are not accounted and are causing refcounting underflow on socket release. Actually the free-after-use problem was fixed by commit c0576e397508 ("net: call cgroup_sk_alloc() earlier in sk_clone_lock()") for the cgroup pointer. So, let's revert it and call mem_cgroup_sk_alloc() just before cgroup_sk_alloc(). This is safe, as we hold a reference to the socket we're cloning, and it holds a reference to the memcg. Also, let's drop BUG_ON(mem_cgroup_is_root()) check from mem_cgroup_sk_alloc(). I see no reasons why bumping the root memcg counter is a good reason to panic, and there are no realistic ways to hit it. Signed-off-by: Roman Gushchin Cc: Eric Dumazet Cc: David S. Miller Cc: Johannes Weiner Cc: Tejun Heo Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- mm/memcontrol.c | 14 ++++++++++++++ net/core/sock.c | 5 +---- net/ipv4/inet_connection_sock.c | 1 - 3 files changed, 15 insertions(+), 5 deletions(-) --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5828,6 +5828,20 @@ void mem_cgroup_sk_alloc(struct sock *sk if (!mem_cgroup_sockets_enabled) return; + /* + * Socket cloning can throw us here with sk_memcg already + * filled. It won't however, necessarily happen from + * process context. So the test for root memcg given + * the current task's memcg won't help us in this case. + * + * Respecting the original socket's memcg is a better + * decision in this case. + */ + if (sk->sk_memcg) { + css_get(&sk->sk_memcg->css); + return; + } + rcu_read_lock(); memcg = mem_cgroup_from_task(current); if (memcg == root_mem_cgroup) --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1677,16 +1677,13 @@ struct sock *sk_clone_lock(const struct newsk->sk_dst_pending_confirm = 0; newsk->sk_wmem_queued = 0; newsk->sk_forward_alloc = 0; - - /* sk->sk_memcg will be populated at accept() time */ - newsk->sk_memcg = NULL; - atomic_set(&newsk->sk_drops, 0); newsk->sk_send_head = NULL; newsk->sk_userlocks = sk->sk_userlocks & ~SOCK_BINDPORT_LOCK; atomic_set(&newsk->sk_zckey, 0); sock_reset_flag(newsk, SOCK_DONE); + mem_cgroup_sk_alloc(newsk); cgroup_sk_alloc(&newsk->sk_cgrp_data); rcu_read_lock(); --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -475,7 +475,6 @@ struct sock *inet_csk_accept(struct sock } spin_unlock_bh(&queue->fastopenq.lock); } - mem_cgroup_sk_alloc(newsk); out: release_sock(sk); if (req)