mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] dm: avoid leaking the caller's thread keyring via the table device file
@ 2026-06-17 14:33 Ingo Blechschmidt
  2026-06-23  8:56 ` Ondrej Kozina
  0 siblings, 1 reply; 2+ messages in thread
From: Ingo Blechschmidt @ 2026-06-17 14:33 UTC (permalink / raw)
  To: Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski
  Cc: dm-devel, linux-kernel

The refactoring in commit a28d893eb327 ("md: port block device access to file")
accidentally causes the caller's thread keyring to be kept alive long
beyond the caller's lifetime.

As a result, "cryptsetup luksSuspend" silently fails to wipe the
LUKS volume key from memory.

In detail: "cryptsetup luksOpen" uses its supposedly ephemeral thread
keyring to pass the volume key to the kernel. dm-crypt's
crypt_set_keyring_key() copies the key material into its own
crypt_config structure and then drops its own reference to the key in
the keyring with key_put().

With this fix, restoring pre-v6.9 behavior, the copy in the thread
keyring is then promptly garbage collected, such that exactly one copy
of the volume key remains. This single copy is correctly wiped from
memory on "cryptsetup luksSuspend".

Without this fix, the thread keyring and the volume key in it remains.
This second copy is only freed on "luksClose". "luksSuspend" neither
knows about this copy nor has any way to remove it, so the key remains
recoverable from RAM after a suspend that is documented to have wiped it.

This fix should not introduce new security problems, as the code is
anyway gated by CAP_SYS_ADMIN. The device-mapper core, not the calling
task, is the legitimate owner of this long-lived file.

Fixes: a28d893eb327 ("md: port block device access to file")
Closes: https://gitlab.com/cryptsetup/cryptsetup/-/work_items/993
Link: https://www.speicherleck.de/iblech/cryptsetup-luksSuspend-issue-reproduction/
Signed-off-by: Ingo Blechschmidt <iblech@speicherleck.de>
Cc: stable@vger.kernel.org
---
This is only my second kernel patch; I don't have a complete grasp of
the potential long-range interactions: This changes the credentials used
to open the backing device for *all* table devices, not just dm-crypt's.
I roughly followed the similar pattern in drivers/block/nbd.c, where
scoped_with_kernel_creds is used for dealing with the socket. Please
exercise extra caution. Happy to revise.

 drivers/md/dm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 7287bed6eb64..d413bfaf3527 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -735,7 +735,16 @@ static struct table_device *open_table_device(struct mapped_device *md,
 		return ERR_PTR(-ENOMEM);
 	refcount_set(&td->count, 1);
 
-	bdev_file = bdev_file_open_by_dev(dev, mode, _dm_claim_ptr, NULL);
+	/*
+	 * Open the backing device with kernel rather than caller
+	 * credentials. Otherwise the caller's credentials would be
+	 * pinned in bdev_file->f_cred until the table device is closed.
+	 * That would keep the caller's thread keyring alive long beyond the
+	 * lifetime of the caller, breaking userspace expectation (e.g.
+	 * cryptsetup(8) leaking the LUKS volume key).
+	 */
+	scoped_with_kernel_creds()
+		bdev_file = bdev_file_open_by_dev(dev, mode, _dm_claim_ptr, NULL);
 	if (IS_ERR(bdev_file)) {
 		r = PTR_ERR(bdev_file);
 		goto out_free_td;
-- 
2.43.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] dm: avoid leaking the caller's thread keyring via the table device file
  2026-06-17 14:33 [PATCH] dm: avoid leaking the caller's thread keyring via the table device file Ingo Blechschmidt
@ 2026-06-23  8:56 ` Ondrej Kozina
  0 siblings, 0 replies; 2+ messages in thread
From: Ondrej Kozina @ 2026-06-23  8:56 UTC (permalink / raw)
  To: Ingo Blechschmidt, Alasdair Kergon, Mike Snitzer,
	Mikulas Patocka, Benjamin Marzinski
  Cc: dm-devel, linux-kernel, gmazyland

Hi,

On 17/06/2026 16:33, Ingo Blechschmidt wrote:
> The refactoring in commit a28d893eb327 ("md: port block device access to file")
> accidentally causes the caller's thread keyring to be kept alive long
> beyond the caller's lifetime.

For the record for anyone tracking the issue interdependently.

I have tested the patch and can ack it fixes the issue for block devices 
opened in DM table constructors. But that's not, unfortunately, the root 
cause of the issue. The keys put in caller's thread keyring can be 
pinned via many kernel APIs taking their own reference on current_cred() 
when a process calls into them. That's something I missed when I added 
the feature in libcryptsetup.

For example: while I was testing the patch I got hit by the issue again 
when I had test LUKS2 image in a file. The cryptsetup creates loop 
device in before it opens dm-crypt mapping over it. And voila the thread 
keyring got pinned again, via loop device.

So to fix it properly, I have opened MR also on the cryptsetup to 
manually release the keyring while releasing a crypt device context: 
https://gitlab.com/cryptsetup/cryptsetup/-/merge_requests/937

Tested-by: Ondrej Kozina <okozina@redhat.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-23  8:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-17 14:33 [PATCH] dm: avoid leaking the caller's thread keyring via the table device file Ingo Blechschmidt
2026-06-23  8:56 ` Ondrej Kozina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome