Re: [PATCH] drbd: dynamically allocate shash descriptor
From: Roland Kammerer <hidden>
Date: 2019-06-17 14:36:39
Also in:
lkml
On Mon, Jun 17, 2019 at 03:24:13PM +0200, Arnd Bergmann wrote:
quoted hunk ↗ jump to hunk
Building with clang and KASAN, we get a warning about an overly large stack frame on 32-bit architectures: drivers/block/drbd/drbd_receiver.c:921:31: error: stack frame size of 1280 bytes in function 'conn_connect' [-Werror,-Wframe-larger-than=] We already allocate other data dynamically in this function, so just do the same for the shash descriptor, which makes up most of this memory. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/block/drbd/drbd_receiver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c index 90ebfcae0ce6..10fb26e862d7 100644 --- a/drivers/block/drbd/drbd_receiver.c +++ b/drivers/block/drbd/drbd_receiver.c@@ -5417,7 +5417,7 @@ static int drbd_do_auth(struct drbd_connection *connection) unsigned int key_len; char secret[SHARED_SECRET_MAX]; /* 64 byte */ unsigned int resp_size; - SHASH_DESC_ON_STACK(desc, connection->cram_hmac_tfm); + struct shash_desc *desc; struct packet_info pi; struct net_conf *nc; int err, rv;@@ -5430,6 +5430,13 @@ static int drbd_do_auth(struct drbd_connection *connection) memcpy(secret, nc->shared_secret, key_len); rcu_read_unlock(); + desc = kmalloc(sizeof(struct shash_desc) + + crypto_shash_descsize(connection->cram_hmac_tfm), + GFP_KERNEL); + if (!desc) { + rv = -1; + goto fail; + } desc->tfm = connection->cram_hmac_tfm; rv = crypto_shash_setkey(connection->cram_hmac_tfm, (u8 *)secret, key_len);@@ -5572,6 +5579,7 @@ static int drbd_do_auth(struct drbd_connection *connection) kfree(response); kfree(right_response); shash_desc_zero(desc); + kfree(desc); return rv; }
Hi Arnd, are you sure your cleanup is okay?
shash_desc_zero(desc); + kfree(desc);
You shash_desc_zero() a potential NULL pointer. memzero_expicit() in the function then dereferences it: memzero_explicit(desc, sizeof(*desc) + crypto_shash_descsize(desc->tfm)); Maybe some if (desc) guard? Best, rck