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

[no-reply at sashiko.virtuozzo.com]

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