Skip to contentSkip to conversation

fixes for iomap_bio_read_folio_range_sync

PATCH In review Christoph Hellwig 4 participants 6 messages Aug 4 – Aug 12

Subsystems (3)
Stable branch VFS Filesystems iomap
lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

Hi all,

two fixes for iomap_bio_read_folio_range_sync, a potential kernel crash when tweaking the device integrity behvavior using sysfs, and a missing bio_uninit that the Sashiko review of the first fix found.

author
#1 lore raw

PATCH 1/2 iomap: don't free integrity payload that doesn't exist

fs_bio_integrity_alloc might not allocate a bio integrity payload if PI verification is disabled on the block device. Check for that case before calling fs_bio_integrity_free in iomap_bio_read_folio_range_sync to avoid a NULL pointer dereferences.

Make the branch cover the PI verification as well - while fs_bio_integrity_verify works without an integrity payload, it requires one to actually do useful work.

Fixes 0b10a370529c ("iomap: support T10 protection information")
Cc <[email protected]> # v7.1
Signed-off-by Christoph Hellwig <[email protected]>
Reviewed-by Anuj Gupta <[email protected]>
Reviewed-by Kanchan Joshi <[email protected]>
Reviewed-by "Darrick J. Wong" <[email protected]>
---
 fs/iomap/bio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/iomap/bio.c +1 -1
index dc8ac7e370a5..30ef78a66b4f 100644
@@ -179,7 +179,7 @@ int iomap_bio_read_folio_range_sync(const struct iomap_iter *iter,
 	if (srcmap->flags & IOMAP_F_INTEGRITY)
 		fs_bio_integrity_alloc(&bio);
 	error = submit_bio_wait(&bio);
-	if (srcmap->flags & IOMAP_F_INTEGRITY) {
+	if (bio_integrity(&bio)) {
 		if (!error)
 			error = fs_bio_integrity_verify(&bio, sector, len);
 		fs_bio_integrity_free(&bio);
Signature · 1 line
2.53.0
author
#2 lore raw

PATCH 2/2 iomap: iomap_bio_read_folio_range_sync is missing a call to bio_uninit

In reply to Christoph Hellwig

Which could leak blkg references.

Fixes c03cea42149d ("iomap: add initial support for writes without buffer heads")
Signed-off-by Christoph Hellwig <[email protected]>
---
 fs/iomap/bio.c | 1 +
 1 file changed, 1 insertion(+)
fs/iomap/bio.c +1 -0
index 30ef78a66b4f..48100c614431 100644
@@ -184,5 +184,6 @@ int iomap_bio_read_folio_range_sync(const struct iomap_iter *iter,
 			error = fs_bio_integrity_verify(&bio, sector, len);
 		fs_bio_integrity_free(&bio);
 	}
+	bio_uninit(&bio);
 	return error;
 }
Signature · 1 line
2.53.0
Reviewed-by on v1 2/2
#3 lore raw

Re: [PATCH 2/2] iomap: iomap_bio_read_folio_range_sync is missing a call to bio_uninit

Quoted 4 lines · Christoph Hellwig

Which could leak blkg references.

Fixes c03cea42149d ("iomap: add initial support for writes without buffer heads")
Signed-off-by Christoph Hellwig <[email protected]>

Makes sense that we have to uninit that which we initted earlier;

Reviewed-by "Darrick J. Wong" <[email protected]>

--D

Quoted 19 lines
---
 fs/iomap/bio.c | 1 +
 1 file changed, 1 insertion(+)
fs/iomap/bio.c +1 -0
index 30ef78a66b4f..48100c614431 100644
@@ -184,5 +184,6 @@ int iomap_bio_read_folio_range_sync(const struct iomap_iter *iter,
 			error = fs_bio_integrity_verify(&bio, sector, len);
 		fs_bio_integrity_free(&bio);
 	}
+	bio_uninit(&bio);
 	return error;
 }
Signature · 1 line
2.53.0
maintainer Applied
#5 lore raw
In reply to Christoph Hellwig
Quoted 10 lines · Christoph Hellwig

fixes for iomap_bio_read_folio_range_sync

Hi all,

two fixes for iomap_bio_read_folio_range_sync, a potential kernel crash when tweaking the device integrity behvavior using sysfs, and a missing bio_uninit that the Sashiko review of the first fix found.

[...]

Since the merge window is imminent I'm routing this through the iomap branch and it's not really a fix for something in this merge window.

---

Applied to the vfs-7.3.iomap branch of the vfs/vfs.git tree. Patches in the vfs-7.3.iomap branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.3.iomap

[1/2] iomap: don't free integrity payload that doesn't exist

      https://git.kernel.org/vfs/vfs/c/8a8685b32c07
[2/2] iomap: iomap_bio_read_folio_range_sync is missing a call to bio_uninit
      https://git.kernel.org/vfs/vfs/c/accb6624e383