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

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

[Samuel Moelius]

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

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

[Benjamin Marzinski]

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

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

[Samuel Moelius]

On Wed, Jul 22, 2026 at 5:14 PM Benjamin Marzinski <bmarzins@redhat.com> wrote:
>
> 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.

Sorry for the delay. I just sent an updated patch.