mm/swap, PM: hibernate: atomically replace hibernation pin

snapshot_set_swap_area() unpins the previously selected swap device and pins the new one in two separate swap_lock critical sections. In the gap between them, swapoff() observes SWP_HIBERNATION cleared, bypasses the guard, and tears down the device, reopening the race the SWP_HIBERNATION pin was meant to close. The window is reachable on any SNAPSHOT_SET_SWAP_AREA call after the snapshot device is opened for hibernation, and on any retry after the resume path's first selection.

Add repin_hibernation_swap_type(), which looks up the new device, clears the old SWP_HIBERNATION flag and sets the new one under a single swap_lock acquisition. The same-device case is short- circuited so userspace can re-select the same swap area without tripping WARN_ON_ONCE and -EBUSY. Switch snapshot_set_swap_area() to the new helper.

A failed lookup now preserves the previous pin instead of dropping it, so a bad SNAPSHOT_SET_SWAP_AREA leaves the prior selection intact. The open and release paths keep using pin_hibernation_swap_type() and unpin_hibernation_swap_type().

The race was identified during AI-assisted review of the SWP_HIBERNATION pinning series.

Quoted 2 lines · On Thu, 30 Apr 2026 20:56:51 +0100 David Carlier <[email protected]> wrote:

The race was identified during AI-assisted review of the SWP_HIBERNATION pinning series.

Thanks, I'll queue this for testing. Can the swap maintainers please do a detailed review at some point?

snapshot_set_swap_area() unpins the previously selected swap device and pins the new one in two separate swap_lock critical sections. In the gap between them, swapoff() observes SWP_HIBERNATION cleared, bypasses the guard, and tears down the device, reopening the race the SWP_HIBERNATION pin was meant to close. The window is reachable on any SNAPSHOT_SET_SWAP_AREA call after the snapshot device is opened for hibernation, and on any retry after the resume path's first selection.

Add repin_hibernation_swap_type(), which looks up the new device, clears the old SWP_HIBERNATION flag and sets the new one under a single swap_lock acquisition. The same-device case is short- circuited so userspace can re-select the same swap area without tripping WARN_ON_ONCE and -EBUSY. Switch snapshot_set_swap_area() to the new helper.

A failed lookup now preserves the previous pin instead of dropping it, so a bad SNAPSHOT_SET_SWAP_AREA leaves the prior selection intact. The open and release paths keep using pin_hibernation_swap_type() and unpin_hibernation_swap_type().

The race was identified during AI-assisted review of the SWP_HIBERNATION pinning series.

Fixes 8e6e0d845823 ("mm/swap, PM: hibernate: fix swapoff race in uswsusp by pinning swap device")

Assisted-by Codex (gpt-5-codex)

Signed-off-by David Carlier <[email protected]>

---
 include/linux/swap.h |  2 ++
 kernel/power/user.c  | 17 ++++--------
 mm/swapfile.c        | 61 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 12 deletions(-)

include/linux/swap.h +2 -0

index 1930f81e6be4..213ecb627a39 100644
@@ -436,6 +436,8 @@ static inline long get_nr_swap_pages(void)
 extern void si_swapinfo(struct sysinfo *);
 extern int pin_hibernation_swap_type(dev_t device, sector_t offset);
 extern void unpin_hibernation_swap_type(int type);
+extern int repin_hibernation_swap_type(int old_type, dev_t device,
+                                       sector_t offset);
 extern int find_hibernation_swap_type(dev_t device, sector_t offset);
 int find_first_swap(dev_t *device);
 extern unsigned int count_swap_pages(int, int);

kernel/power/user.c +5 -12

index d0fcfba7ac23..6e4f40e49319 100644
@@ -218,6 +218,7 @@ static int snapshot_set_swap_area(struct snapshot_data *data,
 {
 	sector_t offset;
 	dev_t swdev;
+	int new_type;
 
 	if (swsusp_swap_in_use())
 		return -EPERM;

@@ -238,19 +239,11 @@ static int snapshot_set_swap_area(struct snapshot_data *data,
 		offset = swap_area.offset;
 	}
 
-	/*
-	 * Unpin the swap device if a swap area was already
-	 * set by SNAPSHOT_SET_SWAP_AREA.
-	 */
-	unpin_hibernation_swap_type(data->swap);
+	new_type = repin_hibernation_swap_type(data->swap, swdev, offset);
+	if (new_type < 0)
+		return new_type;
 
-	/*
-	 * User space encodes device types as two-byte values,
-	 * so we need to recode them
-	 */
-	data->swap = pin_hibernation_swap_type(swdev, offset);
-	if (data->swap < 0)
-		return swdev ? -ENODEV : -EINVAL;
+	data->swap = new_type;
 	data->dev = swdev;
 	return 0;
 }

mm/swapfile.c +61 -0

index c7e173b93e11..4840fd40f36f 100644
@@ -2219,6 +2219,67 @@ int pin_hibernation_swap_type(dev_t device, sector_t offset)
 	return type;
 }
 
+/**
+ * repin_hibernation_swap_type - Atomically replace the hibernation pin
+ * @old_type: Swap type currently pinned (or < 0 if none).
+ * @device:   Block device of the new resume image.
+ * @offset:   Offset identifying the new swap area.
+ *
+ * Look up the swap device for @device/@offset and atomically transfer
+ * the SWP_HIBERNATION pin from @old_type (if valid) to the new device,
+ * all under a single swap_lock critical section. This closes the
+ * swapoff() window that exists when callers unpin and re-pin in two
+ * separate operations.
+ *
+ * If the new device cannot be located, the existing pin on @old_type
+ * is preserved and an error is returned. If @old_type already refers
+ * to the same swap_info_struct as the new lookup, no flag changes are
+ * made and @old_type is returned.
+ *
+ * Return:
+ * >= 0 on success (new swap type).
+ * -EINVAL if @device is invalid.
+ * -ENODEV if the swap device is not found.
+ * -EBUSY  if the new device is already pinned by another context.
+ */
+int repin_hibernation_swap_type(int old_type, dev_t device, sector_t offset)
+{
+	struct swap_info_struct *old_si, *new_si;
+	int new_type;
+
+	spin_lock(&swap_lock);
+
+	new_type = __find_hibernation_swap_type(device, offset);
+	if (new_type < 0) {
+		spin_unlock(&swap_lock);
+		return new_type;
+	}
+
+	new_si = swap_type_to_info(new_type);
+	if (WARN_ON_ONCE(!new_si)) {
+		spin_unlock(&swap_lock);
+		return -ENODEV;
+	}
+
+	old_si = swap_type_to_info(old_type);
+	if (new_si == old_si) {
+		spin_unlock(&swap_lock);
+		return new_type;
+	}
+
+	if (WARN_ON_ONCE(new_si->flags & SWP_HIBERNATION)) {
+		spin_unlock(&swap_lock);
+		return -EBUSY;
+	}
+
+	if (old_si)
+		old_si->flags &= ~SWP_HIBERNATION;
+	new_si->flags |= SWP_HIBERNATION;
+
+	spin_unlock(&swap_lock);
+	return new_type;
+}
+
 /**
  * unpin_hibernation_swap_type - Unpin the swap device for hibernation
  * @type: Swap type previously returned by pin_hibernation_swap_type()

Signature · 1 line

2.53.0

Quoted 155 lines

snapshot_set_swap_area() unpins the previously selected swap device and pins the new one in two separate swap_lock critical sections. In the gap between them, swapoff() observes SWP_HIBERNATION cleared, bypasses the guard, and tears down the device, reopening the race the SWP_HIBERNATION pin was meant to close. The window is reachable on any SNAPSHOT_SET_SWAP_AREA call after the snapshot device is opened for hibernation, and on any retry after the resume path's first selection.

Add repin_hibernation_swap_type(), which looks up the new device, clears the old SWP_HIBERNATION flag and sets the new one under a single swap_lock acquisition. The same-device case is short- circuited so userspace can re-select the same swap area without tripping WARN_ON_ONCE and -EBUSY. Switch snapshot_set_swap_area() to the new helper.

A failed lookup now preserves the previous pin instead of dropping it, so a bad SNAPSHOT_SET_SWAP_AREA leaves the prior selection intact. The open and release paths keep using pin_hibernation_swap_type() and unpin_hibernation_swap_type().

The race was identified during AI-assisted review of the SWP_HIBERNATION pinning series.

Fixes 8e6e0d845823 ("mm/swap, PM: hibernate: fix swapoff race in uswsusp by pinning swap device")

Assisted-by Codex (gpt-5-codex)

Signed-off-by David Carlier <[email protected]>

---
 include/linux/swap.h |  2 ++
 kernel/power/user.c  | 17 ++++--------
 mm/swapfile.c        | 61 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 12 deletions(-)

include/linux/swap.h +2 -0

index 1930f81e6be4..213ecb627a39 100644
@@ -436,6 +436,8 @@ static inline long get_nr_swap_pages(void)
 extern void si_swapinfo(struct sysinfo *);
 extern int pin_hibernation_swap_type(dev_t device, sector_t offset);
 extern void unpin_hibernation_swap_type(int type);
+extern int repin_hibernation_swap_type(int old_type, dev_t device,
+                                       sector_t offset);
 extern int find_hibernation_swap_type(dev_t device, sector_t offset);
 int find_first_swap(dev_t *device);
 extern unsigned int count_swap_pages(int, int);

kernel/power/user.c +5 -12

index d0fcfba7ac23..6e4f40e49319 100644
@@ -218,6 +218,7 @@ static int snapshot_set_swap_area(struct snapshot_data *data,
 {
      sector_t offset;
      dev_t swdev;
+     int new_type;
 
      if (swsusp_swap_in_use())
              return -EPERM;

@@ -238,19 +239,11 @@ static int snapshot_set_swap_area(struct snapshot_data *data,
              offset = swap_area.offset;
      }
 
-     /*
-      * Unpin the swap device if a swap area was already
-      * set by SNAPSHOT_SET_SWAP_AREA.
-      */
-     unpin_hibernation_swap_type(data->swap);
+     new_type = repin_hibernation_swap_type(data->swap, swdev, offset);
+     if (new_type < 0)
+             return new_type;
 
-     /*
-      * User space encodes device types as two-byte values,
-      * so we need to recode them
-      */
-     data->swap = pin_hibernation_swap_type(swdev, offset);
-     if (data->swap < 0)
-             return swdev ? -ENODEV : -EINVAL;
+     data->swap = new_type;
      data->dev = swdev;
      return 0;
 }

mm/swapfile.c +61 -1

index c7e173b93e11..4840fd40f36f 100644
@@ -2219,6 +2219,67 @@ int pin_hibernation_swap_type(dev_t device, sector_t offset)
      return type;
 }
 
+/**
+ * repin_hibernation_swap_type - Atomically replace the hibernation pin
+ * @old_type: Swap type currently pinned (or < 0 if none).
+ * @device:   Block device of the new resume image.
+ * @offset:   Offset identifying the new swap area.
+ *
+ * Look up the swap device for @device/@offset and atomically transfer
+ * the SWP_HIBERNATION pin from @old_type (if valid) to the new device,
+ * all under a single swap_lock critical section. This closes the
+ * swapoff() window that exists when callers unpin and re-pin in two
+ * separate operations.
+ *
+ * If the new device cannot be located, the existing pin on @old_type
+ * is preserved and an error is returned. If @old_type already refers
+ * to the same swap_info_struct as the new lookup, no flag changes are
+ * made and @old_type is returned.
+ *
+ * Return:
+ * >= 0 on success (new swap type).
+ * -EINVAL if @device is invalid.
+ * -ENODEV if the swap device is not found.
+ * -EBUSY  if the new device is already pinned by another context.
+ */
+int repin_hibernation_swap_type(int old_type, dev_t device, sector_t offset)
+{
+     struct swap_info_struct *old_si, *new_si;
+     int new_type;
+
+     spin_lock(&swap_lock);
+
+     new_type = __find_hibernation_swap_type(device, offset);
+     if (new_type < 0) {
+             spin_unlock(&swap_lock);
+             return new_type;
+     }
+
+     new_si = swap_type_to_info(new_type);
+     if (WARN_ON_ONCE(!new_si)) {
+             spin_unlock(&swap_lock);
+             return -ENODEV;
+     }
+
+     old_si = swap_type_to_info(old_type);
+     if (new_si == old_si) {
+             spin_unlock(&swap_lock);
+             return new_type;
+     }
+
+     if (WARN_ON_ONCE(new_si->flags & SWP_HIBERNATION)) {
+             spin_unlock(&swap_lock);
+             return -EBUSY;
+     }
+
+     if (old_si)
+             old_si->flags &= ~SWP_HIBERNATION;
+     new_si->flags |= SWP_HIBERNATION;
+
+     spin_unlock(&swap_lock);
+     return new_type;
+}
+
 /**
  * unpin_hibernation_swap_type - Unpin the swap device for hibernation
  * @type: Swap type previously returned by pin_hibernation_swap_type()
--

Quoted 1 line

2.53.0

I also caught up on this thread late due to travel. Sorry.

Hello David Carlier,

It looks like the same issue and approach was raised previously:
https://lore.kernel.org/all/[email protected]/#t
(+CC DaeMyung)

I had suggested improving the fix by reusing the existing APIs rather than adding a new one. v2 was posted along those lines:
https://lore.kernel.org/all/[email protected]/

To restate the points from that earlier review.

1. I think it is acceptable to release the existing pin when the new set operation fails. there is no significant side effect (see the earlier review for details).

2. If we still want to fix it, reusing the existing APIs seems preferable to adding a new one.

Chris, Andrew - please take the prior thread into account during review.

Thanks,
Youngjun Park

Hi David, Youngjun,

Thanks for the discussion.

This is essentially the same direction as my v1, where I introduced a repin-style helper to transfer the SWP_HIBERNATION pin under a single swap_lock critical section.

After Youngjun's review, I reworked it in v2 to avoid adding a new single-call-site helper and instead reused the existing find/pin/unpin helpers. The current v2 approach keeps the existing pin on failure and avoids introducing a new API.

Given that this is more of a hardening improvement than a strict bug fix, I think the v2 approach is a simpler fit without adding extra API surface.

Happy to adjust if there are specific concerns.

Thanks,
DaeMyung

2026년 5월 2일 (토) 오전 7:00, YoungJun Park <[email protected]>님이 작성:

Quoted 186 lines · David Carlier

Quoted 155 lines

snapshot_set_swap_area() unpins the previously selected swap device and pins the new one in two separate swap_lock critical sections. In the gap between them, swapoff() observes SWP_HIBERNATION cleared, bypasses the guard, and tears down the device, reopening the race the SWP_HIBERNATION pin was meant to close. The window is reachable on any SNAPSHOT_SET_SWAP_AREA call after the snapshot device is opened for hibernation, and on any retry after the resume path's first selection.

Add repin_hibernation_swap_type(), which looks up the new device, clears the old SWP_HIBERNATION flag and sets the new one under a single swap_lock acquisition. The same-device case is short- circuited so userspace can re-select the same swap area without tripping WARN_ON_ONCE and -EBUSY. Switch snapshot_set_swap_area() to the new helper.

A failed lookup now preserves the previous pin instead of dropping it, so a bad SNAPSHOT_SET_SWAP_AREA leaves the prior selection intact. The open and release paths keep using pin_hibernation_swap_type() and unpin_hibernation_swap_type().

The race was identified during AI-assisted review of the SWP_HIBERNATION pinning series.

Fixes 8e6e0d845823 ("mm/swap, PM: hibernate: fix swapoff race in uswsusp by pinning swap device")

Assisted-by Codex (gpt-5-codex)

Signed-off-by David Carlier <[email protected]>

---
 include/linux/swap.h |  2 ++
 kernel/power/user.c  | 17 ++++--------
 mm/swapfile.c        | 61 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 12 deletions(-)

include/linux/swap.h +2 -0

index 1930f81e6be4..213ecb627a39 100644
@@ -436,6 +436,8 @@ static inline long get_nr_swap_pages(void)
 extern void si_swapinfo(struct sysinfo *);
 extern int pin_hibernation_swap_type(dev_t device, sector_t offset);
 extern void unpin_hibernation_swap_type(int type);
+extern int repin_hibernation_swap_type(int old_type, dev_t device,
+                                       sector_t offset);
 extern int find_hibernation_swap_type(dev_t device, sector_t offset);
 int find_first_swap(dev_t *device);
 extern unsigned int count_swap_pages(int, int);

kernel/power/user.c +5 -12

index d0fcfba7ac23..6e4f40e49319 100644
@@ -218,6 +218,7 @@ static int snapshot_set_swap_area(struct snapshot_data *data,
 {
      sector_t offset;
      dev_t swdev;
+     int new_type;
 
      if (swsusp_swap_in_use())
              return -EPERM;

@@ -238,19 +239,11 @@ static int snapshot_set_swap_area(struct snapshot_data *data,
              offset = swap_area.offset;
      }
 
-     /*
-      * Unpin the swap device if a swap area was already
-      * set by SNAPSHOT_SET_SWAP_AREA.
-      */
-     unpin_hibernation_swap_type(data->swap);
+     new_type = repin_hibernation_swap_type(data->swap, swdev, offset);
+     if (new_type < 0)
+             return new_type;
 
