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

[Devel] [PATCH hci-8.1 0/2] 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 both of its ways
out of the walk are broken (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: the walk never stops at the end of the BAT of the lower
  delta, so once the device has grown past it the flip oopses on a NULL
  md page.

Neither is reached through the ploop library. Its reverse merge, the
only sender of the flip, grows the lower delta to the size of the top
and clears the clusters both deltas map before it flips, and a device it
mounted has the lower delta read-only, which the flip refuses with
EACCES before the check. Both take a flip of a table built with the
lower delta read-write, 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 clusters of the base and a direct
  flip_upper_deltas: the stock module returns to dmsetup in atomic
  context ("BUG: scheduling while atomic"), and a read of the device
  then panics the host with "Hard LOCKUP". With the series the message
  fails with EEXIST and the device keeps working;
- set_noresume, suspend, a grow to 2T and flip_upper_deltas: the stock
  module oopses at ploop_check_delta_before_flip+0x86, with the series
  the flip completes and the images keep their data;
- the ploop pytest suite, ploop format, passes.

Denis V. Lunev (2):
  dm-ploop: fix flip_upper_deltas returning with two md_locks held
  dm-ploop: fix flip_upper_deltas walking past the BAT of the lower
    delta

 drivers/md/dm-ploop-cmd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 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 1/2] 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, with both deltas opened read-write, 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 before it flips, and a device it mounted has
the lower delta read-only, which the flip refuses before the check.

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
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

Re: [Devel] [PATCH hci-8.1 1/2] 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, with both deltas opened read-write, 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 before it flips, and a device it mounted has
> the lower delta read-only, which the flip refuses before the check.
> 
> 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
> 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/20261104214635.2063009-1-den@openvz.org?part=1


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

[Devel] [PATCH hci-8.1 2/2] dm-ploop: fix flip_upper_deltas walking past the BAT of the lower delta

[Denis V. Lunev]

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

ploop_check_delta_before_flip() walks the live BAT pages together with
the BAT pages of the lower delta, and means to stop at the page that
holds the last entry of the lower delta. It takes that entry from i
after the scan of the page, when i is already one past end, so the stop
never matches. Once the live BAT has more pages than the lower delta, as
after a grow of the device, the walk runs off the md tree of the lower
delta and dereferences a NULL page: the flip oopses. The reverse merge
of the ploop library grows the lower delta to the size of the top before
it flips, so only a flip outside the library gets there.

Take the last entry of the page from end.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
index d6abbdc3c7e3..74674011a888 100644
--- a/drivers/md/dm-ploop-cmd.c
+++ b/drivers/md/dm-ploop-cmd.c
@@ -1154,7 +1154,7 @@ 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);
+		clu = ploop_page_clu_idx_to_bat_clu(md->id, end);
 		if (clu == nr_be - 1) {
 			stop = true;
 			goto unmap;
-- 
2.53.0

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

Re: [Devel] [PATCH hci-8.1 2/2] dm-ploop: fix flip_upper_deltas walking past the BAT of the lower delta

[no-reply]

> From: Denis V. Lunev <den@openvz.org>
> 
> ploop_check_delta_before_flip() walks the live BAT pages together with
> the BAT pages of the lower delta, and means to stop at the page that
> holds the last entry of the lower delta. It takes that entry from i
> after the scan of the page, when i is already one past end, so the stop
> never matches. Once the live BAT has more pages than the lower delta, as
> after a grow of the device, the walk runs off the md tree of the lower
> delta and dereferences a NULL page: the flip oopses. The reverse merge
> of the ploop library grows the lower delta to the size of the top before
> it flips, so only a flip outside the library gets there.
> 
> Take the last entry of the page from end.
> 
> 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/20261104214635.2063009-1-den@openvz.org?part=2


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