[PATCH] Revert "null_blk: allow byte aligned memory offsets"

[PATCH] Revert "null_blk: allow byte aligned memory offsets"

[Bart Van Assche]

This reverts commit 3451cf34f51bb70c24413abb20b423e64486161b and fixes
the following KASAN complaint when running test zbd/013:

BUG: KASAN: slab-use-after-free in null_handle_data_transfer+0x88c/0xe50 [null_blk]
Write of size 4096 at addr ffff8881ab162000 by task (udev-worker)/78072

CPU: 8 UID: 0 PID: 78072 Comm: (udev-worker) Not tainted 6.18.0-rc5-dbg #14 PREEMPT  737e33391e24fa2fcd9958673f6992b5ee131a07
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Call Trace:
 <TASK>
 show_stack+0x4d/0x60
 dump_stack_lvl+0x61/0x80
 print_address_description.constprop.0+0x8b/0x310
 print_report+0xfd/0x1d7
 kasan_report+0xde/0x1c0
 kasan_check_range+0x10c/0x1f0
 __asan_memcpy+0x3f/0x70
 null_handle_data_transfer+0x88c/0xe50 [null_blk]
 null_process_cmd+0x1a4/0x370 [null_blk]
 null_process_zoned_cmd+0x1ff/0x3c0 [null_blk]
 null_handle_cmd+0x1bd/0x580 [null_blk]
 null_queue_rq+0x568/0x970 [null_blk]
 null_queue_rqs+0xe5/0x2b0 [null_blk]
 __blk_mq_flush_list+0x83/0xb0
 blk_mq_dispatch_queue_requests+0x3d7/0x660
 blk_mq_flush_plug_list+0x1a1/0x730
 __blk_flush_plug+0x290/0x540
 blk_finish_plug+0x53/0xc0
 read_pages+0x456/0xad0
 page_cache_ra_unbounded+0x3cd/0x6e0
 force_page_cache_ra+0x1f0/0x370
 page_cache_sync_ra+0x158/0x870
 filemap_get_pages+0x327/0xcb0
 filemap_read+0x336/0xd30
 blkdev_read_iter+0x15c/0x430
 vfs_read+0x79a/0x1150
 ksys_read+0xfd/0x230
 __x64_sys_read+0x76/0xc0
 x64_sys_call+0x143c/0x17e0
 do_syscall_64+0x96/0x360
 entry_SYSCALL_64_after_hwframe+0x4b/0x53
 </TASK>

Allocated by task 0 on cpu 0 at 3226.274686s:
 kasan_save_stack+0x2a/0x50
 kasan_save_track+0x1c/0x70
 kasan_save_alloc_info+0x3d/0x50
 __kasan_kmalloc+0xa0/0xb0
 __kmalloc_cache_noprof+0x2e9/0x8a0
 kmem_cache_free+0x590/0x870
 mempool_free_slab+0x1b/0x20
 mempool_free+0xd1/0x9b0
 bio_free+0x15e/0x1c0
 bio_put+0x34f/0x790
 bio_endio+0x31d/0x6c0
 blk_update_request+0x425/0xfb0
 blk_mq_end_request+0x5d/0x370
 null_cmd_timer_expired+0x43/0x60 [null_blk]
 __hrtimer_run_queues+0x53e/0xb40
 hrtimer_interrupt+0x32f/0x850
 __sysvec_apic_timer_interrupt+0xdc/0x360
 sysvec_apic_timer_interrupt+0xa4/0xe0
 asm_sysvec_apic_timer_interrupt+0x1f/0x30

Freed by task 14 on cpu 0 at 3226.398721s:
 kasan_save_stack+0x2a/0x50
 kasan_save_track+0x1c/0x70
 __kasan_save_free_info+0x3f/0x60
 __kasan_slab_free+0x67/0x80
 kfree+0x170/0x780
 slab_free_after_rcu_debug+0x6c/0x250
 rcu_do_batch+0x369/0x13f0
 rcu_core+0x385/0x5a0
 rcu_core_si+0x12/0x20
 handle_softirqs+0x1a3/0x930
 run_ksoftirqd+0x3e/0x60
 smpboot_thread_fn+0x311/0xa00
 kthread+0x3cc/0x830
 ret_from_fork+0x39c/0x500
 ret_from_fork_asm+0x11/0x20

