| Message ID | 5f7c2e37-fd4f-4371-3b0d-633b5a99743a@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mikulas Patocka |
| Headers | show |
| Series | dm-verity: don't crash if panic_on_corruption is not selected | expand |
Hi Mikulas, On Tue, Oct 29, 2024 at 11:17 AM Mikulas Patocka <mpatocka@redhat.com> wrote: > > If the user sets panic_on_error and doesn't set panic_on_corruption, > dm-verity should not panic on data mismatch. But, currently it panics, > because it treats data mismatch as I/O error. > > This commit fixes the logic so that if there is data mismatch and > panic_on_corruption or restart_on_corruption is not selected, the system > won't restart or panic. > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > Fixes: f811b83879fb ("dm-verity: introduce the options restart_on_error and panic_on_error") > > --- > drivers/md/dm-verity-target.c | 9 ++++++--- > drivers/md/dm-verity.h | 1 + > 2 files changed, 7 insertions(+), 3 deletions(-) > > Index: linux-2.6/drivers/md/dm-verity-target.c > =================================================================== > --- linux-2.6.orig/drivers/md/dm-verity-target.c 2024-10-16 16:42:46.000000000 +0200 > +++ linux-2.6/drivers/md/dm-verity-target.c 2024-10-16 17:11:13.000000000 +0200 > @@ -356,9 +356,9 @@ static int verity_verify_level(struct dm > else if (verity_handle_err(v, > DM_VERITY_BLOCK_TYPE_METADATA, > hash_block)) { > - struct bio *bio = > - dm_bio_from_per_bio_data(io, > - v->ti->per_io_data_size); > + struct bio *bio; > + io->had_mismatch = true; > + bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size); > dm_audit_log_bio(DM_MSG_PREFIX, "verify-metadata", bio, > block, 0); > r = -EIO; > @@ -482,6 +482,7 @@ static int verity_handle_data_hash_misma > return -EIO; /* Error correction failed; Just return error */ > > if (verity_handle_err(v, DM_VERITY_BLOCK_TYPE_DATA, blkno)) { > + io->had_mismatch = true; > dm_audit_log_bio(DM_MSG_PREFIX, "verify-data", bio, blkno, 0); > return -EIO; > } > @@ -606,6 +607,7 @@ static void verity_finish_io(struct dm_v > > if (unlikely(status != BLK_STS_OK) && > unlikely(!(bio->bi_opf & REQ_RAHEAD)) && > + !io->had_mismatch && > !verity_is_system_shutting_down()) { > if (v->error_mode == DM_VERITY_MODE_PANIC) { > panic("dm-verity device has I/O error"); > @@ -779,6 +781,7 @@ static int verity_map(struct dm_target * > io->orig_bi_end_io = bio->bi_end_io; > io->block = bio->bi_iter.bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT); > io->n_blocks = bio->bi_iter.bi_size >> v->data_dev_block_bits; > + io->had_mismatch = false; > > bio->bi_end_io = verity_end_io; > bio->bi_private = io; > Index: linux-2.6/drivers/md/dm-verity.h > =================================================================== > --- linux-2.6.orig/drivers/md/dm-verity.h 2024-10-16 16:41:30.000000000 +0200 > +++ linux-2.6/drivers/md/dm-verity.h 2024-10-16 17:06:37.000000000 +0200 > @@ -92,6 +92,7 @@ struct dm_verity_io { > sector_t block; > unsigned int n_blocks; > bool in_bh; > + bool had_mismatch; This looks correct to me, although I assume wanting to panic only on I/O errors is probably rare. Reviewed-by: Sami Tolvanen <samitolvanen@google.com> Sami
Index: linux-2.6/drivers/md/dm-verity-target.c =================================================================== --- linux-2.6.orig/drivers/md/dm-verity-target.c 2024-10-16 16:42:46.000000000 +0200 +++ linux-2.6/drivers/md/dm-verity-target.c 2024-10-16 17:11:13.000000000 +0200 @@ -356,9 +356,9 @@ static int verity_verify_level(struct dm else if (verity_handle_err(v, DM_VERITY_BLOCK_TYPE_METADATA, hash_block)) { - struct bio *bio = - dm_bio_from_per_bio_data(io, - v->ti->per_io_data_size); + struct bio *bio; + io->had_mismatch = true; + bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size); dm_audit_log_bio(DM_MSG_PREFIX, "verify-metadata", bio, block, 0); r = -EIO; @@ -482,6 +482,7 @@ static int verity_handle_data_hash_misma return -EIO; /* Error correction failed; Just return error */ if (verity_handle_err(v, DM_VERITY_BLOCK_TYPE_DATA, blkno)) { + io->had_mismatch = true; dm_audit_log_bio(DM_MSG_PREFIX, "verify-data", bio, blkno, 0); return -EIO; } @@ -606,6 +607,7 @@ static void verity_finish_io(struct dm_v if (unlikely(status != BLK_STS_OK) && unlikely(!(bio->bi_opf & REQ_RAHEAD)) && + !io->had_mismatch && !verity_is_system_shutting_down()) { if (v->error_mode == DM_VERITY_MODE_PANIC) { panic("dm-verity device has I/O error"); @@ -779,6 +781,7 @@ static int verity_map(struct dm_target * io->orig_bi_end_io = bio->bi_end_io; io->block = bio->bi_iter.bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT); io->n_blocks = bio->bi_iter.bi_size >> v->data_dev_block_bits; + io->had_mismatch = false; bio->bi_end_io = verity_end_io; bio->bi_private = io; Index: linux-2.6/drivers/md/dm-verity.h =================================================================== --- linux-2.6.orig/drivers/md/dm-verity.h 2024-10-16 16:41:30.000000000 +0200 +++ linux-2.6/drivers/md/dm-verity.h 2024-10-16 17:06:37.000000000 +0200 @@ -92,6 +92,7 @@ struct dm_verity_io { sector_t block; unsigned int n_blocks; bool in_bh; + bool had_mismatch; struct work_struct work; struct work_struct bh_work;
If the user sets panic_on_error and doesn't set panic_on_corruption, dm-verity should not panic on data mismatch. But, currently it panics, because it treats data mismatch as I/O error. This commit fixes the logic so that if there is data mismatch and panic_on_corruption or restart_on_corruption is not selected, the system won't restart or panic. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Fixes: f811b83879fb ("dm-verity: introduce the options restart_on_error and panic_on_error") --- drivers/md/dm-verity-target.c | 9 ++++++--- drivers/md/dm-verity.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)