Skip to contentSkip to conversation

fsverity: stop using PG_error to track error status

PATCH New Eric Biggers 1 participant 1 message Nov 29, 2022

Subsystems (5)
VFS ext4 file system F2FS file system fscrypt: file system level encryption support +1 more subsystems
lore
Share & tools
lore

Eric Biggers

author maintainer
#0 lore raw

PATCH v5 1/1 fsverity: stop using PG_error to track error status

From: Eric Biggers <[email protected]>

As a step towards freeing the PG_error flag for other uses, change ext4 and f2fs to stop using PG_error to track verity errors. Instead, if a verity error occurs, just mark the whole bio as failed. The coarser granularity isn't really a problem since it isn't any worse than what the block layer provides, and errors from a multi-page readahead aren't reported to applications unless a single-page read fails too.

f2fs supports compression, which makes the f2fs changes a bit more complicated than desired, but the basic premise still works.

Note: there are still a few uses of PageError in f2fs, but they are on the write path, so they are unrelated and this patch doesn't touch them.

Reviewed-by Chao Yu <[email protected]>
Acked-by Jaegeuk Kim <[email protected]>
Signed-off-by Eric Biggers <[email protected]>

---

v5: Fixed NULL dereference when reading compressed+verity file on f2fs.

v4: Added a comment for decompression_attempted, added a paragraph to
the commit message, and added Chao's Reviewed-by.

v3: made a small simplification to the f2fs changes. Also dropped the
fscrypt patch since it is upstream now.

 fs/ext4/readpage.c |  8 ++----
 fs/f2fs/compress.c | 64 ++++++++++++++++++++++------------------------
 fs/f2fs/data.c     | 53 +++++++++++++++++++++++---------------
 fs/verity/verify.c | 12 ++++-----
 4 files changed, 72 insertions(+), 65 deletions(-)
fs/ext4/readpage.c +2 -6
index 3d21eae267fca..e604ea4e102b7 100644
@@ -75,14 +75,10 @@ static void __read_end_io(struct bio *bio)
 	bio_for_each_segment_all(bv, bio, iter_all) {
 		page = bv->bv_page;
 
-		/* PG_error was set if verity failed. */
-		if (bio->bi_status || PageError(page)) {
+		if (bio->bi_status)
 			ClearPageUptodate(page);
-			/* will re-read again later */
-			ClearPageError(page);
-		} else {
+		else
 			SetPageUptodate(page);
-		}
 		unlock_page(page);
 	}
 	if (bio->bi_private)
fs/f2fs/compress.c +31 -33

96 lines not on this page · raw message

fs/f2fs/data.c +33 -20

131 lines not on this page · raw message

fs/verity/verify.c +6 -6
index bde8c9b7d25f6..961ba248021f9 100644
@@ -200,9 +200,8 @@ EXPORT_SYMBOL_GPL(fsverity_verify_page);
  * @bio: the bio to verify
  *
  * Verify a set of pages that have just been read from a verity file.  The pages
- * must be pagecache pages that are still locked and not yet uptodate.  Pages
- * that fail verification are set to the Error state.  Verification is skipped
- * for pages already in the Error state, e.g. due to fscrypt decryption failure.
+ * must be pagecache pages that are still locked and not yet uptodate.  If a
+ * page fails verification, then bio->bi_status is set to an error status.
  *
  * This is a helper function for use by the ->readahead() method of filesystems
  * that issue bios to read data directly into the page cache.  Filesystems that

@@ -244,9 +243,10 @@ void fsverity_verify_bio(struct bio *bio)
 		unsigned long level0_ra_pages =
 			min(max_ra_pages, params->level0_blocks - level0_index);
 
-		if (!PageError(page) &&
-		    !verify_page(inode, vi, req, page, level0_ra_pages))
-			SetPageError(page);
+		if (!verify_page(inode, vi, req, page, level0_ra_pages)) {
+			bio->bi_status = BLK_STS_IOERR;
+			break;
+		}
 	}
 
 	fsverity_free_hash_request(params->hash_alg, req);

base-commit: f0c4d9fc9cc9462659728d168387191387e903cc

Signature · 1 line
2.38.1