[Devel] [PATCH hci-8.1 v2 0/3] dm-ploop: fix flip_upper_deltas panicking the host in its pre-flip check

[Devel] [PATCH hci-8.1 v2 0/3] dm-ploop: fix flip_upper_deltas panicking the host in its pre-flip check

[Denis V. Lunev]

ploop_check_delta_before_flip() runs on every flip_upper_deltas, as
PLOOP_DEBUG is always defined in dm-ploop-cmd.c, and has three bugs
(VSTOR-153445):

- patch 1: when it finds a cluster both deltas map, it returns -EEXIST
  with two md_locks held and interrupts disabled. The next pio on that
  BAT page spins on the lock forever, and the host panics on a hard
  lockup;

- patch 2: a flip after a grow of the device, with the lower delta left
  at its old size, oopses in the walk, which runs past the BAT of the
  lower delta. Refuse it: a flip that got past the walk would damage
  the new top delta on the first BAT writeback after resume;

- patch 3: the walk stops one page early when entry nr_be - 1 of the
  lower delta is the first one of its BAT page, and misses a cluster
  both deltas map there.

The ploop library does not get there. Its reverse merge, the only sender
of the flip, grows the lower delta to the size of the top and, with the
device suspended, clears the clusters both deltas map before it flips,
so VSTOR-153445 has no RK for 8.0. This is the fix the v2 cover letter
of "dm-ploop: fix the pio submission stop around cmds" said would be
sent separately.

Tested on dev-cl04, stock 6.12.0-211.52.1.22.11.vz10, dm-ploop built
out of tree with the series on top of the applied dm-ploop v2:
- a chain whose top delta overwrote a cluster of the base, both deltas
  reloaded read-write, and a direct flip_upper_deltas: the message fails
  with EEXIST, the device keeps working and both images pass ploop
  check;
- an online grow from 1G to 2G, both deltas reloaded read-write, a
  direct flip_upper_deltas and then a write past the old size: with v1
  the flip completes and the images no longer open after unmount, the
  ploop target fails the table with EPROTO. With patch 2 alone and with
  the series the flip fails with EBADSLT, and the data below and past
  the old size survives a remount;
- an online grow, a write past the old size, ploop snapshot-delete of
  the base snapshot (the reverse merge of the library) and another write
  past the old size: the flip goes through, the merged image passes
  ploop check and keeps all writes across a remount;
- the ploop pytest suite, ploop and qcow2 formats, passes.

Changes since v1:
- patch 1: drop the read-only and EACCES argument for the library from
  the message, the library reloads the lower delta read-write before
  the flip;
- new patch 2: refuse the flip when the lower delta has fewer BAT
  entries than the device, outside PLOOP_DEBUG;
- patch 3 (was 2/2): say in the message where the old stop matched and
  what it skipped, split the first paragraph, leave the loop with a
  plain break;
- the grow test writes past the old size after the flip.

v1: http://lore.virtuozzo.com/kernel/20261104214635.2063009-1-den@openvz.org/


Denis V. Lunev (3):
  dm-ploop: fix flip_upper_deltas returning with two md_locks held
  dm-ploop: refuse flip_upper_deltas onto a lower delta smaller than the
    device
  dm-ploop: fix the flip_upper_deltas check skipping the last BAT entry

 drivers/md/dm-ploop-cmd.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)


base-commit: 1f21db982845109040d1feb279e9ee34f8e6f065
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v2 1/3] dm-ploop: fix flip_upper_deltas returning with two md_locks held

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

ploop_check_delta_before_flip() takes the md_lock of a live BAT page
and of the matching page of the lower delta before it compares them.
When it finds a cluster both deltas map, it jumps to unmap past both
unlocks: the cmd returns -EEXIST with the locks held and interrupts
disabled, after freeing the page of the lower delta with its lock still
taken. dmsetup comes back to userspace in atomic context, and the next
pio touching that BAT page spins on its md_lock for good, a hard
lockup.

A flip_upper_deltas message on a chain where the top delta overwrote a
cluster of the lower one gets there. The ploop library does not: its
reverse merge clears those entries in the lower delta with the device
suspended before it flips.

Leave the scan only after both locks are dropped.

Fixes: 0d4b4d0a20a9 ("dm-ploop: introduce per-md page locking")
https://virtuozzo.atlassian.net/browse/VSTOR-153445
Feature: dm-ploop: ploop target driver
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-ploop-cmd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
index 45fa4fe7a45a..d6abbdc3c7e3 100644
--- a/drivers/md/dm-ploop-cmd.c
+++ b/drivers/md/dm-ploop-cmd.c
@@ -1146,11 +1146,13 @@ static int ploop_check_delta_before_flip(struct ploop *ploop, struct file *file)
 			    d_bat_entries[i] != BAT_ENTRY_NONE) {
 				ret = -EEXIST;
 				stop = true;
-				goto unmap;
+				break;
 			}
 		}
 		spin_unlock(&d_md->md_lock);
 		spin_unlock_irq(&md->md_lock);
+		if (stop)
+			break;
 
 		clu = ploop_page_clu_idx_to_bat_clu(md->id, i);
 		if (clu == nr_be - 1) {
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v2 2/3] dm-ploop: refuse flip_upper_deltas onto a lower delta smaller than the device

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

A flip_upper_deltas after a grow of the device, with the lower delta
left at its old size, makes a top delta whose header and BAT cover fewer
entries than the device. After resume the BAT writeback puts the grown
header into page 0 of that file and the BAT pages past its old size onto
its data clusters, and the image no longer opens.

Today ploop_check_delta_before_flip() oopses on such a flip before it
gets that far, as its walk runs past the end of the BAT of the lower
delta. Fixing the walk alone would let the flip through.

Read the header of the lower delta on every flip, not only under
PLOOP_DEBUG, and refuse with -EBADSLT when it has fewer BAT entries than
the device, as ploop_add_delta() refuses a lower delta bigger than the
upper one. Compare entries, not BAT pages: fewer entries in the same
number of pages still gives a stack that ploop_add_delta() refuses on
the next load. The reverse merge of the ploop library grows the lower
delta to the size of the device before it flips, so it never gets the
refusal.

Fixes: 6ddad5a4d226 ("dm-ploop: Add ploop target driver")
https://virtuozzo.atlassian.net/browse/VSTOR-153445
Feature: dm-ploop: ploop target driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-ploop-cmd.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
index d6abbdc3c7e3..5549ba448594 100644
--- a/drivers/md/dm-ploop-cmd.c
+++ b/drivers/md/dm-ploop-cmd.c
@@ -1118,20 +1118,27 @@ static int ploop_set_noresume(struct ploop *ploop, char *mode)
 
 static int ploop_check_delta_before_flip(struct ploop *ploop, struct file *file)
 {
-	int ret = 0;
-#ifdef PLOOP_DEBUG
-	u32 i, end, *d_bat_entries, clu, nr_be;
 	struct rb_root md_root = RB_ROOT;
+	u32 nr_be;
+	int ret;
+#ifdef PLOOP_DEBUG
+	u32 i, end, *d_bat_entries, clu;
 	struct md_page *md, *d_md;
 	struct rb_node *node;
 	bool stop = false;
+#endif
 
 	ret = ploop_read_delta_metadata(ploop, file, &md_root, &nr_be);
 	if (ret) {
 		PL_ERR("Error reading metadata");
-		goto out;
+		return ret;
 	}
 
+	ret = -EBADSLT;
+	if (nr_be < ploop->nr_bat_entries)
+		goto free;
+	ret = 0;
+#ifdef PLOOP_DEBUG
 	/* Points to hdr since md_page[0] also contains hdr. */
 	d_md = ploop_md_first_entry(&md_root);
 
@@ -1164,10 +1171,9 @@ static int ploop_check_delta_before_flip(struct ploop *ploop, struct file *file)
 			break;
 		d_md = ploop_md_next_entry(d_md);
 	}
-
-	ploop_free_md_pages_tree(&md_root);
-out:
 #endif
+free:
+	ploop_free_md_pages_tree(&md_root);
 	return ret;
 }
 
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v2 3/3] dm-ploop: fix the flip_upper_deltas check skipping the last BAT entry

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

ploop_check_delta_before_flip() means to stop its walk after the BAT
page that holds entry nr_be - 1 of the lower delta. It takes the last
entry of the page from i after the scan, when i is already end + 1.

When entry nr_be - 1 is the first one of its page (nr_be = 1009, for
example), the test matches on the full page before it, and the walk
stops one page early: a cluster both deltas map at entry nr_be - 1 gets
past the check. Otherwise the test never matches and the walk ends
with the live BAT, which the previous patch keeps the same size as the
BAT of the lower delta.

Take the last entry of the page from end, and leave the loop with a
plain break.

Fixes: 6ddad5a4d226 ("dm-ploop: Add ploop target driver")
https://virtuozzo.atlassian.net/browse/VSTOR-153445
Feature: dm-ploop: ploop target driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-ploop-cmd.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
index 5549ba448594..2d1fa449b052 100644
--- a/drivers/md/dm-ploop-cmd.c
+++ b/drivers/md/dm-ploop-cmd.c
@@ -1161,13 +1161,8 @@ static int ploop_check_delta_before_flip(struct ploop *ploop, struct file *file)
 		if (stop)
 			break;
 
-		clu = ploop_page_clu_idx_to_bat_clu(md->id, i);
-		if (clu == nr_be - 1) {
-			stop = true;
-			goto unmap;
-		}
-unmap:
-		if (stop)
+		clu = ploop_page_clu_idx_to_bat_clu(md->id, end);
+		if (clu == nr_be - 1)
 			break;
 		d_md = ploop_md_next_entry(d_md);
 	}
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 1/3] dm-ploop: fix flip_upper_deltas returning with two md_locks held

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> ploop_check_delta_before_flip() takes the md_lock of a live BAT page
> and of the matching page of the lower delta before it compares them.
> When it finds a cluster both deltas map, it jumps to unmap past both
> unlocks: the cmd returns -EEXIST with the locks held and interrupts
> disabled, after freeing the page of the lower delta with its lock still
> taken. dmsetup comes back to userspace in atomic context, and the next
> pio touching that BAT page spins on its md_lock for good, a hard
> lockup.
> 
> A flip_upper_deltas message on a chain where the top delta overwrote a
> cluster of the lower one gets there. The ploop library does not: its
> reverse merge clears those entries in the lower delta with the device
> suspended before it flips.
> 
> Leave the scan only after both locks are dropped.
> 
> Fixes: 0d4b4d0a20a9 ("dm-ploop: introduce per-md page locking")
> https://virtuozzo.atlassian.net/browse/VSTOR-153445
> Feature: dm-ploop: ploop target driver
> Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
> Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261105145749.2643035-1-den@openvz.org?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 3/3] dm-ploop: fix the flip_upper_deltas check skipping the last BAT entry

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> ploop_check_delta_before_flip() means to stop its walk after the BAT
> page that holds entry nr_be - 1 of the lower delta. It takes the last
> entry of the page from i after the scan, when i is already end + 1.
> 
> When entry nr_be - 1 is the first one of its page (nr_be = 1009, for
> example), the test matches on the full page before it, and the walk
> stops one page early: a cluster both deltas map at entry nr_be - 1 gets
> past the check. Otherwise the test never matches and the walk ends
> with the live BAT, which the previous patch keeps the same size as the
> BAT of the lower delta.
> 
> Take the last entry of the page from end, and leave the loop with a
> plain break.
> 
> Fixes: 6ddad5a4d226 ("dm-ploop: Add ploop target driver")
> https://virtuozzo.atlassian.net/browse/VSTOR-153445
> Feature: dm-ploop: ploop target driver
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261105145749.2643035-1-den@openvz.org?part=3


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 2/3] dm-ploop: refuse flip_upper_deltas onto a lower delta smaller than the device

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dm-ploop: flip check reads the whole lower BAT, not just the header
--

commit 4effd2e3d6e36d504ad411375021219e27a47440
Author: Denis V. Lunev <den@openvz.org>

dm-ploop: refuse flip_upper_deltas onto a lower delta smaller than the device

This patch makes ploop_check_delta_before_flip() read the lower delta
metadata on every flip, not only under PLOOP_DEBUG. It refuses the flip
with -EBADSLT when the lower delta has fewer BAT entries than the device.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-153445

> diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
> --- a/drivers/md/dm-ploop-cmd.c
> +++ b/drivers/md/dm-ploop-cmd.c

[ ... ]

> @@ -1118,20 +1118,27 @@ static int ploop_set_noresume(struct ploop *ploop, char *mode)
>  static int ploop_check_delta_before_flip(struct ploop *ploop, struct file *file)
>  {
> [ ... ]
>  	ret = ploop_read_delta_metadata(ploop, file, &md_root, &nr_be);

[Severity: Low]
The commit message says "Read the header of the lower delta on every
flip", but is that what this call does?

ploop_check_delta_before_flip() calls ploop_read_delta_metadata(), which
reads page 0 and then allocates and reads the whole BAT. It then runs
ploop_convert_bat_entries() over it.

ploop_convert_bat_entries() can reject a lower delta with a bad BAT, for
example:

    if (READ_ONCE(bat_entries[i]) > max_file_clu)
            ret = -EPROTO;

So a flip can now fail with -EPROTO or -EXDEV, in addition to the new
-EBADSLT.

The commit message does not mention the full-BAT allocation and I/O, or
these extra validation errors. Could the message be reworded to describe
what the code does?

This is a message/code mismatch only. PLOOP_DEBUG appears to be defined
unconditionally in dm-ploop-cmd.c, so the old debug walk likely already
read the full metadata in every build. The added cost and new error modes
are therefore smaller than they first look.

[ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261105145749.2643035-1-den@openvz.org?part=2

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 2/3] dm-ploop: refuse flip_upper_deltas onto a lower delta smaller than the device

[Konstantin Khorenko]

Applying with these fixes on my side:
- paragraphs 1 and 2 of the commit message replaced with:

  A flip_upper_deltas after a grow of the device, with the lower delta
  left at its old size, makes a top delta whose header covers fewer
  entries than the delta below it. Unless a write rewrites BAT page 0
  first, the stack no longer loads. When the grow added BAT clusters, a
  write past the old size can also put a BAT page onto a data cluster of
  that file.

  Today ploop_check_delta_before_flip() oopses on such a flip only when
  the grow added BAT pages, as its walk runs past the end of the BAT of
  the lower delta. A grow within the same BAT pages already gets
  through, and fixing the walk alone would let every such flip through.

- paragraph 3: "Read the header of the lower delta on every flip, not
  only under PLOOP_DEBUG" became "Check the size of the lower delta
  outside PLOOP_DEBUG", and "a stack that ploop_add_delta() refuses on
  the next load" became "a top delta whose header covers fewer entries
  than the delta below it".

On Thu, 05 Nov 2026 15:57:48 +0100, Denis V. Lunev wrote:
> A flip_upper_deltas after a grow of the device, with the lower delta
> left at its old size, makes a top delta whose header and BAT cover fewer
> entries than the device. After resume the BAT writeback puts the grown
> header into page 0 of that file and the BAT pages past its old size onto
> its data clusters, and the image no longer opens.

Only dirty BAT pages are written back, and page 0 gets dirty only from
a write to its own clusters. BAT pages land on data only when the grow
added BAT clusters. In your 1G -> 2G test the new top kept its old
m_Size, and that is the EPROTO you saw. My v1 reply described it
wrongly.

> Today ploop_check_delta_before_flip() oopses on such a flip before it
> gets that far, as its walk runs past the end of the BAT of the lower
> delta. Fixing the walk alone would let the flip through.

The old walk oopses only when the grow added BAT pages; a grow within
the same pages returns 0 and the flip goes through today.

> Read the header of the lower delta on every flip, not only under
> PLOOP_DEBUG, and refuse with -EBADSLT when it has fewer BAT entries than

The call reads and converts the whole BAT, as Sashiko said, and it
already ran on every flip since PLOOP_DEBUG is always defined. Whether
the flipped stack fails with EPROTO or EBADSLT depends on how the table
length is set, so the new text names no errno.

With that fixed:

Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 0/3] dm-ploop: fix flip_upper_deltas panicking the host in its pre-flip check

[Konstantin Khorenko]

Applying with these fixes on my side:
- patch 2: paragraphs 1-3 of the commit message reworded, see the reply
  to patch 2.

On Thu, 05 Nov 2026 15:57:46 +0100, Denis V. Lunev wrote:
> - patch 2: a flip after a grow of the device, with the lower delta left
>   at its old size, oopses in the walk, which runs past the BAT of the
>   lower delta. Refuse it: a flip that got past the walk would damage
>   the new top delta on the first BAT writeback after resume;

The code of all three patches is fine. The damage and the oops in patch
2 happen only in some cases, and most of that wording came from my v1
reply, so I fixed the message myself.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 3/3] dm-ploop: fix the flip_upper_deltas check skipping the last BAT entry

[Konstantin Khorenko]

Applying as is.

On Thu, 05 Nov 2026 15:57:49 +0100, Denis V. Lunev wrote:
> Take the last entry of the page from end, and leave the loop with a
> plain break.

Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 0/3] dm-ploop: fix flip_upper_deltas panicking the host in its pre-flip check

[Konstantin Khorenko]

On Thu, 05 Nov 2026 15:57:46 +0100, Denis V. Lunev wrote:
> dm-ploop: fix flip_upper_deltas panicking the host in its pre-flip check
> 
> ploop_check_delta_before_flip() runs on every flip_upper_deltas, as
> PLOOP_DEBUG is always defined in dm-ploop-cmd.c, and has three bugs
> (VSTOR-153445):
> 
> - patch 1: when it finds a cluster both deltas map, it returns -EEXIST
>   with two md_locks held and interrupts disabled. The next pio on that
>   BAT page spins on the lock forever, and the host panics on a hard
>   lockup;
> 
> [...]

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.19.vz10, thanks!

[1/3] dm-ploop: fix flip_upper_deltas returning with two md_locks held
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/e47cf5cfc4c9
[2/3] dm-ploop: refuse flip_upper_deltas onto a lower delta smaller than the device
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/18c4383f77c9
[3/3] dm-ploop: fix the flip_upper_deltas check skipping the last BAT entry
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/f0cff9cebdb0

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel