From mboxrd@z Thu Jan 1 00:00:00 1970 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=-4.0 required=3.0 tests=MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=no 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 824E0C433E0 for ; Mon, 25 May 2020 10:52:31 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1746C2073B for ; Mon, 25 May 2020 10:52:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1746C2073B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 5B33A80034; Mon, 25 May 2020 06:52:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 566138E0008; Mon, 25 May 2020 06:52:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4A12E80034; Mon, 25 May 2020 06:52:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0024.hostedemail.com [216.40.44.24]) by kanga.kvack.org (Postfix) with ESMTP id 2EDC38E0008 for ; Mon, 25 May 2020 06:52:30 -0400 (EDT) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id DBC99180AD815 for ; Mon, 25 May 2020 10:52:29 +0000 (UTC) X-FDA: 76854927618.16.milk37_8a305e97df94f X-HE-Tag: milk37_8a305e97df94f X-Filterd-Recvd-Size: 7869 Received: from mail-ot1-f68.google.com (mail-ot1-f68.google.com [209.85.210.68]) by imf04.hostedemail.com (Postfix) with ESMTP for ; Mon, 25 May 2020 10:52:29 +0000 (UTC) Received: by mail-ot1-f68.google.com with SMTP id z3so13512601otp.9 for ; Mon, 25 May 2020 03:52:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=kn5DgIdG1SurfUunm0E75UQw8EiNvkA9Al8xtkzW3es=; b=KL/FEYk7Jxq3sv/7C9QylQSo+CQtUAKRsrCNcrFEb+uJh0Wg3D8QlpFnAybYFlqQiM UW2QNmfaj5BjI+zHf9NbAvbzjjAmPtrAnF4RXZdLrZVA4fQcgcfduFBBuujplI0vATbj Vzrs+UwXE4vEPlNugamA75+kAnRRseSWqsdWZp8sNN7WU9Jkg/fV+92ABmbsI3A93dXe kH/5dArhKXCiENLEM+ly5DbMqhagIZJT/j52kGuVIr1qxO0P8HA7g6NYLx3tnta+vzK8 9/Qs2SGnrnsHdrBIawW+qnSet6pucHFBlgkCI2VeTvMLpyuaJfaVEGFLkoN448XE3c9c PxxA== X-Gm-Message-State: AOAM531T+WVj08niQlJmqbImpBdMcLY8bL2l/J0DLaJDT90jzAh3GlwN x2xKfLL0DhQcfinhro0wTSa6S/Mdh8oT5jYTzzg= X-Google-Smtp-Source: ABdhPJzN9NDd9Y5vYW/zlgzrOQQ9k3eQjez5M7ybDE92g5OjVQvouP6M4mQN1W/hmM1/gB1byI9HMS7SGlnZuagf08A= X-Received: by 2002:a9d:3d05:: with SMTP id a5mr20639639otc.262.1590403948925; Mon, 25 May 2020 03:52:28 -0700 (PDT) MIME-Version: 1.0 References: <20200519181410.GA1963@dumbo> In-Reply-To: <20200519181410.GA1963@dumbo> From: "Rafael J. Wysocki" Date: Mon, 25 May 2020 12:52:17 +0200 Message-ID: Subject: Re: [PATCH v2] PM: hibernate: restrict writes to the resume device To: Domenico Andreoli , "Darrick J. Wong" Cc: Pavel Machek , Christoph Hellwig , Al Viro , "Ted Ts'o" , Len Brown , Linux PM , Linux Memory Management List , linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" 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: On Tue, May 19, 2020 at 8:14 PM Domenico Andreoli wrote: > > From: Domenico Andreoli > > Hibernation via snapshot device requires write permission to the swap > block device, the one that more often (but not necessarily) is used to > store the hibernation image. > > With this patch, such permissions are granted iff: > > 1) snapshot device config option is enabled > 2) swap partition is used as resume device > > In other circumstances the swap device is not writable from userspace. > > In order to achieve this, every write attempt to a swap device is > checked against the device configured as part of the uswsusp API [0] > using a pointer to the inode struct in memory. If the swap device being > written was not configured for resuming, the write request is denied. > > NOTE: this implementation works only for swap block devices, where the > inode configured by swapon (which sets S_SWAPFILE) is the same used > by SNAPSHOT_SET_SWAP_AREA. > > In case of swap file, SNAPSHOT_SET_SWAP_AREA indeed receives the inode > of the block device containing the filesystem where the swap file is > located (+ offset in it) which is never passed to swapon and then has > not set S_SWAPFILE. > > As result, the swap file itself (as a file) has never an option to be > written from userspace. Instead it remains writable if accessed directly > from the containing block device, which is always writeable from root. > > [0] Documentation/power/userland-swsusp.rst > > v2: > - rename is_hibernate_snapshot_dev() to is_hibernate_resume_dev() > - fix description so to correctly refer to the resume device > > Signed-off-by: Domenico Andreoli > Cc: "Rafael J. Wysocki" > Cc: Pavel Machek > Cc: Darrick J. Wong > Cc: Christoph Hellwig > Cc: viro@zeniv.linux.org.uk > Cc: tytso@mit.edu > Cc: len.brown@intel.com > Cc: linux-pm@vger.kernel.org > Cc: linux-mm@kvack.org > Cc: linux-xfs@vger.kernel.org > Cc: linux-fsdevel@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > > --- > fs/block_dev.c | 3 +-- > include/linux/suspend.h | 6 ++++++ > kernel/power/user.c | 14 +++++++++++++- > 3 files changed, 20 insertions(+), 3 deletions(-) > > Index: b/include/linux/suspend.h > =================================================================== > --- a/include/linux/suspend.h > +++ b/include/linux/suspend.h > @@ -466,6 +466,12 @@ static inline bool system_entering_hiber > static inline bool hibernation_available(void) { return false; } > #endif /* CONFIG_HIBERNATION */ > > +#ifdef CONFIG_HIBERNATION_SNAPSHOT_DEV > +int is_hibernate_resume_dev(const struct inode *); > +#else > +static inline int is_hibernate_resume_dev(const struct inode *i) { return 0; } > +#endif > + > /* Hibernation and suspend events */ > #define PM_HIBERNATION_PREPARE 0x0001 /* Going to hibernate */ > #define PM_POST_HIBERNATION 0x0002 /* Hibernation finished */ > Index: b/kernel/power/user.c > =================================================================== > --- a/kernel/power/user.c > +++ b/kernel/power/user.c > @@ -35,8 +35,14 @@ static struct snapshot_data { > bool ready; > bool platform_support; > bool free_bitmaps; > + struct inode *bd_inode; > } snapshot_state; > > +int is_hibernate_resume_dev(const struct inode *bd_inode) > +{ > + return hibernation_available() && snapshot_state.bd_inode == bd_inode; > +} > + > static int snapshot_open(struct inode *inode, struct file *filp) > { > struct snapshot_data *data; > @@ -95,6 +101,7 @@ static int snapshot_open(struct inode *i > data->frozen = false; > data->ready = false; > data->platform_support = false; > + data->bd_inode = NULL; > > Unlock: > unlock_system_sleep(); > @@ -110,6 +117,7 @@ static int snapshot_release(struct inode > > swsusp_free(); > data = filp->private_data; > + data->bd_inode = NULL; > free_all_swap_pages(data->swap); > if (data->frozen) { > pm_restore_gfp_mask(); > @@ -202,6 +210,7 @@ struct compat_resume_swap_area { > static int snapshot_set_swap_area(struct snapshot_data *data, > void __user *argp) > { > + struct block_device *bdev; > sector_t offset; > dev_t swdev; > > @@ -232,9 +241,12 @@ static int snapshot_set_swap_area(struct > data->swap = -1; > return -EINVAL; > } > - data->swap = swap_type_of(swdev, offset, NULL); > + data->swap = swap_type_of(swdev, offset, &bdev); > if (data->swap < 0) > return -ENODEV; > + > + data->bd_inode = bdev->bd_inode; > + bdput(bdev); > return 0; > } > > Index: b/fs/block_dev.c > =================================================================== > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -2023,8 +2023,7 @@ ssize_t blkdev_write_iter(struct kiocb * > if (bdev_read_only(I_BDEV(bd_inode))) > return -EPERM; > > - /* uswsusp needs write permission to the swap */ > - if (IS_SWAPFILE(bd_inode) && !hibernation_available()) > + if (IS_SWAPFILE(bd_inode) && !is_hibernate_resume_dev(bd_inode)) > return -ETXTBSY; > > if (!iov_iter_count(from)) > > -- The patch looks OK to me. Darrick, what do you think?