-     /*
-      * User space encodes device types as two-byte values,
-      * so we need to recode them
-      */
-     data->swap = pin_hibernation_swap_type(swdev, offset);
-     if (data->swap < 0)
-             return swdev ? -ENODEV : -EINVAL;
+     data->swap = new_type;
      data->dev = swdev;
      return 0;
 }

mm/swapfile.c +61 -1

index c7e173b93e11..4840fd40f36f 100644
@@ -2219,6 +2219,67 @@ int pin_hibernation_swap_type(dev_t device, sector_t offset)
      return type;
 }
 
+/**
+ * repin_hibernation_swap_type - Atomically replace the hibernation pin
+ * @old_type: Swap type currently pinned (or < 0 if none).
+ * @device:   Block device of the new resume image.
+ * @offset:   Offset identifying the new swap area.
+ *
+ * Look up the swap device for @device/@offset and atomically transfer
+ * the SWP_HIBERNATION pin from @old_type (if valid) to the new device,
+ * all under a single swap_lock critical section. This closes the
+ * swapoff() window that exists when callers unpin and re-pin in two
+ * separate operations.
+ *
+ * If the new device cannot be located, the existing pin on @old_type
+ * is preserved and an error is returned. If @old_type already refers
+ * to the same swap_info_struct as the new lookup, no flag changes are
+ * made and @old_type is returned.
+ *
+ * Return:
+ * >= 0 on success (new swap type).
+ * -EINVAL if @device is invalid.
+ * -ENODEV if the swap device is not found.
+ * -EBUSY  if the new device is already pinned by another context.
+ */
+int repin_hibernation_swap_type(int old_type, dev_t device, sector_t offset)
+{
+     struct swap_info_struct *old_si, *new_si;
+     int new_type;
+
+     spin_lock(&swap_lock);
+
+     new_type = __find_hibernation_swap_type(device, offset);
+     if (new_type < 0) {
+             spin_unlock(&swap_lock);
+             return new_type;
+     }
+
+     new_si = swap_type_to_info(new_type);
+     if (WARN_ON_ONCE(!new_si)) {
+             spin_unlock(&swap_lock);
+             return -ENODEV;
+     }
+
+     old_si = swap_type_to_info(old_type);
+     if (new_si == old_si) {
+             spin_unlock(&swap_lock);
+             return new_type;
+     }
+
+     if (WARN_ON_ONCE(new_si->flags & SWP_HIBERNATION)) {
+             spin_unlock(&swap_lock);
+             return -EBUSY;
+     }
+
+     if (old_si)
+             old_si->flags &= ~SWP_HIBERNATION;
+     new_si->flags |= SWP_HIBERNATION;
+
+     spin_unlock(&swap_lock);
+     return new_type;
+}
+
 /**
  * unpin_hibernation_swap_type - Unpin the swap device for hibernation
  * @type: Swap type previously returned by pin_hibernation_swap_type()
--

Quoted 1 line

2.53.0

I also caught up on this thread late due to travel. Sorry.

Hello David Carlier,

It looks like the same issue and approach was raised previously:
https://lore.kernel.org/all/[email protected]/#t
(+CC DaeMyung)

I had suggested improving the fix by reusing the existing APIs rather than adding a new one. v2 was posted along those lines:
https://lore.kernel.org/all/[email protected]/

To restate the points from that earlier review.

1. I think it is acceptable to release the existing pin when the new set operation fails. there is no significant side effect (see the earlier review for details).

2. If we still want to fix it, reusing the existing APIs seems preferable to adding a new one.

Chris, Andrew - please take the prior thread into account during review.

Thanks,
Youngjun Park