On Tue, 24 Sep 2024, Eric Biggers wrote: > 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? It tries to do FEC, if it succeeds, it continues, if it doesn't succeed, it panics or restarts. > > @@ -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 The function verity_finish_io is called when we are going to return the bio to the upper layer. So, the semantics is "panic if an I/O error would be returned to the layer above". Mikulas