diff mbox series

[net] tipc: increment the tmp aead refcnt before attaching it

Message ID c273cb4165a007c0125fac044def1416bd302fc7.1617677123.git.lucien.xin@gmail.com (mailing list archive)
State Accepted
Commit 2a2403ca3add03f542f6b34bef9f74649969b06d
Delegated to: Netdev Maintainers
Headers show
Series [net] tipc: increment the tmp aead refcnt before attaching it | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 17 this patch: 16
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 14 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 17 this patch: 16
netdev/header_inline success Link

Commit Message

Xin Long April 6, 2021, 2:45 a.m. UTC
Li Shuang found a NULL pointer dereference crash in her testing:

  [] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
  [] RIP: 0010:tipc_crypto_rcv_complete+0xc8/0x7e0 [tipc]
  [] Call Trace:
  []  <IRQ>
  []  tipc_crypto_rcv+0x2d9/0x8f0 [tipc]
  []  tipc_rcv+0x2fc/0x1120 [tipc]
  []  tipc_udp_recv+0xc6/0x1e0 [tipc]
  []  udpv6_queue_rcv_one_skb+0x16a/0x460
  []  udp6_unicast_rcv_skb.isra.35+0x41/0xa0
  []  ip6_protocol_deliver_rcu+0x23b/0x4c0
  []  ip6_input+0x3d/0xb0
  []  ipv6_rcv+0x395/0x510
  []  __netif_receive_skb_core+0x5fc/0xc40

This is caused by NULL returned by tipc_aead_get(), and then crashed when
dereferencing it later in tipc_crypto_rcv_complete(). This might happen
when tipc_crypto_rcv_complete() is called by two threads at the same time:
the tmp attached by tipc_crypto_key_attach() in one thread may be released
by the one attached by that in the other thread.

This patch is to fix it by incrementing the tmp's refcnt before attaching
it instead of calling tipc_aead_get() after attaching it.

Fixes: fc1b6d6de220 ("tipc: introduce TIPC encryption & authentication")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/tipc/crypto.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

patchwork-bot+netdevbpf@kernel.org April 6, 2021, 11:30 p.m. UTC | #1
Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Tue,  6 Apr 2021 10:45:23 +0800 you wrote:
> Li Shuang found a NULL pointer dereference crash in her testing:
> 
>   [] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
>   [] RIP: 0010:tipc_crypto_rcv_complete+0xc8/0x7e0 [tipc]
>   [] Call Trace:
>   []  <IRQ>
>   []  tipc_crypto_rcv+0x2d9/0x8f0 [tipc]
>   []  tipc_rcv+0x2fc/0x1120 [tipc]
>   []  tipc_udp_recv+0xc6/0x1e0 [tipc]
>   []  udpv6_queue_rcv_one_skb+0x16a/0x460
>   []  udp6_unicast_rcv_skb.isra.35+0x41/0xa0
>   []  ip6_protocol_deliver_rcu+0x23b/0x4c0
>   []  ip6_input+0x3d/0xb0
>   []  ipv6_rcv+0x395/0x510
>   []  __netif_receive_skb_core+0x5fc/0xc40
> 
> [...]

Here is the summary with links:
  - [net] tipc: increment the tmp aead refcnt before attaching it
    https://git.kernel.org/netdev/net/c/2a2403ca3add

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c
index f4fca8f..97710ce 100644
--- a/net/tipc/crypto.c
+++ b/net/tipc/crypto.c
@@ -1941,12 +1941,13 @@  static void tipc_crypto_rcv_complete(struct net *net, struct tipc_aead *aead,
 			goto rcv;
 		if (tipc_aead_clone(&tmp, aead) < 0)
 			goto rcv;
+		WARN_ON(!refcount_inc_not_zero(&tmp->refcnt));
 		if (tipc_crypto_key_attach(rx, tmp, ehdr->tx_key, false) < 0) {
 			tipc_aead_free(&tmp->rcu);
 			goto rcv;
 		}
 		tipc_aead_put(aead);
-		aead = tipc_aead_get(tmp);
+		aead = tmp;
 	}
 
 	if (unlikely(err)) {