linux-kernel - [PATCH 3.16 221/328] dm: disable CRYPTO_TFM_REQ_MAY_SLEEP to fix a GFP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1544392233.663922937@decadent.org.uk>
Date:   Sun, 09 Dec 2018 21:50:33 +0000
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, "Mike Snitzer" <snitzer@...hat.com>,
        "Mikulas Patocka" <mpatocka@...hat.com>
Subject: [PATCH 3.16 221/328] dm: disable CRYPTO_TFM_REQ_MAY_SLEEP to fix
 a GFP_KERNEL recursion deadlock

3.16.62-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Mikulas Patocka <mpatocka@...hat.com>

commit 432061b3da64e488be3403124a72a9250bbe96d4 upstream.

There's a XFS on dm-crypt deadlock, recursing back to itself due to the
crypto subsystems use of GFP_KERNEL, reported here:
https://bugzilla.kernel.org/show_bug.cgi?id=200835

* dm-crypt calls crypt_convert in xts mode
* init_crypt from xts.c calls kmalloc(GFP_KERNEL)
* kmalloc(GFP_KERNEL) recurses into the XFS filesystem, the filesystem
	tries to submit some bios and wait for them, causing a deadlock

Fix this by updating both the DM crypt and integrity targets to no
longer use the CRYPTO_TFM_REQ_MAY_SLEEP flag, which will change the
crypto allocations from GFP_KERNEL to GFP_ATOMIC, therefore they can't
recurse into a filesystem.  A GFP_ATOMIC allocation can fail, but
init_crypt() in xts.c handles the allocation failure gracefully - it
will fall back to preallocated buffer if the allocation fails.

The crypto API maintainer says that the crypto API only needs to
allocate memory when dealing with unaligned buffers and therefore
turning CRYPTO_TFM_REQ_MAY_SLEEP off is safe (see this discussion:
https://www.redhat.com/archives/dm-devel/2018-August/msg00195.html )

Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
Signed-off-by: Mike Snitzer <snitzer@...hat.com>
[bwh: Backported to 3.16:
 - Drop change to crypt_alloc_req_aead() in dm-crypt
 - Drop changes to dm-integrity
 - Adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -262,7 +262,7 @@ static int crypt_iv_essiv_init(struct cr
 
 	sg_init_one(&sg, cc->key, cc->key_size);
 	desc.tfm = essiv->hash_tfm;
-	desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+	desc.flags = 0;
 
 	err = crypto_hash_digest(&desc, &sg, cc->key_size, essiv->salt);
 	if (err)
@@ -533,7 +533,7 @@ static int crypt_iv_lmk_one(struct crypt
 	int i, r;
 
 	sdesc.desc.tfm = lmk->hash_tfm;
-	sdesc.desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+	sdesc.desc.flags = 0;
 
 	r = crypto_shash_init(&sdesc.desc);
 	if (r)
@@ -690,7 +690,7 @@ static int crypt_iv_tcw_whitening(struct
 
 	/* calculate crc32 for every 32bit part and xor it */
 	sdesc.desc.tfm = tcw->crc32_tfm;
-	sdesc.desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+	sdesc.desc.flags = 0;
 	for (i = 0; i < 4; i++) {
 		r = crypto_shash_init(&sdesc.desc);
 		if (r)
@@ -891,7 +891,7 @@ static void crypt_alloc_req(struct crypt
 
 	ablkcipher_request_set_tfm(ctx->req, cc->tfms[key_index]);
 	ablkcipher_request_set_callback(ctx->req,
-	    CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
+	    CRYPTO_TFM_REQ_MAY_BACKLOG,
 	    kcryptd_async_done, dmreq_of_req(cc, ctx->req));
 }