linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] PM: hibernate: restrict writes to the resume device
@ 2020-05-19 18:14 Domenico Andreoli
  2020-05-25 10:52 ` Rafael J. Wysocki
  0 siblings, 1 reply; 4+ messages in thread
From: Domenico Andreoli @ 2020-05-19 18:14 UTC (permalink / raw)
  To: Rafael J. Wysocki, Darrick J. Wong
  Cc: Pavel Machek, Christoph Hellwig, viro, tytso, len.brown,
	linux-pm, linux-mm, linux-xfs, linux-fsdevel, linux-kernel,
	Domenico Andreoli

From: Domenico Andreoli <domenico.andreoli@linux.com>

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 <domenico.andreoli@linux.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Darrick J. Wong <darrick.wong@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
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))

-- 
rsa4096: 3B10 0CA1 8674 ACBA B4FE  FCD2 CE5B CF17 9960 DE13
ed25519: FFB4 0CC3 7F2E 091D F7DA  356E CC79 2832 ED38 CB05

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] PM: hibernate: restrict writes to the resume device
  2020-05-19 18:14 [PATCH v2] PM: hibernate: restrict writes to the resume device Domenico Andreoli
@ 2020-05-25 10:52 ` Rafael J. Wysocki
  2020-05-26 16:19   ` Darrick J. Wong
  0 siblings, 1 reply; 4+ messages in thread
From: Rafael J. Wysocki @ 2020-05-25 10:52 UTC (permalink / raw)
  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,
	linux-fsdevel, Linux Kernel Mailing List

On Tue, May 19, 2020 at 8:14 PM Domenico Andreoli
<domenico.andreoli@linux.com> wrote:
>
> From: Domenico Andreoli <domenico.andreoli@linux.com>
>
> 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 <domenico.andreoli@linux.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Darrick J. Wong <darrick.wong@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> 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?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] PM: hibernate: restrict writes to the resume device
  2020-05-25 10:52 ` Rafael J. Wysocki
@ 2020-05-26 16:19   ` Darrick J. Wong
  2020-05-27 15:58     ` Rafael J. Wysocki
  0 siblings, 1 reply; 4+ messages in thread
From: Darrick J. Wong @ 2020-05-26 16:19 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Domenico Andreoli, Pavel Machek, Christoph Hellwig, Al Viro,
	Ted Ts'o, Len Brown, Linux PM, Linux Memory Management List,
	linux-xfs, linux-fsdevel, Linux Kernel Mailing List

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
> <domenico.andreoli@linux.com> wrote:
> >
> > From: Domenico Andreoli <domenico.andreoli@linux.com>
> >
> > 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 <domenico.andreoli@linux.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: Darrick J. Wong <darrick.wong@oracle.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > 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 <darrick.wong@oracle.com>

--D


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] PM: hibernate: restrict writes to the resume device
  2020-05-26 16:19   ` Darrick J. Wong
@ 2020-05-27 15:58     ` Rafael J. Wysocki
  0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2020-05-27 15:58 UTC (permalink / raw)
  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, linux-fsdevel, Linux Kernel Mailing List

On Tue, May 26, 2020 at 6:19 PM Darrick J. Wong <darrick.wong@oracle.com> 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
> > <domenico.andreoli@linux.com> wrote:
> > >
> > > From: Domenico Andreoli <domenico.andreoli@linux.com>
> > >
> > > 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 <domenico.andreoli@linux.com>
> > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > Cc: Pavel Machek <pavel@ucw.cz>
> > > Cc: Darrick J. Wong <darrick.wong@oracle.com>
> > > Cc: Christoph Hellwig <hch@lst.de>
> > > 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 <darrick.wong@oracle.com>

Thanks!

So the patch has been applied as 5.8 material.

Cheers!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-05-27 15:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 18:14 [PATCH v2] PM: hibernate: restrict writes to the resume device Domenico Andreoli
2020-05-25 10:52 ` Rafael J. Wysocki
2020-05-26 16:19   ` Darrick J. Wong
2020-05-27 15:58     ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).