Re: [PATCH v4 3/5] bio-integrity-fs: use integrity interval instead of sector as seed

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional dereference of `bip` and `bi` in `fs_bio_integrity_verify` and `fs_bio_integrity_free` causing a kernel panic.
--

commit 1da3b7d905f7d5c0c600e7d3a5fbf3cb0dc853c7
Author: Caleb Sander Mateos <csander@purestorage.com>

bio-integrity-fs: use integrity interval instead of sector as seed
    
bip_iter.bi_sector is meant to be in units of integrity intervals rather
than 512-byte sectors. bio_integrity_verify() doesn't actually use it
currently (it uses the passed in struct bvec_iter's bi_sector instead).
But let's set it to the expected value for consistency.

> diff --git a/block/bio-integrity-fs.c b/block/bio-integrity-fs.c
> index 9c5fe5fa8f0da..770eacb2220ff 100644
> --- a/block/bio-integrity-fs.c
> +++ b/block/bio-integrity-fs.c
> @@ -67,7 +67,7 @@ int fs_bio_integrity_verify(struct bio *bio, sector_t sector, unsigned int size)
>  	 * bio.  Requires the submitter to remember the sector and the size.
>  	 */
>  	memset(&bip->bip_iter, 0, sizeof(bip->bip_iter));
> -	bip->bip_iter.bi_sector = sector;
> +	bip->bip_iter.bi_sector = bio_integrity_intervals(bi, sector);

[Severity: High]
This is a pre-existing issue, but can this unconditionally dereference bip
and bi?

Looking at iomap_bio_read_folio_range_sync() in fs/iomap/bio.c, it calls
fs_bio_integrity_verify() and fs_bio_integrity_free() without validating if
the integrity payload was actually allocated:

fs/iomap/bio.c:iomap_bio_read_folio_range_sync() {
    ...
    if (!error)
        error = fs_bio_integrity_verify(&bio, sector, len);
    fs_bio_integrity_free(&bio);
    ...
}

If fs_bio_integrity_alloc() returns 0 without actually allocating a
bio_integrity_payload, bio_integrity(bio) will be NULL. This can happen if
the device's integrity profile is unregistered concurrently, or if the
operation doesn't require integrity checks.

When that happens, fs_bio_integrity_verify() would dereference a NULL bip
during the memset(&bip->bip_iter, 0, ...) call, and a NULL bi at the call to
bio_integrity_intervals(bi, sector). Similarly, fs_bio_integrity_free() would
subsequently dereference bip on the same path.

Could we add a check for a valid bip and bi before these accesses?

>  	bip->bip_iter.bi_size = bio_integrity_bytes(bi, size >> SECTOR_SHIFT);
>  	return blk_status_to_errno(bio_integrity_verify(bio, &data_iter));
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260627054220.2174166-1-csander@purestorage.com?part=3