Re: [RFC PATCH 2/2] dm-verity: don't restart or panic on errors during system shutdown

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Nov 10, 2026 at 01:06:21PM +0000, 周伟江 (Weijiang Zhou) wrote:
> Commit 252bd1256396 ("dm verity: skip verity work if I/O error when system
> is shutting down") stopped I/O errors during shutdown from triggering a
> restart, since devices may legitimately be gone by then. Errors that reach
> verity_handle_err(), e.g. through a failed hash block read followed by a
> failed FEC attempt, are not covered and still call kernel_restart() or
> panic().
>
> During a reboot that is both wrong (nothing is corrupt) and harmful: it
> races with the reboot already in progress and replaces its reason with
> "dm-verity device corrupted", which bootloaders such as Android's treat as
> a verity failure and switch the device to EIO mode.
> 
> Apply the same shutdown check in verity_handle_err(). Corruption is still
> logged, recorded in hash_failed and reported via uevent, and the I/O still
> fails.

verity_handle_err() is supposed to be called only on corruption.  Just
as established by patch 1, there is a bug where an I/O error is
unnecessarily escalated to corruption.

This patch might still be worthwhile, to be robust against hypothetical
storage devices that return successful reads with incorrect data while
the system is shutting down.  Just the commit message and comments don't
really match up with that.

>+/*
>+ * Skip verity work in response to I/O error when system is shutting down.
>+ */
>+static inline bool verity_is_system_shutting_down(void)
>+{
>+return system_state == SYSTEM_HALT || system_state == SYSTEM_POWER_OFF
>+|| system_state == SYSTEM_RESTART;
>+}

The above comment is misleading.  This patch makes this function be
called by verity_handle_err() which is for handling corruption.  It also
occurs *during* the verity work.

> +/*
> + * Devices may already be quiesced (e.g. UFS set offline by its
> + * shutdown hook), so a failure here does not imply corruption.  Fail
> + * the I/O but do not restart or panic, which would also override the
> + * reboot reason of the shutdown already in progress.
> + */
> +if (verity_is_system_shutting_down())
> +return 1;

Shouldn't this go earlier in the function?  By the time we get here,
verity_handle_err() has already recorded the corruption in struct
dm_verity (where DM_TABLE_STATUS can read it) and sent a uevent to
userspace notifying them of the corruption.  That isn't very consistent
with the idea that if verity_is_system_shutting_down() then corruption
is not reliable and should not be reported.

Side note: as Mikulas mentioned, your patches were also corrupted and do
not apply.  You also should not include email footers telling people to
delete your emails, as they may do what you ask.

- Eric