[RFC PATCH] dm-verity: disable recursive forward error correction

[RFC PATCH] dm-verity: disable recursive forward error correction

[Mikulas Patocka]

Hi

What do you think about this patch?

There are two problems with the recursive correction:

1. It may cause denial-of-service. In fec_read_bufs, there is a loop that 
has 253 iterations. For each iteration, we may call verity_hash_for_block 
recursively. There is a limit of 4 nested recursions - that means that 
there may be at most 253^4 (4 billion) iterations. Red Hat QE team 
actually created an image that that pushes dm-verity to this limit - and 
this image just makes the udev-worker process get stuck in the 'D' state.

2. It probably doesn't work. In fec_read_bufs we store data into the 
variable "fio->bufs", but fio bufs is shared between recursive 
invocations, if "verity_hash_for_block" invoked correction recursively, it 
would overwrite partially filled fio->bufs.

So, I'm suggesting to remove recursive correction at all. This patch
passed the cryptsetup testsuite. Does it pass your tests too?

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Reported-by: Guangwu Zhang <guazhang@redhat.com>

---
 drivers/md/dm-verity-fec.c    |    4 +---
 drivers/md/dm-verity-fec.h    |    3 ---
 drivers/md/dm-verity-target.c |    2 +-
 3 files changed, 2 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/md/dm-verity-fec.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-verity-fec.c	2025-11-14 15:15:31.000000000 +0100
+++ linux-2.6/drivers/md/dm-verity-fec.c	2025-11-14 16:33:42.000000000 +0100
@@ -417,10 +417,8 @@ int verity_fec_decode(struct dm_verity *
 	if (!verity_fec_is_enabled(v))
 		return -EOPNOTSUPP;
 
-	if (fio->level >= DM_VERITY_FEC_MAX_RECURSION) {
-		DMWARN_LIMIT("%s: FEC: recursion too deep", v->data_dev->name);
+	if (fio->level)
 		return -EIO;
-	}
 
 	fio->level++;
 
Index: linux-2.6/drivers/md/dm-verity-fec.h
===================================================================
--- linux-2.6.orig/drivers/md/dm-verity-fec.h	2025-11-14 15:15:31.000000000 +0100
+++ linux-2.6/drivers/md/dm-verity-fec.h	2025-11-14 15:21:39.000000000 +0100
@@ -23,9 +23,6 @@
 #define DM_VERITY_FEC_BUF_MAX \
 	(1 << (PAGE_SHIFT - DM_VERITY_FEC_BUF_RS_BITS))
 
-/* maximum recursion level for verity_fec_decode */
-#define DM_VERITY_FEC_MAX_RECURSION	4
-
 #define DM_VERITY_OPT_FEC_DEV		"use_fec_from_device"
 #define DM_VERITY_OPT_FEC_BLOCKS	"fec_blocks"
 #define DM_VERITY_OPT_FEC_START		"fec_start"
Index: linux-2.6/drivers/md/dm-verity-target.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-verity-target.c	2025-08-15 17:28:23.000000000 +0200
+++ linux-2.6/drivers/md/dm-verity-target.c	2025-11-14 15:23:23.000000000 +0100
@@ -1690,7 +1690,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, 12, 0},
+	.version	= {1, 13, 0},
 	.module		= THIS_MODULE,
 	.ctr		= verity_ctr,
 	.dtr		= verity_dtr,

Re: [RFC PATCH] dm-verity: disable recursive forward error correction

[Sami Tolvanen]

Hi Mikulas,

On Fri, Nov 14, 2025 at 7:54 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> Hi
>
> What do you think about this patch?
>
> There are two problems with the recursive correction:
>
> 1. It may cause denial-of-service. In fec_read_bufs, there is a loop that
> has 253 iterations. For each iteration, we may call verity_hash_for_block
> recursively. There is a limit of 4 nested recursions - that means that
> there may be at most 253^4 (4 billion) iterations. Red Hat QE team
> actually created an image that that pushes dm-verity to this limit - and
> this image just makes the udev-worker process get stuck in the 'D' state.
>
> 2. It probably doesn't work. In fec_read_bufs we store data into the
> variable "fio->bufs", but fio bufs is shared between recursive
> invocations, if "verity_hash_for_block" invoked correction recursively, it
> would overwrite partially filled fio->bufs.
>
> So, I'm suggesting to remove recursive correction at all. This patch
> passed the cryptsetup testsuite. Does it pass your tests too?
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Reported-by: Guangwu Zhang <guazhang@redhat.com>

This looks reasonable to me. I only remember seeing recursive error
correction trigger in a real system when someone attempts to use the
wrong root hash, which is obviously not recoverable.

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami

Re: [RFC PATCH] dm-verity: disable recursive forward error correction

[Mikulas Patocka]

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



On Mon, 17 Nov 2025, Sami Tolvanen wrote:

> Hi Mikulas,
> 
> On Fri, Nov 14, 2025 at 7:54 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> > Hi
> >
> > What do you think about this patch?
> >
> > There are two problems with the recursive correction:
> >
> > 1. It may cause denial-of-service. In fec_read_bufs, there is a loop that
> > has 253 iterations. For each iteration, we may call verity_hash_for_block
> > recursively. There is a limit of 4 nested recursions - that means that
> > there may be at most 253^4 (4 billion) iterations. Red Hat QE team
> > actually created an image that that pushes dm-verity to this limit - and
> > this image just makes the udev-worker process get stuck in the 'D' state.
> >
> > 2. It probably doesn't work. In fec_read_bufs we store data into the
> > variable "fio->bufs", but fio bufs is shared between recursive
> > invocations, if "verity_hash_for_block" invoked correction recursively, it
> > would overwrite partially filled fio->bufs.
> >
> > So, I'm suggesting to remove recursive correction at all. This patch
> > passed the cryptsetup testsuite. Does it pass your tests too?
> >
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > Reported-by: Guangwu Zhang <guazhang@redhat.com>
> 
> This looks reasonable to me. I only remember seeing recursive error
> correction trigger in a real system when someone attempts to use the
> wrong root hash, which is obviously not recoverable.
> 
> Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
> 
> Sami

Thanks, OK.

I staged the patch for 6.19.

Mikulas

Re: [RFC PATCH] dm-verity: disable recursive forward error correction

[Eric Biggers]

On Fri, Nov 14, 2025 at 04:54:01PM +0100, Mikulas Patocka wrote:
> Hi
> 
> What do you think about this patch?
> 
> There are two problems with the recursive correction:
> 
> 1. It may cause denial-of-service. In fec_read_bufs, there is a loop that 
> has 253 iterations. For each iteration, we may call verity_hash_for_block 
> recursively. There is a limit of 4 nested recursions - that means that 
> there may be at most 253^4 (4 billion) iterations. Red Hat QE team 
> actually created an image that that pushes dm-verity to this limit - and 
> this image just makes the udev-worker process get stuck in the 'D' state.
> 
> 2. It probably doesn't work. In fec_read_bufs we store data into the 
> variable "fio->bufs", but fio bufs is shared between recursive 
> invocations, if "verity_hash_for_block" invoked correction recursively, it 
> would overwrite partially filled fio->bufs.
> 
> So, I'm suggesting to remove recursive correction at all. This patch
> passed the cryptsetup testsuite. Does it pass your tests too?
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Reported-by: Guangwu Zhang <guazhang@redhat.com>
> 
> ---
>  drivers/md/dm-verity-fec.c    |    4 +---
>  drivers/md/dm-verity-fec.h    |    3 ---
>  drivers/md/dm-verity-target.c |    2 +-
>  3 files changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Eric Biggers <ebiggers@kernel.org>

- Eric