Last potentially related work creation:
 kasan_save_stack+0x2a/0x50
 kasan_record_aux_stack+0xad/0xc0
 __call_rcu_common.constprop.0+0xfb/0xbb0
 call_rcu+0x12/0x20
 kmem_cache_free+0x5bc/0x870
 mempool_free_slab+0x1b/0x20
 mempool_free+0xd1/0x9b0
 bio_free+0x15e/0x1c0
 bio_put+0x34f/0x790
 bio_endio+0x31d/0x6c0
 blk_update_request+0x425/0xfb0
 blk_mq_end_request+0x5d/0x370
 null_cmd_timer_expired+0x43/0x60 [null_blk]
 __hrtimer_run_queues+0x53e/0xb40
 hrtimer_interrupt+0x32f/0x850
 __sysvec_apic_timer_interrupt+0xdc/0x360
 sysvec_apic_timer_interrupt+0xa4/0xe0
 asm_sysvec_apic_timer_interrupt+0x1f/0x30

The buggy address belongs to the object at ffff8881ab162000
 which belongs to the cache kmalloc-32 of size 32
The buggy address is located 0 bytes inside of
 freed 32-byte region [ffff8881ab162000, ffff8881ab162020)

Cc: Keith Busch <kbusch@kernel.org>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/block/null_blk/main.c  | 45 ++++++++++++++++------------------
 drivers/block/null_blk/zoned.c |  2 +-
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index f1e67962ecae..ea3fc4241f82 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1130,27 +1130,25 @@ static int null_make_cache_space(struct nullb *nullb, unsigned long n)
 }
 
 static blk_status_t copy_to_nullb(struct nullb *nullb, void *source,
-				  loff_t pos, size_t n, bool is_fua)
+				  sector_t sector, size_t n, bool is_fua)
 {
 	size_t temp, count = 0;
+	unsigned int offset;
 	struct nullb_page *t_page;
-	sector_t sector;
 
 	while (count < n) {
-		temp = min3(nullb->dev->blocksize, n - count,
-			    PAGE_SIZE - offset_in_page(pos));
-		sector = pos >> SECTOR_SHIFT;
+		temp = min_t(size_t, nullb->dev->blocksize, n - count);
 
 		if (null_cache_active(nullb) && !is_fua)
 			null_make_cache_space(nullb, PAGE_SIZE);
 
+		offset = (sector & SECTOR_MASK) << SECTOR_SHIFT;
 		t_page = null_insert_page(nullb, sector,
 			!null_cache_active(nullb) || is_fua);
 		if (!t_page)
 			return BLK_STS_NOSPC;
 
-		memcpy_to_page(t_page->page, offset_in_page(pos),
-			       source + count, temp);
+		memcpy_to_page(t_page->page, offset, source + count, temp);
 
 		__set_bit(sector & SECTOR_MASK, t_page->bitmap);
 
@@ -1158,33 +1156,33 @@ static blk_status_t copy_to_nullb(struct nullb *nullb, void *source,
 			null_free_sector(nullb, sector, true);
 
 		count += temp;
-		pos += temp;
+		sector += temp >> SECTOR_SHIFT;
 	}
 	return BLK_STS_OK;
 }
 
-static void copy_from_nullb(struct nullb *nullb, void *dest, loff_t pos,
+static void copy_from_nullb(struct nullb *nullb, void *dest, sector_t sector,
 			    size_t n)
 {
 	size_t temp, count = 0;
+	unsigned int offset;
 	struct nullb_page *t_page;
-	sector_t sector;
 
 	while (count < n) {
-		temp = min3(nullb->dev->blocksize, n - count,
-			    PAGE_SIZE - offset_in_page(pos));
-		sector = pos >> SECTOR_SHIFT;
+		temp = min_t(size_t, nullb->dev->blocksize, n - count);
 
+		offset = (sector & SECTOR_MASK) << SECTOR_SHIFT;
 		t_page = null_lookup_page(nullb, sector, false,
 			!null_cache_active(nullb));
+
 		if (t_page)
-			memcpy_from_page(dest + count, t_page->page,
-					 offset_in_page(pos), temp);
+			memcpy_from_page(dest + count, t_page->page, offset,
+					 temp);
 		else
 			memset(dest + count, 0, temp);
 
 		count += temp;
-		pos += temp;
+		sector += temp >> SECTOR_SHIFT;
 	}
 }
 
@@ -1230,7 +1228,7 @@ static blk_status_t null_handle_flush(struct nullb *nullb)
 }
 
 static blk_status_t null_transfer(struct nullb *nullb, struct page *page,
-	unsigned int len, unsigned int off, bool is_write, loff_t pos,
+	unsigned int len, unsigned int off, bool is_write, sector_t sector,
 	bool is_fua)
 {
 	struct nullb_device *dev = nullb->dev;
@@ -1242,10 +1240,10 @@ static blk_status_t null_transfer(struct nullb *nullb, struct page *page,
 	if (!is_write) {
 		if (dev->zoned)
 			valid_len = null_zone_valid_read_len(nullb,
-				pos >> SECTOR_SHIFT, len);
+				sector, len);
 
 		if (valid_len) {
-			copy_from_nullb(nullb, p, pos, valid_len);
+			copy_from_nullb(nullb, p, sector, valid_len);
 			off += valid_len;
 			len -= valid_len;
 		}
@@ -1255,7 +1253,7 @@ static blk_status_t null_transfer(struct nullb *nullb, struct page *page,
 		flush_dcache_page(page);
 	} else {
 		flush_dcache_page(page);
-		err = copy_to_nullb(nullb, p, pos, len, is_fua);
+		err = copy_to_nullb(nullb, p, sector, len, is_fua);
 	}
 
 	kunmap_local(p);
@@ -1273,7 +1271,7 @@ static blk_status_t null_handle_data_transfer(struct nullb_cmd *cmd,
 	struct nullb *nullb = cmd->nq->dev->nullb;
 	blk_status_t err = BLK_STS_OK;
 	unsigned int len;
-	loff_t pos = blk_rq_pos(rq) << SECTOR_SHIFT;
+	sector_t sector = blk_rq_pos(rq);
 	unsigned int max_bytes = nr_sectors << SECTOR_SHIFT;
 	unsigned int transferred_bytes = 0;
 	struct req_iterator iter;
@@ -1285,11 +1283,11 @@ static blk_status_t null_handle_data_transfer(struct nullb_cmd *cmd,
 		if (transferred_bytes + len > max_bytes)
 			len = max_bytes - transferred_bytes;
 		err = null_transfer(nullb, bvec.bv_page, len, bvec.bv_offset,
-				     op_is_write(req_op(rq)), pos,
+				     op_is_write(req_op(rq)), sector,
 				     rq->cmd_flags & REQ_FUA);
 		if (err)
 			break;
-		pos += len;
+		sector += len >> SECTOR_SHIFT;
 		transferred_bytes += len;
 		if (transferred_bytes >= max_bytes)
 			break;
@@ -1946,7 +1944,6 @@ static int null_add_dev(struct nullb_device *dev)
 		.logical_block_size	= dev->blocksize,
 		.physical_block_size	= dev->blocksize,
 		.max_hw_sectors		= dev->max_sectors,
-		.dma_alignment		= 1,
 	};
 
 	struct nullb *nullb;
diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index dbf292a8eae9..6a93b12a06ff 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -242,7 +242,7 @@ size_t null_zone_valid_read_len(struct nullb *nullb,
 {
 	struct nullb_device *dev = nullb->dev;
 	struct nullb_zone *zone = &dev->zones[null_zone_no(dev, sector)];
-	unsigned int nr_sectors = DIV_ROUND_UP(len, SECTOR_SHIFT);
+	unsigned int nr_sectors = len >> SECTOR_SHIFT;
 
 	/* Read must be below the write pointer position */
 	if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL ||

Re: [PATCH] Revert "null_blk: allow byte aligned memory offsets"

[Chaitanya Kulkarni]

On 11/11/25 14:08, Bart Van Assche wrote:
> This reverts commit 3451cf34f51bb70c24413abb20b423e64486161b and fixes
> the following KASAN complaint when running test zbd/013:
>

My blktests automation didn't caught this, need to fix that.

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH] Revert "null_blk: allow byte aligned memory offsets"

[Keith Busch]

On Tue, Nov 11, 2025 at 02:08:33PM -0800, Bart Van Assche wrote:
> This reverts commit 3451cf34f51bb70c24413abb20b423e64486161b and fixes
> the following KASAN complaint when running test zbd/013

Can I please just get one chance to fix it before a kneejerk revert??

This was in v1, it was mistakenly dropped from v2.

---
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index f1e67962ecaeb..55c3f4d62c2e3 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1240,9 +1240,12 @@ static blk_status_t null_transfer(struct nullb *nullb, struct page *page,

        p = kmap_local_page(page) + off;
        if (!is_write) {
-               if (dev->zoned)
+               if (dev->zoned) {
                        valid_len = null_zone_valid_read_len(nullb,
                                pos >> SECTOR_SHIFT, len);
+                       if (valid_len != len)
+                               valid_len -= (pos & (SECTOR_SIZE - 1));
+               }

                if (valid_len) {
                        copy_from_nullb(nullb, p, pos, valid_len);
--

Re: [PATCH] Revert "null_blk: allow byte aligned memory offsets"

[Chaitanya Kulkarni]

On 11/11/25 18:02, Keith Busch wrote:
> On Tue, Nov 11, 2025 at 02:08:33PM -0800, Bart Van Assche wrote:
>> This reverts commit 3451cf34f51bb70c24413abb20b423e64486161b and fixes
>> the following KASAN complaint when running test zbd/013
> Can I please just get one chance to fix it before a kneejerk revert??
>
> This was in v1, it was mistakenly dropped from v2.
>
> ---
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index f1e67962ecaeb..55c3f4d62c2e3 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -1240,9 +1240,12 @@ static blk_status_t null_transfer(struct nullb *nullb, struct page *page,
>
>          p = kmap_local_page(page) + off;
>          if (!is_write) {
> -               if (dev->zoned)
> +               if (dev->zoned) {
>                          valid_len = null_zone_valid_read_len(nullb,
>                                  pos >> SECTOR_SHIFT, len);
> +                       if (valid_len != len)
> +                               valid_len -= (pos & (SECTOR_SIZE - 1));
> +               }
>
>                  if (valid_len) {
>                          copy_from_nullb(nullb, p, pos, valid_len);
> --


+1 on fix before revert,

-ck

Re: [PATCH] Revert "null_blk: allow byte aligned memory offsets"

[Jens Axboe]

On 11/11/25 7:02 PM, Keith Busch wrote:
> On Tue, Nov 11, 2025 at 02:08:33PM -0800, Bart Van Assche wrote:
>> This reverts commit 3451cf34f51bb70c24413abb20b423e64486161b and fixes
>> the following KASAN complaint when running test zbd/013
> 
> Can I please just get one chance to fix it before a kneejerk revert??

Yes let's please just fix the issue, this seems overly eager for no
reason at all rather than to appease a test case.

-- 
Jens Axboe

Re: [PATCH] Revert "null_blk: allow byte aligned memory offsets"

[Bart Van Assche]

On 11/11/25 6:02 PM, Keith Busch wrote:
> On Tue, Nov 11, 2025 at 02:08:33PM -0800, Bart Van Assche wrote:
>> This reverts commit 3451cf34f51bb70c24413abb20b423e64486161b and fixes
>> the following KASAN complaint when running test zbd/013
> 
> Can I please just get one chance to fix it before a kneejerk revert??

Of course. I only posted a revert because I knew that Jens wouldn't
apply it immediately and would give you the chance to come up with a
fix.

Thanks,

Bart.