[RFC PATCH 0/2] dm-verity: don't let FEC decode into a dm_bufio page that is already in the cache

[RFC PATCH 0/2] dm-verity: don't let FEC decode into a dm_bufio page that is already in the cache

[周伟江 (Weijiang Zhou)]

Hi,

We would like to ask for your help with something we ran into on dm-verity.

On 6.18 we are seeing a hash block being reported as corrupted while FEC is
in the middle of recovering it, and we suspect the reason is that the block
becomes visible in the dm_bufio cache before FEC has written it. It looks to
us like a regression from 51ba14fb368e (6.15), but we would very much like a
second opinion before going any further.

Two patches follow, as an RFC: they have not been built or run on a device
yet, and we would rather agree on the direction with you first.

1. Root cause

As far as we can tell, verity_verify_level() handles a failed hash block
read by taking a fresh buffer with dm_bufio_new() and decoding into it with
verity_fec_decode():

    data = dm_bufio_new(v->bufio, hash_block, &buf);        /* <-- */

Our understanding is that dm_bufio_new() inserts that buffer into v->bufio
immediately, with no B_READING and no read_error, and never submits I/O for
it. While FEC runs, the cache would then hold a page that cannot be told
apart from a block which was read successfully and simply has not been hash-
checked yet, while its contents are whatever the allocator returned.

FEC then walks the RS codeword and, for the message blocks that live on the
data device, calls verity_hash_for_block() to decide which of them are
erasures - through the same v->bufio. So if one of those data blocks happens
to be covered by the hash block being recovered, the nested
verity_verify_level() would take that placeholder out of the cache, hash
zeros, get a mismatch and, since nested FEC is refused (fio->level != 0 ->
-EIO), end up in verity_handle_err(), which reports corruption.

Both the publication and the read-back happen in the same thread, so it does
not look like a race to us. Whether it triggers seems to depend only on the
geometry.

2. Symptom

Unisoc ums9635, Android 17, kernel 6.18.21, dm-verity over a virtual A/B
system partition: 4K blocks, sha256, fec_roots=2, created with
restart_on_corruption. Rebooting a slot whose snapshot had not been merged
yet, we got:

device-mapper: verity: 254:3: metadata block 229730 is corrupted
reboot: Restarting system with command 'dm-verity device corrupted'

The hash read did fail with -EIO, because the UFS was already offline. The
media itself is fine: we rebuilt that slot from the COW and verified both
slots' hash trees offline, zero bad blocks, and block 229730 contains
exactly the digest the tree expects.

If our reading is right, any failed hash read can trigger this, not just one
during shutdown. With 228497 data blocks, 251 of the 1786 level-0 hash
blocks, all 14 level-1 blocks and the root block have a codeword data block
underneath them, so an I/O error on any of those would be reported as
corruption. android16-6.12 has no 51ba14fb368e, and there it simply returns
-EIO.

3. Code evidence

    /* drivers/md/dm-verity-target.c, verity_verify_level() */
    if (IS_ERR(data)) {
            if (skip_unverified)
                    return 1;
            r = PTR_ERR(data);
            data = dm_bufio_new(v->bufio, hash_block, &buf);        /* <-- */

    /* drivers/md/dm-verity-fec.c, fec_read_bufs() */
    /* locate erasures if the block is on the data device */
    if (bufio == v->fec->data_bufio &&
        verity_hash_for_block(v, io, block, want_digest, &is_zero) == 0) {

What we can add from the minidump, all of it in thread T329 (kverityd):

- block 229730 was read from the device twice; the nested read issued no I/O
  and was served from the cache
- T329 was preempted 171 times between the failed read and the report,
  matching the 171 successful reads of blocks i = 0..170 before the nested
  one
- kprobe: real digest 8b1f331f... equals sha256(salt || 4096 * 0x00), wanted
  digest bac80063... equals offset 0x840 of L1 block 228507 in the image we
  rebuilt offline
- dump, report and restart all carry TID 329

In our case it happens at i = 171: message block 171 * 911 + 158 = 155939,
whose leaf hash is 228512 + (155939 >> 7) = 229730 (region_blocks 911 from
fec_blocks 230298 and roots 2, index_in_region 158).

The guards we looked at do not seem to cover this path: 252bd1256396 in
verity_end_io applies to the outer bio, which succeeded; verity_finish_io
checks bio completion, while verity_handle_err() has no system_state check;
and dm_bufio_release only drops buffers with read_error set, which an
NF_FRESH buffer does not have. If we are missing one, we would be glad to be
corrected.

4. Reproducer, no device and no shutdown needed

1. put a dm-dust layer below the verity hash area and mark one level-1 block
   bad
2. drop the page cache and let the bufio cache go
3. read a data block that has that level-1 block as an ancestor

In our hands an unmodified 6.18 reports "metadata block N is corrupted"
every time, single threaded. We would be happy to turn this into a proper
reproducer if it is useful.

5. Patch 1

What we have tried so far is to decode into a private buffer, so that
nothing enters v->bufio before it holds recovered data:

    fec_buf = kmalloc(1 << v->hash_dev_block_bits, GFP_NOIO);
    if (!fec_buf)
            return r;
    if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA,
                          want_digest, hash_block, fec_buf) == 0) {
            memcpy(want_digest, fec_buf + offset, v->digest_size);
            r = 0;
    }
    kfree(fec_buf);
    return r;

