From patchwork Tue Jan 15 00:23:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Jordan X-Patchwork-Id: 10763659 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1E493746 for ; Tue, 15 Jan 2019 00:23:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 042FB2C1E3 for ; Tue, 15 Jan 2019 00:23:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E74AB2C291; Tue, 15 Jan 2019 00:23:37 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 576302C1E3 for ; Tue, 15 Jan 2019 00:23:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3DF3C8E0003; Mon, 14 Jan 2019 19:23:35 -0500 (EST) Delivered-To: linux-mm-outgoing@kvack.org Received: by kanga.kvack.org (Postfix, from userid 40) id 38CEA8E0002; Mon, 14 Jan 2019 19:23:35 -0500 (EST) X-Original-To: int-list-linux-mm@kvack.org X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 256E08E0003; Mon, 14 Jan 2019 19:23:35 -0500 (EST) X-Original-To: linux-mm@kvack.org X-Delivered-To: linux-mm@kvack.org Received: from mail-yb1-f200.google.com (mail-yb1-f200.google.com [209.85.219.200]) by kanga.kvack.org (Postfix) with ESMTP id E947B8E0002 for ; Mon, 14 Jan 2019 19:23:34 -0500 (EST) Received: by mail-yb1-f200.google.com with SMTP id r9so436284ybp.16 for ; Mon, 14 Jan 2019 16:23:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:dkim-signature:from:to:cc:subject:date :message-id:in-reply-to:references:mime-version :content-transfer-encoding; bh=Iec5fTCgy6dLIMaBGCTdOd4yL0swh5y5vdcBFKfE5a4=; b=Z3hEmnXEk9vkIa8A24SGjYEKYYtwauNl4g9HSSWfD3H7FX5s8Wxwe20kpeAIMG+FCP RgQvshYEtkZnfJRjD3qoyYYrYcqVEVHDmaJW7HzneW/gSfKg3XMLlE4oQb+3I36v9B3Q XSa+f0X83aHw2tIYv+OHr1NFxkwvo5u8/lnBNAEFdx19BEdHYisNPQe+7mADPayI+wv9 InKZLhY2m/gpTlSJ6dcjUcmlDlBk+1a7sfmjcNaTG0nXrSva+k6NzA+3AMa1rBrD5qBy 5xDEGZ+eJM+ATq/2Ksk9h/wRAiaERi+LQCuJ8pfylxwgBV93XElYNqBgK5kruvfS1TWN 2i7A== X-Gm-Message-State: AJcUukefmaH08OfKo+J/3HKcf38tjjq+TENZarZub+ZmlDUbHyyBP2L1 +5ATrMh035IRCaQT4gVokIPsSifN8iYcCK/d1IaOm/FvzbKM3VuLk0QXurhWe4L21jFarqb8OqT t9S03fGyb3+WQNaHFm6uQlcfGOb5nVK7piThMoYKX0KdtsjbjOCP566W+UNhUhbWtfw== X-Received: by 2002:a25:3dc4:: with SMTP id k187mr899510yba.480.1547511814650; Mon, 14 Jan 2019 16:23:34 -0800 (PST) X-Google-Smtp-Source: ALg8bN7VrI5jK5iOlA8FJrP1YwTVZnpDox1VRVig3V7YikqNErxlPeEKYSezqcokXjrtUR4uxR+V X-Received: by 2002:a25:3dc4:: with SMTP id k187mr899483yba.480.1547511813800; Mon, 14 Jan 2019 16:23:33 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1547511813; cv=none; d=google.com; s=arc-20160816; b=ooko5lBFMkM9uffimsPJZd6qqXBjVxJ10UL4P9fxWpK9YnNWI9Z8L+iKZztcOnIoLq qso7AFdpZmY4YFVb17bR7jtwwfo9oBZg1rGHXqN+niKoeIJtkL2xMGdnpXkqpLRYBsxI /BMfnR3j2AeZ2ZTrtGM3+KpJpiLu6KSwWUZIvsfhp4eFwh2P6LpRkudlUDvzqDBULYzQ R4jtw15tbOqE3BINcx5TuIr9scJ5k3S4zYhz07LpXyn6skuGxEezYx2PUDVevItXg01K IC7kjftzzkgU/bDxFV3p+kAbm5pEl95qLz71mBBXCEUSvs7pttqYl2mI63AnPFho4POT jX3A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:dkim-signature; bh=Iec5fTCgy6dLIMaBGCTdOd4yL0swh5y5vdcBFKfE5a4=; b=Ass4yYgjKznkYTWa2O8WSLtohTspeKXz/TM0iXKwkKfZluup2Mav5Mbkb7kgHqQTOp dJjYPn4+WBTNaW8/mTHboLRZReHFh7WPf14towLf2pYmmUj9zmOskdA39sg8butqGq07 RPmezWH924wE6ydWiR0t+mkNSgUrfPXgVXk/vWLwqJBBbEFtErmUWY2Ka745X43bCUVM c1RZ+l4ig9wvo4nqm2nKcY0HJ0H27k3zbQmwuNyusTeUPzwXNRXu4ySdjyAY7k8nZtcu NQH2DZXbiliSLNBK0xpfGj6/1iwzHgA4ilRM4k1rb7NGJGPyVBmEVeb58H9Z2zqszWhF EzcA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@oracle.com header.s=corp-2018-07-02 header.b=Ybmr4kvs; spf=pass (google.com: domain of daniel.m.jordan@oracle.com designates 156.151.31.85 as permitted sender) smtp.mailfrom=daniel.m.jordan@oracle.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=oracle.com Received: from userp2120.oracle.com (userp2120.oracle.com. [156.151.31.85]) by mx.google.com with ESMTPS id k11si1167380ybd.76.2019.01.14.16.23.33 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 14 Jan 2019 16:23:33 -0800 (PST) Received-SPF: pass (google.com: domain of daniel.m.jordan@oracle.com designates 156.151.31.85 as permitted sender) client-ip=156.151.31.85; Authentication-Results: mx.google.com; dkim=pass header.i=@oracle.com header.s=corp-2018-07-02 header.b=Ybmr4kvs; spf=pass (google.com: domain of daniel.m.jordan@oracle.com designates 156.151.31.85 as permitted sender) smtp.mailfrom=daniel.m.jordan@oracle.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=oracle.com Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id x0F0J8eR020464; Tue, 15 Jan 2019 00:23:16 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=corp-2018-07-02; bh=Iec5fTCgy6dLIMaBGCTdOd4yL0swh5y5vdcBFKfE5a4=; b=Ybmr4kvsioDJGOT2aD8iZZuh7KzuA8ynzGS/sv6MWAyxgYvwo506m+9lLiYhuIUMdc7k 4pNqoxXvIO79QiUBk1zobNkLelgOhQkE0V9XU5gDIXsSbJu4mTyI65riYjdRcUPhC2um RQj4RU6AnvZDJ5ZcgdjKcAoOXfp3cBfWYQZ1ItKVSwp4+kJUzsjM0zoVZoOoWnl6OjfQ v7CfG0qQ3V5V/gGrU2a9AQIvJwdeAbYFO/dooY72RCHJvXf1o0IiHNmsllIMp0J/LlyY +fZHnLGSeiHWKRqXumzvXjd3qLvYoJWgsISVL+RHdblkvNqK03ERZYlSRKqjyeAYYBNG 5Q== Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by userp2120.oracle.com with ESMTP id 2pybjs0yvj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 15 Jan 2019 00:23:16 +0000 Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id x0F0NElq025328 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 15 Jan 2019 00:23:14 GMT Received: from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id x0F0NBkm024624; Tue, 15 Jan 2019 00:23:12 GMT Received: from localhost.localdomain (/73.60.114.248) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 14 Jan 2019 16:23:11 -0800 From: Daniel Jordan To: akpm@linux-foundation.org Cc: dan.carpenter@oracle.com, andrea.parri@amarulasolutions.com, shli@kernel.org, ying.huang@intel.com, dave.hansen@linux.intel.com, sfr@canb.auug.org.au, osandov@fb.com, tj@kernel.org, ak@linux.intel.com, linux-mm@kvack.org, kernel-janitors@vger.kernel.org, paulmck@linux.ibm.com, stern@rowland.harvard.edu, peterz@infradead.org, will.deacon@arm.com, daniel.m.jordan@oracle.com Subject: [PATCH] mm, swap: bounds check swap_info accesses to avoid NULL derefs Date: Mon, 14 Jan 2019 19:23:05 -0500 Message-Id: <20190115002305.15402-1-daniel.m.jordan@oracle.com> X-Mailer: git-send-email 2.20.0 In-Reply-To: <20190114222529.43zay6r242ipw5jb@ca-dmjordan1.us.oracle.com> References: <20190114222529.43zay6r242ipw5jb@ca-dmjordan1.us.oracle.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=9136 signatures=668680 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=3 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1810050000 definitions=main-1901150000 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: X-Virus-Scanned: ClamAV using ClamSMTP Dan Carpenter reports a potential NULL dereference in get_swap_page_of_type: Smatch complains that the NULL checks on "si" aren't consistent. This seems like a real bug because we have not ensured that the type is valid and so "si" can be NULL. Add the missing check for NULL, taking care to use a read barrier to ensure CPU1 observes CPU0's updates in the correct order: CPU0 CPU1 alloc_swap_info() if (type >= nr_swapfiles) swap_info[type] = p /* handle invalid entry */ smp_wmb() smp_rmb() ++nr_swapfiles p = swap_info[type] Without smp_rmb, CPU1 might observe CPU0's write to nr_swapfiles before CPU0's write to swap_info[type] and read NULL from swap_info[type]. Ying Huang noticed that other places don't order these reads properly. Introduce swap_type_to_swap_info to encourage correct usage. Use READ_ONCE and WRITE_ONCE to follow the Linux Kernel Memory Model (see tools/memory-model/Documentation/explanation.txt). This ordering need not be enforced in places where swap_lock is held (e.g. si_swapinfo) because swap_lock serializes updates to nr_swapfiles and the swap_info array. This is a theoretical problem, no actual reports of it exist. Fixes: ec8acf20afb8 ("swap: add per-partition lock for swapfile") Reported-by: Dan Carpenter Signed-off-by: Daniel Jordan Cc: Alan Stern Cc: Andi Kleen Cc: Andrea Parri Cc: Andrew Morton Cc: Dan Carpenter Cc: Dave Hansen Cc: Huang Ying Cc: Omar Sandoval Cc: Paul McKenney Cc: Peter Zijlstra (Intel) Cc: Shaohua Li Cc: Stephen Rothwell Cc: Tejun Heo Cc: Will Deacon Reviewed-by: Andrea Parri Acked-by: Peter Zijlstra (Intel) --- I'd appreciate it if someone more familiar with memory barriers could check this over. Thanks. Probably no need for stable, this is all theoretical. Against linux-mmotm tag v5.0-rc1-mmotm-2019-01-09-13-40 mm/swapfile.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/mm/swapfile.c b/mm/swapfile.c index f0edf7244256..dad52fc67045 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -99,6 +99,15 @@ static atomic_t proc_poll_event = ATOMIC_INIT(0); 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)) + return NULL; + + smp_rmb(); /* Pairs with smp_wmb in alloc_swap_info. */ + return READ_ONCE(swap_info[type]); +} + static inline unsigned char swap_count(unsigned char ent) { return ent & ~SWAP_HAS_CACHE; /* may include COUNT_CONTINUED flag */ @@ -1045,12 +1054,14 @@ int get_swap_pages(int n_goal, swp_entry_t swp_entries[], int entry_size) /* The only caller of this function is now suspend routine */ swp_entry_t get_swap_page_of_type(int type) { - struct swap_info_struct *si; + struct swap_info_struct *si = swap_type_to_swap_info(type); pgoff_t offset; - si = swap_info[type]; + if (!si) + goto fail; + spin_lock(&si->lock); - if (si && (si->flags & SWP_WRITEOK)) { + if (si->flags & SWP_WRITEOK) { atomic_long_dec(&nr_swap_pages); /* This is called for allocating swap entry, not cache */ offset = scan_swap_map(si, 1); @@ -1061,6 +1072,7 @@ swp_entry_t get_swap_page_of_type(int type) atomic_long_inc(&nr_swap_pages); } spin_unlock(&si->lock); +fail: return (swp_entry_t) {0}; } @@ -1072,9 +1084,9 @@ static struct swap_info_struct *__swap_info_get(swp_entry_t entry) if (!entry.val) goto out; type = swp_type(entry); - if (type >= nr_swapfiles) + p = swap_type_to_swap_info(type); + if (!p) goto bad_nofile; - p = swap_info[type]; if (!(p->flags & SWP_USED)) goto bad_device; offset = swp_offset(entry); @@ -1212,9 +1224,9 @@ struct swap_info_struct *get_swap_device(swp_entry_t entry) if (!entry.val) goto out; type = swp_type(entry); - if (type >= nr_swapfiles) + si = swap_type_to_swap_info(type); + if (!si) goto bad_nofile; - si = swap_info[type]; preempt_disable(); if (!(si->flags & SWP_VALID)) @@ -1765,10 +1777,9 @@ int swap_type_of(dev_t device, sector_t offset, struct block_device **bdev_p) sector_t swapdev_block(int type, pgoff_t offset) { struct block_device *bdev; + struct swap_info_struct *si = swap_type_to_swap_info(type); - if ((unsigned int)type >= nr_swapfiles) - return 0; - if (!(swap_info[type]->flags & SWP_WRITEOK)) + if (!si || !(si->flags & SWP_WRITEOK)) return 0; return map_swap_entry(swp_entry(type, offset), &bdev); } @@ -2799,9 +2810,9 @@ static void *swap_start(struct seq_file *swap, loff_t *pos) if (!l) return SEQ_START_TOKEN; - for (type = 0; type < nr_swapfiles; type++) { + for (type = 0; type < READ_ONCE(nr_swapfiles); type++) { smp_rmb(); /* read nr_swapfiles before swap_info[type] */ - si = swap_info[type]; + si = READ_ONCE(swap_info[type]); if (!(si->flags & SWP_USED) || !si->swap_map) continue; if (!--l) @@ -2821,9 +2832,9 @@ static void *swap_next(struct seq_file *swap, void *v, loff_t *pos) else type = si->type + 1; - for (; type < nr_swapfiles; type++) { + for (; type < READ_ONCE(nr_swapfiles); type++) { smp_rmb(); /* read nr_swapfiles before swap_info[type] */ - si = swap_info[type]; + si = READ_ONCE(swap_info[type]); if (!(si->flags & SWP_USED) || !si->swap_map) continue; ++*pos; @@ -2930,14 +2941,14 @@ static struct swap_info_struct *alloc_swap_info(void) } if (type >= nr_swapfiles) { p->type = type; - swap_info[type] = p; + 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.) */ smp_wmb(); - nr_swapfiles++; + WRITE_ONCE(nr_swapfiles, nr_swapfiles + 1); } else { kvfree(p); p = swap_info[type];