RE: [PATCH net v2] tipc: fix slab-use-after-free Read in tipc_aead_decrypt_done

[Tung Quang Nguyen]

>-----Original Message-----
>From: Doruk Tan Ozturk <doruk@xxxxxxx>
>Subject: [PATCH net v2] tipc: fix slab-use-after-free Read in
>tipc_aead_decrypt_done
>
>The async decrypt completion path mirrors the encrypt one but is missing the
>net reference that guards against the tipc_crypto being freed during netns
>teardown.
>
>When crypto_aead_decrypt() is offloaded to cryptd (the SIMD aead wrapper
>queues the request when crypto_simd_usable() is false), the cryptd worker
>runs tipc_aead_decrypt_done() asynchronously. If the bearer's netns is torn
>down in the meantime, tipc_exit_net() -> tipc_crypto_stop() frees the
>tipc_crypto, and the completion then reads it: tipc_aead_decrypt_done()
>dereferences aead->crypto->stats and aead->crypto->net, and
>tipc_crypto_rcv_complete() dereferences aead->crypto->aead[] and the node
>table, reading freed memory:
>
>  BUG: KASAN: slab-use-after-free in tipc_crypto_rcv_complete
>  Read of size 8 at addr ffff888104c8c808 by task kworker/3:2/70
>  Workqueue: cryptd cryptd_queue_worker
>  Call Trace:
>   tipc_crypto_rcv_complete+0x1dd6/0x2240
>   tipc_aead_decrypt_done+0x1c3/0x300
>   cryptd_aead_crypt+0x3ae/0x660
>   cryptd_queue_worker+0x12b/0x200
>   process_one_work+0x66c/0x10c0
>   worker_thread+0x55d/0xc80
>   kthread+0x269/0x340
>  Allocated by task 1550:
>   tipc_crypto_start+0x7e/0x890
>   tipc_init_net+0x30d/0x480
>   ...
>  Freed by task 116:
>   tipc_crypto_stop+0x1a4/0x2a0
>   tipc_exit_net+0x11c/0x1c0
>   cleanup_net+0x510/0xaf0
>

Can you decode above stack trace (using linux/scripts/decode_stacktrace.sh) for more readable text ?

>This is the same class of bug that commit e279024617134 ("net/tipc: fix slab-
>use-after-free Read in tipc_aead_encrypt_done") fixed for the encrypt side.
>The encrypt path takes maybe_get_net(aead->crypto->net) before
>crypto_aead_encrypt() and drops it with put_net() on the synchronous return
>paths and in tipc_aead_encrypt_done(); the -EINPROGRESS /-EBUSY return
>keeps the reference for the async callback to release. The decrypt path was left
>without the equivalent guard.
>
>Mirror the encrypt-side fix on the decrypt path: take a net reference before
>crypto_aead_decrypt() (failing with -ENODEV and the matching bearer put if it
>cannot be acquired), keep it across the -EINPROGRESS/ -EBUSY async return,
>and drop it with put_net() on the synchronous success/error return and at the
>end of tipc_aead_decrypt_done().
>
>Reproduced under KASAN on v6.12.92: a UDP bearer with a cluster key is

Is the issue reproducible on latest net branch or just on the old v6.12.92 as you mentioned ?

>flooded with encrypted frames from an unknown peer (driving the cluster- key
>decrypt path) while the bearer's netns is repeatedly torn down. The SIMD aead
>must be forced onto its cryptd async child for the completion to outlive
>tipc_crypto_stop(); with the patch applied the same workload runs cleanly.
>Found by 0sec automated security-research tooling while auditing the siblings
>of commit e279024617134.
>
>Found by 0sec automated security-research tooling (https://0sec.ai).
>
>Fixes: fc1b6d6de220 ("tipc: introduce TIPC encryption & authentication")
>Cc: stable@xxxxxxxxxxxxxxx
>Signed-off-by: Doruk Tan Ozturk <doruk@xxxxxxx>
>Reviewed-by: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx>
>---
>v2:
> - Add Cc: stable@xxxxxxxxxxxxxxx and Alexander Lobakin's Reviewed-by
>   (per his review of v1). No functional change.
>
> net/tipc/crypto.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c index
>6d3b6b89b1d1..84a6489da036 100644
>--- a/net/tipc/crypto.c
>+++ b/net/tipc/crypto.c
>@@ -941,12 +941,20 @@ static int tipc_aead_decrypt(struct net *net, struct
>tipc_aead *aead,
> 		goto exit;
> 	}
>
>+	/* Get net to avoid freed tipc_crypto when delete namespace */
>+	if (!maybe_get_net(aead->crypto->net)) {
>+		tipc_bearer_put(b);
>+		rc = -ENODEV;
>+		goto exit;
>+	}
>+
> 	/* Now, do decrypt */
> 	rc = crypto_aead_decrypt(req);
> 	if (rc == -EINPROGRESS || rc == -EBUSY)
> 		return rc;
>
> 	tipc_bearer_put(b);
>+	put_net(aead->crypto->net);
>
> exit:
> 	kfree(ctx);
>@@ -984,6 +992,7 @@ static void tipc_aead_decrypt_done(void *data, int err)
> 	}
>
> 	tipc_bearer_put(b);
>+	put_net(net);
> }
>
> static inline int tipc_ehdr_size(struct tipc_ehdr *ehdr)
>--
>2.43.0
>