We believe this closes the window rather than narrowing it. A recovered hash
block is no longer cached, so a later access reads it again and, if the
device is still failing, recovers it again - which costs something only on
media that already returns errors, and keeps the change inside the function
that got us here. We also wondered about the other end, having
dm_bufio_new() refuse to publish a buffer before it is filled: that would
hold for every caller rather than just verity, but NF_FRESH exists precisely
so a caller can fill the buffer itself and other targets rely on it, so it
looked like a larger change than this needs. We are quite happy to redo it
there if you prefer.

Patch 2 is unrelated to cache visibility and came out of the same dump:
verity_handle_err() does not consult system_state, so during a shutdown it
still restarts or panics for an error that cannot mean corruption, and
overwrites the reason of the reboot already in progress. It reuses
verity_is_system_shutting_down() and fails the I/O instead. Please look at
the two separately.

6. Questions

1. Have you seen this before, or is a fix already queued somewhere?
   verity_verify_level() and fec_read_bufs() look unchanged in master
   (05441d697f7f), and the v2 review of 51ba14fb368e does not mention cache
   visibility, but you would know better than we do.
2. Would you be willing to check whether our reading of the dm_bufio_new() +
   FEC interaction is right, and whether the shape of patch 1 makes sense?
   If we have it wrong, we would much rather hear it now than later.

The dump comes from a product device so it is not public, but we can extract
anything you would like to see: the raw log_buf, the sched_trace window
around T329, the kprobe output. And of course we are glad to test anything
you suggest on the device.

Thank you for your time, and for the work on dm-verity.

BRs
weijiang.zhou
Unisoc ota team
________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。

[RFC PATCH 1/2] dm-verity: don't decode FEC into a dm_bufio page that is already in the cache

[周伟江 (Weijiang Zhou)]

Commit 51ba14fb368e ("dm-verity: do forward error correction on metadata
I/O errors") handles a failed hash block read by allocating the block with
dm_bufio_new() and decoding into it with verity_fec_decode().

dm_bufio_new() inserts the buffer into the v->bufio cache immediately, with
no B_READING state and no read_error, i.e. indistinguishable from a
successfully read block that merely has not been hash-verified yet. Its
contents are whatever the allocator returned (all zeroes with init_on_alloc),
and no I/O is ever submitted for it.

FEC then reads every other block of the RS codeword and, for blocks on the
data device, calls verity_hash_for_block() on them to decide whether they
are erasures. That walks the hash tree through the same v->bufio. If one of
those data blocks is covered by the hash block being recovered (or by a
descendant of it, when it is an upper level block), the nested
verity_verify_level() gets the placeholder from the cache, hashes the
uninitialized contents, sees a mismatch, cannot recurse into FEC again
(fio->level != 0) and calls verity_handle_err(). With restart_on_corruption
that reboots the machine on a single transient read error, although the
media is intact.

Whether this happens is purely a function of the geometry. With 4K blocks,
sha256 and roots=2, 251 of the 1786 level-0 hash blocks, all level-1 blocks
and the root block have a codeword data block underneath them, so an I/O
error on any of those reliably produces "metadata block N is corrupted".
It was observed on Android devices where a hash read failed with -EIO while
UFS was already offline during reboot.

Decode into a private buffer instead and only copy out the wanted digest on
success. The recovered block is not cached, so a later access will read and,
if needed, recover it again; that is only a cost on devices that are already
failing reads.

Fixes: 51ba14fb368e ("dm-verity: do forward error correction on metadata I/O errors")
Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com>
Signed-off-by: weijiang.zhou <weijiang.zhou@unisoc.com>
---
 drivers/md/dm-verity-target.c | 28 +++++++++++++++++++---------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 08a58a0b5e49..42ee24d823e3 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -266,22 +266,29 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io,
 }

 if (IS_ERR(data)) {
+u8 *fec_buf;
+
 if (skip_unverified)
 return 1;
 r = PTR_ERR(data);
-data = dm_bufio_new(v->bufio, hash_block, &buf);
-if (IS_ERR(data))
+/*
+ * Recover the hash block into a private buffer.  It must not be
+ * made visible in v->bufio before it has been recovered: FEC
+ * verifies the other blocks of the RS codeword through
+ * verity_hash_for_block(), which may need this very hash block
+ * and would then hash an uninitialized buffer, reporting
+ * spurious corruption.
+ */
+fec_buf = kmalloc(1 << v->hash_dev_block_bits, GFP_NOIO);
+if (!fec_buf)
 return r;
 if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA,
-      want_digest, hash_block, data) == 0) {
-aux = dm_bufio_get_aux_data(buf);
-aux->hash_verified = 1;
-goto release_ok;
-} else {
-dm_bufio_release(buf);
-dm_bufio_forget(v->bufio, hash_block);
-return r;
+      want_digest, hash_block, fec_buf) == 0) {
+memcpy(want_digest, fec_buf + offset, v->digest_size);
+r = 0;
 }
+kfree(fec_buf);
+return r;
 }

 aux = dm_bufio_get_aux_data(buf);
@@ -323,7 +330,6 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io,
 }
 }

-release_ok:
 data += offset;
 memcpy(want_digest, data, v->digest_size);
 r = 0;
--
2.43.0
________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。

Re: [RFC PATCH 1/2] dm-verity: don't decode FEC into a dm_bufio page that is already in the cache

[Mikulas Patocka]

[-- Attachment #1: Type: text/plain, Size: 5964 bytes --]

Hi


On Tue, 10 Nov 2026, 周伟江 (Weijiang Zhou) wrote:

> Commit 51ba14fb368e ("dm-verity: do forward error correction on metadata
> I/O errors") handles a failed hash block read by allocating the block with
> dm_bufio_new() and decoding into it with verity_fec_decode().
> 
> dm_bufio_new() inserts the buffer into the v->bufio cache immediately, with
> no B_READING state and no read_error, i.e. indistinguishable from a
> successfully read block that merely has not been hash-verified yet. Its
> contents are whatever the allocator returned (all zeroes with init_on_alloc),
> and no I/O is ever submitted for it.

I don't see why this should be a problem.

If the device returns random data, FEC is supposed to correct it.

If the device returns an I/O error and we create a buffer with random 
data, this is similar to the case where the device returned random data - 
and FEC should also correct it. It doesn't matter whether the random data 
came from the device or from uninitialized kernel memory.

So, it seems to me that this patch is just papering over the problem.

> FEC then reads every other block of the RS codeword and, for blocks on the
> data device, calls verity_hash_for_block() on them to decide whether they
> are erasures. That walks the hash tree through the same v->bufio. If one of
> those data blocks is covered by the hash block being recovered (or by a
> descendant of it, when it is an upper level block), the nested

This is the real problem that should be fixed. Correcting a block 
shouldn't depend on the content of the block being corrected.

> verity_verify_level() gets the placeholder from the cache, hashes the
> uninitialized contents, sees a mismatch, cannot recurse into FEC again
> (fio->level != 0) and calls verity_handle_err(). With restart_on_corruption
> that reboots the machine on a single transient read error, although the
> media is intact.
> 
> Whether this happens is purely a function of the geometry. With 4K blocks,
> sha256 and roots=2, 251 of the 1786 level-0 hash blocks, all level-1 blocks
> and the root block have a codeword data block underneath them, so an I/O
> error on any of those reliably produces "metadata block N is corrupted".
> It was observed on Android devices where a hash read failed with -EIO while
> UFS was already offline during reboot.
> 
> Decode into a private buffer instead and only copy out the wanted digest on
> success. The recovered block is not cached, so a later access will read and,
> if needed, recover it again; that is only a cost on devices that are already
> failing reads.
> 
> Fixes: 51ba14fb368e ("dm-verity: do forward error correction on metadata I/O errors")
> Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com>
> Signed-off-by: weijiang.zhou <weijiang.zhou@unisoc.com>
> ---
>  drivers/md/dm-verity-target.c | 28 +++++++++++++++++++---------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 08a58a0b5e49..42ee24d823e3 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -266,22 +266,29 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io,
>  }
> 
>  if (IS_ERR(data)) {
> +u8 *fec_buf;
> +
>  if (skip_unverified)
>  return 1;
>  r = PTR_ERR(data);
> -data = dm_bufio_new(v->bufio, hash_block, &buf);
> -if (IS_ERR(data))
> +/*
> + * Recover the hash block into a private buffer.  It must not be
> + * made visible in v->bufio before it has been recovered: FEC
> + * verifies the other blocks of the RS codeword through
> + * verity_hash_for_block(), which may need this very hash block
> + * and would then hash an uninitialized buffer, reporting
> + * spurious corruption.
> + */
> +fec_buf = kmalloc(1 << v->hash_dev_block_bits, GFP_NOIO);
> +if (!fec_buf)
>  return r;
>  if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA,
> -      want_digest, hash_block, data) == 0) {
> -aux = dm_bufio_get_aux_data(buf);
> -aux->hash_verified = 1;
> -goto release_ok;
> -} else {
> -dm_bufio_release(buf);
> -dm_bufio_forget(v->bufio, hash_block);
> -return r;
> +      want_digest, hash_block, fec_buf) == 0) {
> +memcpy(want_digest, fec_buf + offset, v->digest_size);
> +r = 0;
>  }
> +kfree(fec_buf);
> +return r;
>  }
> 
>  aux = dm_bufio_get_aux_data(buf);
> @@ -323,7 +330,6 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io,
>  }
>  }
> 
> -release_ok:
>  data += offset;
>  memcpy(want_digest, data, v->digest_size);
>  r = 0;
> --
> 2.43.0

Your mailing software deleted tabs from the patch. Please, fix it.

Mikulas

> ________________________________
>  This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
> 本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。
> 

Re: [RFC PATCH 1/2] dm-verity: don't decode FEC into a dm_bufio page that is already in the cache

[周伟江 (Weijiang Zhou)]

Hi Mikulas,

thank you for the careful review, and for pointing straight at what
needs to be fixed.  We agree with you: the 1/2 sent earlier only hides
the symptom, please ignore it.

To make sure we read the defect the same way: in fec_read_bufs() a block
whose read fails is recorded as an erasure outright; a block that reads
back fine and lives on the data device is judged by fec_is_erasure(),
which compares it against the digest verity_hash_for_block() produced
for it.  Getting that digest can walk the hash tree and read, level by
level, the hash block covering that data block.  When the block being
recovered is itself a metadata block, a data block of the same codeword
can have its hash path run through it - at level 0 one of the data
blocks it covers, above level 0 one of the hash blocks below it.  So
the walk asks for exactly the block we are rebuilding: the one
dm_bufio_new() put in the cache holding allocator leftovers, with no
read ever issued for it.  The digest cannot match, recursion into FEC
is blocked by fio->level, and the walk ends in verity_handle_err() -
the "metadata block N is corrupted" from our devices, which under
restart_on_corruption means a reboot on a single transient read error.
This is only reachable on the second try, the one that asks for
erasures: the first try recovers the block without searching for them.

Rather than defending a patch of our own, what we would really
appreciate is your help with the complete fix.  This is a path where a
partial change worries us: the erasure search, the recursion guard in
verity_fec_decode() and the hash_verified state are all involved, and
from the outside we cannot judge which of them a correct fix has to
touch.  Our concern is that we close the case we can see and open
another risk somewhere we cannot.

Would you describe the fix you would consider correct - where the
change belongs, and what else it has to cover?  We will implement it
exactly as you specify, verify it on the configuration that reproduces
the problem, and send it to you for review before it goes any further.

Thanks again for your time,
Weijiang
________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。

Re: [RFC PATCH 1/2] dm-verity: don't decode FEC into a dm_bufio page that is already in the cache

[Mikulas Patocka]

[-- Attachment #1: Type: text/plain, Size: 4100 bytes --]

Hi


On Wed, 11 Nov 2026, 周伟江 (Weijiang Zhou) wrote:

> Hi Mikulas,
> 
> thank you for the careful review, and for pointing straight at what
> needs to be fixed.  We agree with you: the 1/2 sent earlier only hides
> the symptom, please ignore it.
> 
> To make sure we read the defect the same way: in fec_read_bufs() a block
> whose read fails is recorded as an erasure outright; a block that reads
> back fine and lives on the data device is judged by fec_is_erasure(),
> which compares it against the digest verity_hash_for_block() produced
> for it.  Getting that digest can walk the hash tree and read, level by
> level, the hash block covering that data block.  When the block being
> recovered is itself a metadata block, a data block of the same codeword
> can have its hash path run through it - at level 0 one of the data
> blocks it covers, above level 0 one of the hash blocks below it.  So
> the walk asks for exactly the block we are rebuilding: the one
> dm_bufio_new() put in the cache holding allocator leftovers, with no
> read ever issued for it.  The digest cannot match, recursion into FEC
> is blocked by fio->level, and the walk ends in verity_handle_err() -
> the "metadata block N is corrupted" from our devices, which under
> restart_on_corruption means a reboot on a single transient read error.
> This is only reachable on the second try, the one that asks for
> erasures: the first try recovers the block without searching for them.
> 
> Rather than defending a patch of our own, what we would really
> appreciate is your help with the complete fix.  This is a path where a

I think that the correct fix would be to ensure that the erasure blocks 
for a given tree level are stored at the same level as the blocks that 
they protect. Then, you will never hit the situation where the correction 
recurses into the same block that it attempts to correct. You would need 
to modify the veritysetup tool for that and maybe it would also need to 
introduce a new on-disk format.

The forward error correction was written by Sami Tolvanen, so maybe he can 
help with that.

Mikulas

> partial change worries us: the erasure search, the recursion guard in
> verity_fec_decode() and the hash_verified state are all involved, and
> from the outside we cannot judge which of them a correct fix has to
> touch.  Our concern is that we close the case we can see and open
> another risk somewhere we cannot.
> 
> Would you describe the fix you would consider correct - where the
> change belongs, and what else it has to cover?  We will implement it
> exactly as you specify, verify it on the configuration that reproduces
> the problem, and send it to you for review before it goes any further.
> 
> Thanks again for your time,
> Weijiang
> ________________________________
>  This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
> 本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。
> 

Re: [RFC PATCH 1/2] dm-verity: don't decode FEC into a dm_bufio page that is already in the cache

[Sami Tolvanen]

On Tue, Nov 10, 2026 at 1:08 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> Hi
>
>
> On Tue, 10 Nov 2026, 周伟江 (Weijiang Zhou) wrote:
>
> > Commit 51ba14fb368e ("dm-verity: do forward error correction on metadata
> > I/O errors") handles a failed hash block read by allocating the block with
> > dm_bufio_new() and decoding into it with verity_fec_decode().
> >
> > dm_bufio_new() inserts the buffer into the v->bufio cache immediately, with
> > no B_READING state and no read_error, i.e. indistinguishable from a
> > successfully read block that merely has not been hash-verified yet. Its
> > contents are whatever the allocator returned (all zeroes with init_on_alloc),
> > and no I/O is ever submitted for it.
>
> I don't see why this should be a problem.
>
> If the device returns random data, FEC is supposed to correct it.
>
> If the device returns an I/O error and we create a buffer with random
> data, this is similar to the case where the device returned random data -
> and FEC should also correct it. It doesn't matter whether the random data
> came from the device or from uninitialized kernel memory.
>
> So, it seems to me that this patch is just papering over the problem.
>
> > FEC then reads every other block of the RS codeword and, for blocks on the
> > data device, calls verity_hash_for_block() on them to decide whether they
> > are erasures. That walks the hash tree through the same v->bufio. If one of
> > those data blocks is covered by the hash block being recovered (or by a
> > descendant of it, when it is an upper level block), the nested
>
> This is the real problem that should be fixed. Correcting a block
> shouldn't depend on the content of the block being corrected.

It doesn't depend on it. We only look for erasures because knowing
which blocks are corrupt allows us to correct twice as many errors,
but this is completely optional. Sounds like the correct solution is
not to call verity_handle_err() on verification errors while we're
only looking up erasures, since these are not fatal errors.

> > verity_verify_level() gets the placeholder from the cache, hashes the
> > uninitialized contents, sees a mismatch, cannot recurse into FEC again
> > (fio->level != 0) and calls verity_handle_err(). With restart_on_corruption
> > that reboots the machine on a single transient read error, although the
> > media is intact.
> >
> > Whether this happens is purely a function of the geometry. With 4K blocks,
> > sha256 and roots=2, 251 of the 1786 level-0 hash blocks, all level-1 blocks
> > and the root block have a codeword data block underneath them, so an I/O
> > error on any of those reliably produces "metadata block N is corrupted".
> > It was observed on Android devices where a hash read failed with -EIO while
> > UFS was already offline during reboot.

We also shouldn't try to correct I/O errors when the system is
shutting down. We already have verity_is_system_shutting_down() for
this purpose, perhaps we should use it in verity_fec_decode() too?

Sami

Re: [RFC PATCH 1/2] dm-verity: don't decode FEC into a dm_bufio page that is already in the cache

[Eric Biggers]

On Tue, Nov 10, 2026 at 10:08:02PM +0100, Mikulas Patocka wrote:
> On Tue, 10 Nov 2026, 周伟江 (Weijiang Zhou) wrote:
> > Commit 51ba14fb368e ("dm-verity: do forward error correction on metadata
> > I/O errors") handles a failed hash block read by allocating the block with
> > dm_bufio_new() and decoding into it with verity_fec_decode().
> > 
> > dm_bufio_new() inserts the buffer into the v->bufio cache immediately, with
> > no B_READING state and no read_error, i.e. indistinguishable from a
> > successfully read block that merely has not been hash-verified yet. Its
> > contents are whatever the allocator returned (all zeroes with init_on_alloc),
> > and no I/O is ever submitted for it.
> 
> I don't see why this should be a problem.
> 
> If the device returns random data, FEC is supposed to correct it.
> 
> If the device returns an I/O error and we create a buffer with random 
> data, this is similar to the case where the device returned random data - 
> and FEC should also correct it. It doesn't matter whether the random data 
> came from the device or from uninitialized kernel memory.
> 
> So, it seems to me that this patch is just papering over the problem.

dm-verity does use FEC to try to recover from I/O errors, but this
doesn't mean that I/O errors and corruption are interchangeable and
handled in the same way.  See the difference at the end of
verity_handle_data_hash_mismatch(), for example.  It calls
verity_handle_err() only on actual corruption, not an I/O error.  That
triggers the corruption policy controlled by 'v->mode', which is
different from the I/O error policy controlled by 'v->error_mode'.

> > FEC then reads every other block of the RS codeword and, for blocks on the
> > data device, calls verity_hash_for_block() on them to decide whether they
> > are erasures. That walks the hash tree through the same v->bufio. If one of
> > those data blocks is covered by the hash block being recovered (or by a
> > descendant of it, when it is an upper level block), the nested
> 
> This is the real problem that should be fixed. Correcting a block 
> shouldn't depend on the content of the block being corrected.

There does seem to be a bug where when FEC is trying to read data block
hashes to correct a block, it can detect corruption in the hash tree and
be unable to correct it due to FEC being non-recursive.  That triggers
the corruption policy, when really it shouldn't and FEC should just
consider erasure information unavailable.  That is fixable.

But it is not the root cause problem here.  The root cause problem is
that the call to dm_bufio_new() injects uninitialized data into the hash
block cache as if it came from a successful read.  That turns an I/O
error into corruption, making the corruption policy take effect merely
due to an I/O error.  This is not limited to FEC reading the data block
erasures, either; any concurrent reader can trip over this corruption.

It seems that dm_bufio_new() was designed for writers that overwrite the
full block without reading any of it while also excluding concurrent
readers, e.g. using locks.  That just does not work here, unfortunately.

Using a kmalloc() buffer (this patch) kind of works but is a hack
because it doesn't actually populate the cache at all.  I think the
correct solution would be to have the FEC output initially just go into
fio->output, then populate the cache atomically with the data.  Seems
that dm-bufio.c will need a new function for that.

- Eric

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

[周伟江 (Weijiang Zhou)]

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.

Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com>
Signed-off-by: weijiang.zhou <weijiang.zhou@unisoc.com>
---
 drivers/md/dm-verity-target.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 42ee24d823e3..756f1202490b 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -169,6 +169,15 @@ static void verity_hash_at_level(struct dm_verity *v, sector_t block, int level,
 *offset = idx << (v->hash_dev_block_bits - v->hash_per_block_bits);
 }

+/*
+ * 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;
+}
+
 /*
  * Handle verification errors.
  */
@@ -216,6 +225,15 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type,
 if (v->mode == DM_VERITY_MODE_LOGGING)
 return 0;

+/*
+ * 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;
+
 if (v->mode == DM_VERITY_MODE_RESTART)
 kernel_restart("dm-verity device corrupted");

@@ -598,15 +616,6 @@ static int verity_verify_io(struct dm_verity_io *io)
 return r;
 }

-/*
- * 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;
-}
-
 static void restart_io_error(struct work_struct *w)
 {
 kernel_restart("dm-verity device has I/O error");
--
2.43.0
________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。

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

[Mikulas Patocka]

[-- Attachment #1: Type: text/plain, Size: 4348 bytes --]



On Tue, 10 Nov 2026, 周伟江 (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.

This patch is acceptable - but, please, resend the patch with proper TABs.

Mikulas

> Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com>
> Signed-off-by: weijiang.zhou <weijiang.zhou@unisoc.com>
> ---
>  drivers/md/dm-verity-target.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 42ee24d823e3..756f1202490b 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -169,6 +169,15 @@ static void verity_hash_at_level(struct dm_verity *v, sector_t block, int level,
>  *offset = idx << (v->hash_dev_block_bits - v->hash_per_block_bits);
>  }
> 
> +/*
> + * 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;
> +}
> +
>  /*
>   * Handle verification errors.
>   */
> @@ -216,6 +225,15 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type,
>  if (v->mode == DM_VERITY_MODE_LOGGING)
>  return 0;
> 
> +/*
> + * 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;
> +
>  if (v->mode == DM_VERITY_MODE_RESTART)
>  kernel_restart("dm-verity device corrupted");
> 
> @@ -598,15 +616,6 @@ static int verity_verify_io(struct dm_verity_io *io)
>  return r;
>  }
> 
> -/*
> - * 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;
> -}
> -
>  static void restart_io_error(struct work_struct *w)
>  {
>  kernel_restart("dm-verity device has I/O error");
> --
> 2.43.0
> ________________________________
>  This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
> 本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。
> 

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

[周伟江 (Weijiang Zhou)]

Hi Mikulas,

thanks for the review, and sorry about the TABs.  They were not lost while
composing: the message left my client intact, and the rich-text path of our
corporate mail setup re-encoded the text part on the way out, dropped every
TAB and reflowed the long lines.

I have moved to a submission path that hands the MIME to the server byte for
byte, and checked the round trip through a mailbox first.  The patch below is
the same patch, unchanged - no code was touched since your review, only the
whitespace is back.

Thanks,
Weijiang

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.

Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com>
Signed-off-by: weijiang.zhou <weijiang.zhou@unisoc.com>
---
 drivers/md/dm-verity-target.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 42ee24d823e3..756f1202490b 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -169,6 +169,15 @@ static void verity_hash_at_level(struct dm_verity *v, sector_t block, int level,
 *offset = idx << (v->hash_dev_block_bits - v->hash_per_block_bits);
 }

+/*
+ * 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;
+}
+
 /*
  * Handle verification errors.
  */
@@ -216,6 +225,15 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type,
 if (v->mode == DM_VERITY_MODE_LOGGING)
 return 0;

+/*
+ * 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;
+
 if (v->mode == DM_VERITY_MODE_RESTART)
 kernel_restart("dm-verity device corrupted");

@@ -598,15 +616,6 @@ static int verity_verify_io(struct dm_verity_io *io)
 return r;
 }

-/*
- * 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;
-}
-
 static void restart_io_error(struct work_struct *w)
 {
 kernel_restart("dm-verity device has I/O error");
--
2.43.0
________________________________
 This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。

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

[Mikulas Patocka]

[-- Attachment #1: Type: text/plain, Size: 4937 bytes --]



On Wed, 11 Nov 2026, 周伟江 (Weijiang Zhou) wrote:

> Hi Mikulas,
> 
> thanks for the review, and sorry about the TABs.  They were not lost while
> composing: the message left my client intact, and the rich-text path of our
> corporate mail setup re-encoded the text part on the way out, dropped every
> TAB and reflowed the long lines.
> 
> I have moved to a submission path that hands the MIME to the server byte for
> byte, and checked the round trip through a mailbox first.  The patch below is
> the same patch, unchanged - no code was touched since your review, only the
> whitespace is back.
> 
> Thanks,
> Weijiang

Hi

This email has deleted TABs too.

I applied the patch manually, no need to resend it.

Mikulas

> 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.
> 
> Signed-off-by: Hongyu Jin <hongyu.jin@unisoc.com>
> Signed-off-by: weijiang.zhou <weijiang.zhou@unisoc.com>
> ---
>  drivers/md/dm-verity-target.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 42ee24d823e3..756f1202490b 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -169,6 +169,15 @@ static void verity_hash_at_level(struct dm_verity *v, sector_t block, int level,
>  *offset = idx << (v->hash_dev_block_bits - v->hash_per_block_bits);
>  }
> 
> +/*
> + * 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;
> +}
> +
>  /*
>   * Handle verification errors.
>   */
> @@ -216,6 +225,15 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type,
>  if (v->mode == DM_VERITY_MODE_LOGGING)
>  return 0;
> 
> +/*
> + * 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;
> +
>  if (v->mode == DM_VERITY_MODE_RESTART)
>  kernel_restart("dm-verity device corrupted");
> 
> @@ -598,15 +616,6 @@ static int verity_verify_io(struct dm_verity_io *io)
>  return r;
>  }
> 
> -/*
> - * 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;
> -}
> -
>  static void restart_io_error(struct work_struct *w)
>  {
>  kernel_restart("dm-verity device has I/O error");
> --
> 2.43.0
> ________________________________
>  This email (including its attachments) is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. Unauthorized use, dissemination, distribution or copying of this email or the information herein or taking any action in reliance on the contents of this email or the information herein, by anyone other than the intended recipient, or an employee or agent responsible for delivering the message to the intended recipient, is strictly prohibited. If you are not the intended recipient, please do not read, copy, use or disclose any part of this e-mail to others. Please notify the sender immediately and permanently delete this e-mail and any attachments if you received it in error. Internet communications cannot be guaranteed to be timely, secure, error-free or virus-free. The sender does not accept liability for any errors or omissions.
> 本邮件及其附件具有保密性质，受法律保护不得泄露，仅发送给本邮件所指特定收件人。严禁非经授权使用、宣传、发布或复制本邮件或其内容。若非该特定收件人，请勿阅读、复制、 使用或披露本邮件的任何内容。若误收本邮件，请从系统中永久性删除本邮件及所有附件，并以回复邮件的方式即刻告知发件人。无法保证互联网通信及时、安全、无误或防毒。发件人对任何错漏均不承担责任。
> 

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

[Eric Biggers]

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