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 66C8EC433E0 for ; Wed, 27 May 2020 15:58:26 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1EA6F208E4 for ; Wed, 27 May 2020 15:58:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1EA6F208E4 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 AA5E4800B6; Wed, 27 May 2020 11:58:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A57DB80010; Wed, 27 May 2020 11:58:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 96CB8800B6; Wed, 27 May 2020 11:58:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0102.hostedemail.com [216.40.44.102]) by kanga.kvack.org (Postfix) with ESMTP id 80E3380010 for ; Wed, 27 May 2020 11:58:25 -0400 (EDT) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 2E08D801B36A for ; Wed, 27 May 2020 15:58:25 +0000 (UTC) X-FDA: 76862956170.30.cook39_5201dcc26d53 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin30.hostedemail.com (Postfix) with ESMTP id 15A9B18775B2B for ; Wed, 27 May 2020 15:58:25 +0000 (UTC) X-HE-Tag: cook39_5201dcc26d53 X-Filterd-Recvd-Size: 9207 Received: from mail-ot1-f66.google.com (mail-ot1-f66.google.com [209.85.210.66]) by imf33.hostedemail.com (Postfix) with ESMTP for ; Wed, 27 May 2020 15:58:24 +0000 (UTC) Received: by mail-ot1-f66.google.com with SMTP id u23so6790otq.10 for ; Wed, 27 May 2020 08:58:24 -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=AQuabQyL0NgSQawHuCBbM06Uabm6iw+SZxZ1s5FLlmI=; b=MGgOUYFZbSJ/DDzHChkIygpl73msdamBRTO1Cz1xjlSZD4qp4wh3FdSerJRia4pSTo 6AAkX3dkLQ3zwItgBMb7G2AYwjwEQ/D2vKoPlDEISM9At1wY3uoNtKyHW2jcZb7Trn8y M4bb7Jn7opUj+Enyqq6kjnlnpfXx6P9JIhuAsvbQbIU9L0IG9OA/6Wv63n1+nm7G9crh sPKe5WTDtN2aIv1uUahOFRPbTIXTgQcuo2eoeCvaskmw60cXr6Ln+TfX3SsHlrmhB9gK 7x8ppObAxbMfP4+w/ESK+CP+0QNvyQ0HGa481tQYgWBparmTCO+v4RDcF7+rNeYN1gua nM1g== X-Gm-Message-State: AOAM5309VTHKLJAhuUK5mdPzYjFmeIAp1+ZYp5w0tWOlZkZewnMvSF+A t/g9J2iwZuE5t8F3D0Pf0Xs2d61gnKxSQBPszXc= X-Google-Smtp-Source: ABdhPJxZVUkNZZJOj47bYNYMRKPYK5sTIzrR5L6ethTLfIfWOYb2kkqtX77sqotxWR7aaeZzl2Agv4JRbcsf2h3jXXw= X-Received: by 2002:a9d:3d05:: with SMTP id a5mr5178288otc.262.1590595103734; Wed, 27 May 2020 08:58:23 -0700 (PDT) MIME-Version: 1.0 References: <20200519181410.GA1963@dumbo> <20200526161932.GD252930@magnolia> In-Reply-To: <20200526161932.GD252930@magnolia> From: "Rafael J. Wysocki" Date: Wed, 27 May 2020 17:58:12 +0200 Message-ID: Subject: Re: [PATCH v2] PM: hibernate: restrict writes to the resume device To: "Darrick J. Wong" , Domenico Andreoli Cc: "Rafael J. Wysocki" , 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-Rspamd-Queue-Id: 15A9B18775B2B X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam01 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 26, 2020 at 6:19 PM Darrick J. Wong wrote: > > On Mon, May 25, 2020 at 12:52:17PM +0200, Rafael J. Wysocki wrote: > > 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? > > Looks fine to me too. > > I kinda wonder how uswsusp prevents the bdev from being swapoff'd (or > just plain disappearing) such that bd_inode will never point to a > recycled inode, but I guess since we're only comparing pointer values > it's not a big deal for this patch... > > Acked-by: Darrick J. Wong Thanks! So the patch has been applied as 5.8 material. Cheers!