From: Mikulas Patocka mpatocka@redhat.com
[ Upstream commit d9f3e47d3fae0c101d9094bc956ed24e7a0ee801 ]
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 pushes dm-verity to this limit - and this image just makes the udev-worker process get stuck in the 'D' state.
2. It 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.
Signed-off-by: Mikulas Patocka mpatocka@redhat.com Reported-by: Guangwu Zhang guazhang@redhat.com Reviewed-by: Sami Tolvanen samitolvanen@google.com Reviewed-by: Eric Biggers ebiggers@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Summary and Recommendation
### What Problem This Commit Solves
This commit fixes **two critical bugs** in dm-verity's Forward Error Correction (FEC):
1. **Denial-of-Service vulnerability**: The recursive FEC allows 4 levels of nesting with 253 iterations per level, resulting in up to 253^4 (~4 billion) potential iterations. Red Hat QE demonstrated this causes the `udev-worker` process to hang in uninterruptible 'D' state.
2. **Data corruption bug**: The `fio->bufs` buffer is shared across all recursion levels. When `verity_hash_for_block` triggers nested FEC correction, it corrupts partially-filled buffers from outer levels. The recursive FEC feature fundamentally doesn't work.
### Stable Kernel Criteria Assessment
| Criterion | Assessment | |-----------|------------| | Obviously correct | ✅ YES - Simple condition change that completely disables broken recursion | | Fixes real bug | ✅ YES - DoS and data corruption, reproducible by Red Hat QE | | Important issue | ✅ YES - System hang (DoS), affects Android/Chromebook verified boot | | Small and contained | ✅ YES - ~20 lines across 3 files, removes code rather than adding | | No new features | ✅ YES - Removes broken functionality |
### Risk vs Benefit Analysis
**Benefits:** - Eliminates a system-hang DoS vulnerability - Fixes a data corruption bug in FEC recovery - Affects widely-deployed dm-verity users (Android, Chromebooks, verified boot systems) - Conservative fix - disables broken feature rather than complex repair
**Risks:** - Minimal - the recursive FEC was fundamentally broken anyway - Version bump (1.12→1.13) is cosmetic; documents behavioral change - Theoretical: some error correction scenarios may not work, but they were already broken
### Additional Considerations
- **Reviewers**: Sami Tolvanen (Google) and Eric Biggers (kernel crypto expert) - strong vetting - **Author**: Mikulas Patocka, dm subsystem maintainer - **Bug origin**: FEC feature added in 2015 (commit a739ff3f543af), so affects all current LTS kernels - **Dependencies**: Self-contained, should apply cleanly to stable trees - **No explicit `Cc: stable`**: But severity and fix quality strongly support backporting
### Conclusion
This is an excellent stable candidate. It fixes a proven DoS vulnerability and data corruption bug in security-critical dm-verity infrastructure. The fix is minimal, conservative (disables rather than patches), well-reviewed by domain experts, and authored by the subsystem maintainer. The affected FEC recursion feature was broken since introduction, so removing it has no practical downside. The user impact is high given dm-verity's deployment in Android and other verified boot systems.
**YES**
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(-)
diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c index 72047b47a7a0a..e41bde1d3b15b 100644 --- a/drivers/md/dm-verity-fec.c +++ b/drivers/md/dm-verity-fec.c @@ -413,10 +413,8 @@ int verity_fec_decode(struct dm_verity *v, struct dm_verity_io *io, 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++;
diff --git a/drivers/md/dm-verity-fec.h b/drivers/md/dm-verity-fec.h index 09123a6129538..ec37e607cb3f0 100644 --- a/drivers/md/dm-verity-fec.h +++ b/drivers/md/dm-verity-fec.h @@ -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" diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 66a00a8ccb398..c8695c079cfe0 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -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,