On Tue, Sep 24, 2024 at 03:18:29PM +0200, Mikulas Patocka wrote:
> Maxim Suhanov reported that dm-verity doesn't crash if an I/O error
> happens. In theory, this could be used to subvert security, because an
> attacker can create sectors that return error with the Write Uncorrectable
> command. Some programs may misbehave if they have to deal with EIO.
>
> This commit fixes dm-verity, so that if "panic_on_corruption" or
> "restart_on_corruption" was specified and an I/O error happens, the
> machine will panic or restart.
How does this interact with FEC, which can correct for I/O errors on the
underlying media by reconstructing the data?
> @@ -596,6 +596,19 @@ static void verity_finish_io(struct dm_v
> if (!static_branch_unlikely(&use_bh_wq_enabled) || !io->in_bh)
> verity_fec_finish_io(io);
>
> + if (unlikely(status != BLK_STS_OK) && unlikely(!(bio->bi_opf & REQ_RAHEAD))) {
> + if (v->mode == DM_VERITY_MODE_RESTART ||
> + v->mode == DM_VERITY_MODE_PANIC)
> + DMERR_LIMIT("%s has error: %s", v->data_dev->name,
> + blk_status_to_str(status));
> +
> + if (v->mode == DM_VERITY_MODE_RESTART)
> + emergency_restart();
> +
> + if (v->mode == DM_VERITY_MODE_PANIC)
> + panic("dm-verity device has I/O error");
The blk_status_t code that is checked above is usually the one that
verity_verify_io() produced, not the original one. This could be an error from
a cause other than an I/O error on the underlying media, like data corruption
detected by dm-verity, or an error from the crypto API.
Maybe what you are trying to do is really "panic if an I/O error would be
returned to the layer above", not "panic if an I/O error occurred on the
underlying media"? If so, this needs to be clearly explained.
- Eric
