* [PATCH] dm-verity: do forward error correction on metadata I/O errors
@ 2025-02-10 15:04 Mikulas Patocka
2025-02-10 16:39 ` Milan Broz
2025-02-10 16:57 ` [PATCH] " Sami Tolvanen
0 siblings, 2 replies; 6+ messages in thread
From: Mikulas Patocka @ 2025-02-10 15:04 UTC (permalink / raw)
To: Akilesh Kailash
Cc: Sami Tolvanen, akailash, kernel-team, Alasdair Kergon,
Mike Snitzer, Milan Broz, dm-devel, linux-kernel
Do forward error correction if metadata I/O fails.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
drivers/md/dm-verity-target.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/md/dm-verity-target.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-verity-target.c 2025-02-04 13:52:45.000000000 +0100
+++ linux-2.6/drivers/md/dm-verity-target.c 2025-02-10 15:55:42.000000000 +0100
@@ -324,8 +324,22 @@ static int verity_verify_level(struct dm
&buf, bio->bi_ioprio);
}
- if (IS_ERR(data))
- return PTR_ERR(data);
+ if (IS_ERR(data)) {
+ r = PTR_ERR(data);
+ data = dm_bufio_new(v->bufio, hash_block, &buf);
+ if (IS_ERR(data))
+ return r;
+ if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA,
+ 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;
+ }
+ }
aux = dm_bufio_get_aux_data(buf);
@@ -366,6 +380,7 @@ static int verity_verify_level(struct dm
}
}
+release_ok:
data += offset;
memcpy(want_digest, data, v->digest_size);
r = 0;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] dm-verity: do forward error correction on metadata I/O errors 2025-02-10 15:04 [PATCH] dm-verity: do forward error correction on metadata I/O errors Mikulas Patocka @ 2025-02-10 16:39 ` Milan Broz 2025-02-10 16:46 ` [PATCH v2] " Mikulas Patocka 2025-02-10 16:57 ` [PATCH] " Sami Tolvanen 1 sibling, 1 reply; 6+ messages in thread From: Milan Broz @ 2025-02-10 16:39 UTC (permalink / raw) To: Mikulas Patocka, Akilesh Kailash Cc: Sami Tolvanen, kernel-team, Alasdair Kergon, Mike Snitzer, dm-devel, linux-kernel On 2/10/25 4:04 PM, Mikulas Patocka wrote: > Do forward error correction if metadata I/O fails. > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Not directly related to this patch, but... please could you also increase the dm-verity version? I just implemented support for the errors-as-corruption dm-verity flags in veritysetup. The version of dm-verity stayed at 1.10 for a very long, and the IO error processing change was a functional change that we would like to detect. We cannot set version retrospectively, but at least now with other changes. (Veritysetup tries the requested flags; if activation fails, it displays a better error message based on the detected target version. That way it works even with backports.) Thanks, Milan > > --- > drivers/md/dm-verity-target.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > Index: linux-2.6/drivers/md/dm-verity-target.c > =================================================================== > --- linux-2.6.orig/drivers/md/dm-verity-target.c 2025-02-04 13:52:45.000000000 +0100 > +++ linux-2.6/drivers/md/dm-verity-target.c 2025-02-10 15:55:42.000000000 +0100 > @@ -324,8 +324,22 @@ static int verity_verify_level(struct dm > &buf, bio->bi_ioprio); > } > > - if (IS_ERR(data)) > - return PTR_ERR(data); > + if (IS_ERR(data)) { > + r = PTR_ERR(data); > + data = dm_bufio_new(v->bufio, hash_block, &buf); > + if (IS_ERR(data)) > + return r; > + if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA, > + 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; > + } > + } > > aux = dm_bufio_get_aux_data(buf); > > @@ -366,6 +380,7 @@ static int verity_verify_level(struct dm > } > } > > +release_ok: > data += offset; > memcpy(want_digest, data, v->digest_size); > r = 0; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] dm-verity: do forward error correction on metadata I/O errors 2025-02-10 16:39 ` Milan Broz @ 2025-02-10 16:46 ` Mikulas Patocka 0 siblings, 0 replies; 6+ messages in thread From: Mikulas Patocka @ 2025-02-10 16:46 UTC (permalink / raw) To: Milan Broz Cc: Akilesh Kailash, Sami Tolvanen, kernel-team, Alasdair Kergon, Mike Snitzer, dm-devel, linux-kernel On Mon, 10 Feb 2025, Milan Broz wrote: > On 2/10/25 4:04 PM, Mikulas Patocka wrote: > > Do forward error correction if metadata I/O fails. > > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > Not directly related to this patch, but... > please could you also increase the dm-verity version? > > I just implemented support for the errors-as-corruption > dm-verity flags in veritysetup. > > The version of dm-verity stayed at 1.10 for a very long, and > the IO error processing change was a functional change that we > would like to detect. > We cannot set version retrospectively, but at least now with other > changes. > > (Veritysetup tries the requested flags; if activation fails, > it displays a better error message based on the detected > target version. That way it works even with backports.) > > Thanks, > Milan OK. From: Mikulas Patocka <mpatocka@redhat.com> Do forward error correction if metadata I/O fails. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> --- drivers/md/dm-verity-target.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) Index: linux-2.6/drivers/md/dm-verity-target.c =================================================================== --- linux-2.6.orig/drivers/md/dm-verity-target.c 2025-02-10 16:24:56.000000000 +0100 +++ linux-2.6/drivers/md/dm-verity-target.c 2025-02-10 17:43:50.000000000 +0100 @@ -324,8 +324,22 @@ static int verity_verify_level(struct dm &buf, bio->bi_ioprio); } - if (IS_ERR(data)) - return PTR_ERR(data); + if (IS_ERR(data)) { + r = PTR_ERR(data); + data = dm_bufio_new(v->bufio, hash_block, &buf); + if (IS_ERR(data)) + return r; + if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA, + 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; + } + } aux = dm_bufio_get_aux_data(buf); @@ -366,6 +380,7 @@ static int verity_verify_level(struct dm } } +release_ok: data += offset; memcpy(want_digest, data, v->digest_size); r = 0; @@ -1761,7 +1776,7 @@ static struct target_type verity_target .name = "verity", /* Note: the LSMs depend on the singleton and immutable features */ .features = DM_TARGET_SINGLETON | DM_TARGET_IMMUTABLE, - .version = {1, 10, 0}, + .version = {1, 11, 0}, .module = THIS_MODULE, .ctr = verity_ctr, .dtr = verity_dtr, ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dm-verity: do forward error correction on metadata I/O errors 2025-02-10 15:04 [PATCH] dm-verity: do forward error correction on metadata I/O errors Mikulas Patocka 2025-02-10 16:39 ` Milan Broz @ 2025-02-10 16:57 ` Sami Tolvanen 2025-02-10 19:05 ` Eric Biggers 1 sibling, 1 reply; 6+ messages in thread From: Sami Tolvanen @ 2025-02-10 16:57 UTC (permalink / raw) To: Mikulas Patocka Cc: Akilesh Kailash, kernel-team, Alasdair Kergon, Mike Snitzer, Milan Broz, dm-devel, linux-kernel Hi Mikulas, On Mon, Feb 10, 2025 at 7:04 AM Mikulas Patocka <mpatocka@redhat.com> wrote: > > Do forward error correction if metadata I/O fails. > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > --- > drivers/md/dm-verity-target.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > Index: linux-2.6/drivers/md/dm-verity-target.c > =================================================================== > --- linux-2.6.orig/drivers/md/dm-verity-target.c 2025-02-04 13:52:45.000000000 +0100 > +++ linux-2.6/drivers/md/dm-verity-target.c 2025-02-10 15:55:42.000000000 +0100 > @@ -324,8 +324,22 @@ static int verity_verify_level(struct dm > &buf, bio->bi_ioprio); > } > > - if (IS_ERR(data)) > - return PTR_ERR(data); > + if (IS_ERR(data)) { > + r = PTR_ERR(data); > + data = dm_bufio_new(v->bufio, hash_block, &buf); > + if (IS_ERR(data)) > + return r; > + if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA, > + 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; > + } > + } Don't we still have to check for io->in_bh before trying to correct the error? Overall, it would be nice not to duplicate code here. Should the metadata error correction / handling be moved to a separate function similar to verity_handle_data_hash_mismatch? Sami ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dm-verity: do forward error correction on metadata I/O errors 2025-02-10 16:57 ` [PATCH] " Sami Tolvanen @ 2025-02-10 19:05 ` Eric Biggers 2025-02-17 21:35 ` Mikulas Patocka 0 siblings, 1 reply; 6+ messages in thread From: Eric Biggers @ 2025-02-10 19:05 UTC (permalink / raw) To: Sami Tolvanen Cc: Mikulas Patocka, Akilesh Kailash, kernel-team, Alasdair Kergon, Mike Snitzer, Milan Broz, dm-devel, linux-kernel On Mon, Feb 10, 2025 at 08:57:55AM -0800, Sami Tolvanen wrote: > Hi Mikulas, > > On Mon, Feb 10, 2025 at 7:04 AM Mikulas Patocka <mpatocka@redhat.com> wrote: > > > > Do forward error correction if metadata I/O fails. > > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > > > --- > > drivers/md/dm-verity-target.c | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > Index: linux-2.6/drivers/md/dm-verity-target.c > > =================================================================== > > --- linux-2.6.orig/drivers/md/dm-verity-target.c 2025-02-04 13:52:45.000000000 +0100 > > +++ linux-2.6/drivers/md/dm-verity-target.c 2025-02-10 15:55:42.000000000 +0100 > > @@ -324,8 +324,22 @@ static int verity_verify_level(struct dm > > &buf, bio->bi_ioprio); > > } > > > > - if (IS_ERR(data)) > > - return PTR_ERR(data); > > + if (IS_ERR(data)) { > > + r = PTR_ERR(data); > > + data = dm_bufio_new(v->bufio, hash_block, &buf); > > + if (IS_ERR(data)) > > + return r; > > + if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA, > > + 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; > > + } > > + } > > Don't we still have to check for io->in_bh before trying to correct the error? > > Overall, it would be nice not to duplicate code here. Should the > metadata error correction / handling be moved to a separate function > similar to verity_handle_data_hash_mismatch? > It's also incorrect to do this when skip_verified=true, since in that case want_digest (which verity_fec_decode() uses) has not been initialized yet. - Eric ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dm-verity: do forward error correction on metadata I/O errors 2025-02-10 19:05 ` Eric Biggers @ 2025-02-17 21:35 ` Mikulas Patocka 0 siblings, 0 replies; 6+ messages in thread From: Mikulas Patocka @ 2025-02-17 21:35 UTC (permalink / raw) To: Eric Biggers Cc: Sami Tolvanen, Akilesh Kailash, kernel-team, Alasdair Kergon, Mike Snitzer, Milan Broz, dm-devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2425 bytes --] On Mon, 10 Feb 2025, Eric Biggers wrote: > On Mon, Feb 10, 2025 at 08:57:55AM -0800, Sami Tolvanen wrote: > > Hi Mikulas, > > > > On Mon, Feb 10, 2025 at 7:04 AM Mikulas Patocka <mpatocka@redhat.com> wrote: > > > > > > Do forward error correction if metadata I/O fails. > > > > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > > > > > --- > > > drivers/md/dm-verity-target.c | 19 +++++++++++++++++-- > > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > > > Index: linux-2.6/drivers/md/dm-verity-target.c > > > =================================================================== > > > --- linux-2.6.orig/drivers/md/dm-verity-target.c 2025-02-04 13:52:45.000000000 +0100 > > > +++ linux-2.6/drivers/md/dm-verity-target.c 2025-02-10 15:55:42.000000000 +0100 > > > @@ -324,8 +324,22 @@ static int verity_verify_level(struct dm > > > &buf, bio->bi_ioprio); > > > } > > > > > > - if (IS_ERR(data)) > > > - return PTR_ERR(data); > > > + if (IS_ERR(data)) { > > > + r = PTR_ERR(data); > > > + data = dm_bufio_new(v->bufio, hash_block, &buf); > > > + if (IS_ERR(data)) > > > + return r; > > > + if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_METADATA, > > > + 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; > > > + } > > > + } > > > > Don't we still have to check for io->in_bh before trying to correct the error? dm_bufio_get doesn't return an error code. But adding a test for it doesn't hurt. > > Overall, it would be nice not to duplicate code here. Should the > > metadata error correction / handling be moved to a separate function > > similar to verity_handle_data_hash_mismatch? > > > > It's also incorrect to do this when skip_verified=true, since in that case > want_digest (which verity_fec_decode() uses) has not been initialized yet. > > - Eric Yes, I changed it to return "1" if "skip_unverified" is set. Mikulas ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-17 21:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-10 15:04 [PATCH] dm-verity: do forward error correction on metadata I/O errors Mikulas Patocka 2025-02-10 16:39 ` Milan Broz 2025-02-10 16:46 ` [PATCH v2] " Mikulas Patocka 2025-02-10 16:57 ` [PATCH] " Sami Tolvanen 2025-02-10 19:05 ` Eric Biggers 2025-02-17 21:35 ` Mikulas Patocka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox