From patchwork Thu May 13 06:48:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Huang, Ying" X-Patchwork-Id: 12255251 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CBBE2C433B4 for ; Thu, 13 May 2021 06:49:04 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5E62061435 for ; Thu, 13 May 2021 06:49:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5E62061435 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 98AD06B0036; Thu, 13 May 2021 02:49:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 93A8D6B006E; Thu, 13 May 2021 02:49:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7B4AB6B0070; Thu, 13 May 2021 02:49:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0219.hostedemail.com [216.40.44.219]) by kanga.kvack.org (Postfix) with ESMTP id 49EFD6B0036 for ; Thu, 13 May 2021 02:49:03 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id CCAB8B2AC for ; Thu, 13 May 2021 06:49:02 +0000 (UTC) X-FDA: 78135280524.21.FE0C020 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by imf09.hostedemail.com (Postfix) with ESMTP id 38130600010A for ; Thu, 13 May 2021 06:48:50 +0000 (UTC) IronPort-SDR: 8vhjNprVGfK1vpdGW5h/iZpwhLCGToVUAoikre0pFRHgcmlJwFwOtrHqObmM+juVf/RKuAsVOj nD34fuNi0eLw== X-IronPort-AV: E=McAfee;i="6200,9189,9982"; a="285386877" X-IronPort-AV: E=Sophos;i="5.82,296,1613462400"; d="scan'208";a="285386877" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 May 2021 23:49:00 -0700 IronPort-SDR: U9vE5e7D+uHcEkdlJ/9H2KC6Qxjc69tiJh3o7GqE9EzEZkUc7GIRHQNZ9GKnHijKme+lwIbi0/ WGEta5zyBZQg== X-IronPort-AV: E=Sophos;i="5.82,296,1613462400"; d="scan'208";a="625872650" Received: from yhuang6-desk1.sh.intel.com ([10.239.13.1]) by fmsmga005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 May 2021 23:48:56 -0700 From: Huang Ying To: Andrew Morton Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Huang Ying , Daniel Jordan , Dan Carpenter , Andrea Parri , Peter Zijlstra , Andi Kleen , Dave Hansen , Omar Sandoval , Paul McKenney , Tejun Heo , Will Deacon , Miaohe Lin Subject: [PATCH] mm, swap: Remove unnecessary smp_rmb() in swap_type_to_swap_info() Date: Thu, 13 May 2021 14:48:37 +0800 Message-Id: <20210513064837.3949064-1-ying.huang@intel.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=fail reason="No valid SPF, No valid DKIM" header.from=intel.com (policy=none); spf=none (imf09.hostedemail.com: domain of ying.huang@intel.com has no SPF policy when checking 192.55.52.43) smtp.mailfrom=ying.huang@intel.com X-Stat-Signature: zyuz8kh63tzqga6e7wkd1mwg9nqi75zf X-Rspamd-Queue-Id: 38130600010A X-Rspamd-Server: rspam02 Received-SPF: none (intel.com>: No applicable sender policy available) receiver=imf09; identity=mailfrom; envelope-from=""; helo=mga05.intel.com; client-ip=192.55.52.43 X-HE-DKIM-Result: none/none X-HE-Tag: 1620888530-936316 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Before commit c10d38cc8d3e ("mm, swap: bounds check swap_info array accesses to avoid NULL derefs"), the typical code to reference the swap_info[] is as follows, type = swp_type(swp_entry); if (type >= nr_swapfiles) /* handle invalid swp_entry */; p = swap_info[type]; /* access fields of *p. OOPS! p may be NULL! */ Because the ordering isn't guaranteed, it's possible that "p" is read before checking "type". And that may result in NULL pointer dereference. So in commit c10d38cc8d3e, the code becomes, struct swap_info_struct *swap_type_to_swap_info(int type) { if (type >= READ_ONCE(nr_swapfiles)) return NULL; smp_rmb(); return READ_ONCE(swap_info[type]); } /* users */ type = swp_type(swp_entry); p = swap_type_to_swap_info(type); if (!p) /* handle invalid swp_entry */; /* access fields of *p */ Because "p" is checked to be non-zero before dereference, smp_rmb() isn't needed anymore. We still need to guarantee swap_info[type] is read before dereference. That can be satisfied via the data dependency ordering of READ_ONCE(swap_info[type]). The corresponding smp_wmb() is adjusted in alloc_swap_info() too. And, we don't need to read "nr_swapfiles" too. Because if "type >= nr_swapfiles", swap_info[type] will be NULL. We just need to make sure we will not access out of the boundary of the array. With that change, nr_swapfiles will only be accessed with swap_lock held, except in swapcache_free_entries(). Where the absolute correctness of the value isn't needed, as described in the comments. Signed-off-by: "Huang, Ying" Cc: Daniel Jordan Cc: Dan Carpenter Cc: Andrea Parri Cc: Peter Zijlstra (Intel) Cc: Andi Kleen Cc: Dave Hansen Cc: Omar Sandoval Cc: Paul McKenney Cc: Tejun Heo Cc: Will Deacon Cc: Miaohe Lin --- mm/swapfile.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/mm/swapfile.c b/mm/swapfile.c index 2aad85751991..4c1fb28bbe0e 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -100,10 +100,14 @@ atomic_t nr_rotate_swap = ATOMIC_INIT(0); static struct swap_info_struct *swap_type_to_swap_info(int type) { - if (type >= READ_ONCE(nr_swapfiles)) + if (type >= MAX_SWAPFILES) return NULL; - smp_rmb(); /* Pairs with smp_wmb in alloc_swap_info. */ + /* + * The data dependency ordering from the READ_ONCE() pairs + * with smp_wmb() in alloc_swap_info() to guarantee the + * swap_info_struct fields are read after swap_info[type]. + */ return READ_ONCE(swap_info[type]); } @@ -2884,14 +2888,10 @@ static struct swap_info_struct *alloc_swap_info(void) } if (type >= nr_swapfiles) { p->type = type; - WRITE_ONCE(swap_info[type], p); - /* - * Write swap_info[type] before nr_swapfiles, in case a - * racing procfs swap_start() or swap_next() is reading them. - * (We never shrink nr_swapfiles, we never free this entry.) - */ + /* Paired with READ_ONCE() in swap_type_to_swap_info() */ smp_wmb(); - WRITE_ONCE(nr_swapfiles, nr_swapfiles + 1); + WRITE_ONCE(swap_info[type], p); + nr_swapfiles++; } else { defer = p; p = swap_info[type];