Re: [PATCH] dm dust: use target-relative sectors in badblock messages

[Benjamin Marzinski <bmarzins@redhat.com>]

On Tue, Jun 09, 2026 at 12:23:45AM +0000, Samuel Moelius wrote:
> dm-dust stores bad blocks in target-relative sectors, but some messages
> report or compare values in a different coordinate space. A table with a
> non-zero target offset can therefore describe one sector while reporting
> another.
> 
> This makes diagnostics misleading and can make scripted validation look
> at the wrong logical block.
> 
> Keep the badblock control path in target-relative coordinates when
> reporting and matching stored bad blocks.
> 
> Assisted-by: Codex:gpt-5.5-cyber-preview
> Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>

I agree that the block number is misleading to the point of being wrong.
However I don't think that dm-dust is inconsistent. dust_message()
checks if the bad block fits in the underlying device size. It doesn't
check if it fits in the target size. When dust_map() is called, it
checks for the block relative to the underlying device. So it
consitently treats the block number as referring to a block on the
underlying device, not referring to a block relative to the start of the
dm-dust target. This is weird. It means that dm-dust allows you to mark
bad blocks that aren't actually part of the dm-dust device, or mark bad
blocks that aren't aligned with the dm-dust target. That seems wrong,
and the code setting up max_block_sectors sure makes it look like the
blocks are supposed to be relative to the target.  The question is, "Do
existing dm-dust users expect this current behavior?"
Documentation/admin-guide/device-mapper/dm-dust.rst doesn't mention what
a bad block is relative to.

Overall, I think that dm-dust is not a heavily used target, and quite
possibly you are the first person to try using it will a non-zero offset
on the underlying device, so I think it's probably o.k. to change the
behavior here to something sensible. But the commit message should be
clearer that it is changing what a badblock refers to (from a block on
the underlying device to a block relative to the start of the dm-dust
target).

Also if you wouldn't mind removing the sector_div() line from
__dust_map_write(), that would also be a welcome change. thisblock
already points to the block, and dividing it by sect_per_block again
just makes __dust_map_write() report the wrong block got removed from
the badblocklist.

-Ben

> ---
>  drivers/md/dm-dust.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c
> index c7e3077fb1f5..45d4154209b7 100644
> --- a/drivers/md/dm-dust.c
> +++ b/drivers/md/dm-dust.c
> @@ -224,15 +224,16 @@ static int dust_map_write(struct dust_device *dd, sector_t thisblock,
>  static int dust_map(struct dm_target *ti, struct bio *bio)
>  {
>  	struct dust_device *dd = ti->private;
> +	sector_t dust_sector = dm_target_offset(ti, bio->bi_iter.bi_sector);
>  	int r;
>  
>  	bio_set_dev(bio, dd->dev->bdev);
> -	bio->bi_iter.bi_sector = dd->start + dm_target_offset(ti, bio->bi_iter.bi_sector);
> +	bio->bi_iter.bi_sector = dd->start + dust_sector;
>  
>  	if (bio_data_dir(bio) == READ)
> -		r = dust_map_read(dd, bio->bi_iter.bi_sector, dd->fail_read_on_bb);
> +		r = dust_map_read(dd, dust_sector, dd->fail_read_on_bb);
>  	else
> -		r = dust_map_write(dd, bio->bi_iter.bi_sector, dd->fail_read_on_bb);
> +		r = dust_map_write(dd, dust_sector, dd->fail_read_on_bb);
>  
>  	return r;
>  }
> @@ -415,7 +416,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv,
>  			char *result, unsigned int maxlen)
>  {
>  	struct dust_device *dd = ti->private;
> -	sector_t size = bdev_nr_sectors(dd->dev->bdev);
> +	sector_t size = dm_sector_div_up(ti->len, dd->sect_per_block);
>  	bool invalid_msg = false;
>  	int r = -EINVAL;
>  	unsigned long long tmp, block;
> @@ -462,8 +463,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv,
>  			return r;
>  
>  		block = tmp;
> -		sector_div(size, dd->sect_per_block);
> -		if (block > size) {
> +		if (block >= size) {
>  			DMERR("selected block value out of range");
>  			return r;
>  		}
> @@ -490,8 +490,7 @@ static int dust_message(struct dm_target *ti, unsigned int argc, char **argv,
>  			return r;
>  		}
>  		wr_fail_cnt = tmp_ui;
> -		sector_div(size, dd->sect_per_block);
> -		if (block > size) {
> +		if (block >= size) {
>  			DMERR("selected block value out of range");
>  			return r;
>  		}
> -- 
> 2.43.0