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
