The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] dm dust: use target-relative sectors in badblock messages
@ 2026-06-09  0:23 Samuel Moelius
  2026-07-22 21:14 ` Benjamin Marzinski
  0 siblings, 1 reply; 2+ messages in thread
From: Samuel Moelius @ 2026-06-09  0:23 UTC (permalink / raw)
  To: Alasdair Kergon
  Cc: Samuel Moelius, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski,
	open list:DEVICE-MAPPER  (LVM), open list

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] dm dust: use target-relative sectors in badblock messages
  2026-06-09  0:23 [PATCH] dm dust: use target-relative sectors in badblock messages Samuel Moelius
@ 2026-07-22 21:14 ` Benjamin Marzinski
  0 siblings, 0 replies; 2+ messages in thread
From: Benjamin Marzinski @ 2026-07-22 21:14 UTC (permalink / raw)
  To: Samuel Moelius
  Cc: Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
	open list:DEVICE-MAPPER  (LVM), open list

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-22 21:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-09  0:23 [PATCH] dm dust: use target-relative sectors in badblock messages Samuel Moelius
2026-07-22 21:14 ` Benjamin Marzinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox