crypt_dec_pending reads io->error before calling atomic_dec_and_test.
Another context, for example crypt_endio called from an interrupt, may
set io->error and drop its reference between the read and the
decrement. crypt_dec_pending then drops the last reference and completes
the bio with the stale status - so a read that failed and was never
decrypted, or a write that failed, is reported as successful.
The read was placed before the decrement by commit b35f8caa0890 ("dm
crypt: wait for endio to complete before destruction"), because that
commit freed dm_crypt_io before calling bio_endio. This is no longer the
case, dm_crypt_io lives in the per-bio data now.
Read io->error after atomic_dec_and_test instead. atomic_dec_and_test is
fully ordered, so no additional barrier is needed.
Fixes: b35f8caa0890 ("dm crypt: wait for endio to complete before destruction")
Cc: [email protected]
Reviewed-by: Jose Fernandez (Anthropic) <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Assisted-by: Claude:unspecified
---
Found by inspection. On x86-64 the load and the lock decl are adjacent
instructions, so in practice this needs an interrupt or preemption on
the CPU doing the final crypt_dec_pending.
It could only be demonstrated with an mdelay() inserted between the two
in kcryptd context: with dm-crypt on top of the "error" target, 20 of
20 failed direct writes were then reported as successful, and 0 of 20
with this patch and the same delay.
The affected pairs include kcryptd_io_read_work vs the read clone's
crypt_endio (deferred or recheck read), kcryptd_crypt_write_convert vs
the write clone's crypt_endio, and with an async cipher the read and
write convert paths vs kcryptd_async_done.
---
drivers/md/dm-crypt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 608b617fb817f..9e170de50ad32 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1745,7 +1745,6 @@ static void crypt_dec_pending(struct dm_crypt_io *io)
{
struct crypt_config *cc = io->cc;
struct bio *base_bio = io->base_bio;
- blk_status_t error = io->error;
if (!atomic_dec_and_test(&io->io_pending))
return;
@@ -1767,7 +1766,7 @@ static void crypt_dec_pending(struct dm_crypt_io *io)
else
kfree(io->integrity_metadata);
- base_bio->bi_status = error;
+ base_bio->bi_status = io->error;
bio_endio(base_bio);
}
---
base-commit: 39c5aa3bd8ec3912d2cd0b3fe092642b0d2b0713
change-id: 20260825-dm-crypt-dec-pending-467e8ca9765e
