From 86c3a3e964d910a62eeb277d60b2a60ebefa9feb Mon Sep 17 00:00:00 2001 From: Tadeusz Struk Date: Thu, 11 Nov 2021 12:59:16 -0800 Subject: tipc: use consistent GFP flags Some functions, like tipc_crypto_start use inconsisten GFP flags when allocating memory. The mentioned function use GFP_ATOMIC to to alloc a crypto instance, and then calls alloc_ordered_workqueue() which allocates memory with GFP_KERNEL. tipc_aead_init() function even uses GFP_KERNEL and GFP_ATOMIC interchangeably. No doc comment specifies what context a function is designed to work in, but the flags should at least be consistent within a function. Cc: Jon Maloy Cc: Ying Xue Cc: "David S. Miller" Cc: Jakub Kicinski Cc: netdev@vger.kernel.org Cc: tipc-discussion@lists.sourceforge.net Cc: linux-kernel@vger.kernel.org Signed-off-by: Tadeusz Struk Signed-off-by: David S. Miller --- net/tipc/crypto.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'net/tipc') diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c index dc60c32bb70d..e701651f6533 100644 --- a/net/tipc/crypto.c +++ b/net/tipc/crypto.c @@ -524,7 +524,7 @@ static int tipc_aead_init(struct tipc_aead **aead, struct tipc_aead_key *ukey, return -EEXIST; /* Allocate a new AEAD */ - tmp = kzalloc(sizeof(*tmp), GFP_ATOMIC); + tmp = kzalloc(sizeof(*tmp), GFP_KERNEL); if (unlikely(!tmp)) return -ENOMEM; @@ -1470,7 +1470,7 @@ int tipc_crypto_start(struct tipc_crypto **crypto, struct net *net, return -EEXIST; /* Allocate crypto */ - c = kzalloc(sizeof(*c), GFP_ATOMIC); + c = kzalloc(sizeof(*c), GFP_KERNEL); if (!c) return -ENOMEM; @@ -1484,7 +1484,7 @@ int tipc_crypto_start(struct tipc_crypto **crypto, struct net *net, } /* Allocate statistic structure */ - c->stats = alloc_percpu_gfp(struct tipc_crypto_stats, GFP_ATOMIC); + c->stats = alloc_percpu(struct tipc_crypto_stats); if (!c->stats) { if (c->wq) destroy_workqueue(c->wq); @@ -2457,7 +2457,7 @@ static void tipc_crypto_work_tx(struct work_struct *work) } /* Lets duplicate it first */ - skey = kmemdup(aead->key, tipc_aead_key_size(aead->key), GFP_ATOMIC); + skey = kmemdup(aead->key, tipc_aead_key_size(aead->key), GFP_KERNEL); rcu_read_unlock(); /* Now, generate new key, initiate & distribute it */ -- cgit From 271351d255b09e39c7f6437738cba595f9b235be Mon Sep 17 00:00:00 2001 From: Xin Long Date: Mon, 15 Nov 2021 07:45:24 -0500 Subject: tipc: only accept encrypted MSG_CRYPTO msgs The MSG_CRYPTO msgs are always encrypted and sent to other nodes for keys' deployment. But when receiving in peers, if those nodes do not validate it and make sure it's encrypted, one could craft a malicious MSG_CRYPTO msg to deploy its key with no need to know other nodes' keys. This patch is to do that by checking TIPC_SKB_CB(skb)->decrypted and discard it if this packet never got decrypted. Note that this is also a supplementary fix to CVE-2021-43267 that can be triggered by an unencrypted malicious MSG_CRYPTO msg. Fixes: 1ef6f7c9390f ("tipc: add automatic session key exchange") Acked-by: Ying Xue Acked-by: Jon Maloy Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/tipc/link.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'net/tipc') diff --git a/net/tipc/link.c b/net/tipc/link.c index 1b7a487c8841..09ae8448f394 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -1298,8 +1298,11 @@ static bool tipc_data_input(struct tipc_link *l, struct sk_buff *skb, return false; #ifdef CONFIG_TIPC_CRYPTO case MSG_CRYPTO: - tipc_crypto_msg_rcv(l->net, skb); - return true; + if (TIPC_SKB_CB(skb)->decrypted) { + tipc_crypto_msg_rcv(l->net, skb); + return true; + } + fallthrough; #endif default: pr_warn("Dropping received illegal msg type\n"); -- cgit From 3e6db079751afd527bf3db32314ae938dc571916 Mon Sep 17 00:00:00 2001 From: Tadeusz Struk Date: Mon, 15 Nov 2021 08:01:43 -0800 Subject: tipc: check for null after calling kmemdup kmemdup can return a null pointer so need to check for it, otherwise the null key will be dereferenced later in tipc_crypto_key_xmit as can be seen in the trace [1]. Cc: tipc-discussion@lists.sourceforge.net Cc: stable@vger.kernel.org # 5.15, 5.14, 5.10 [1] https://syzkaller.appspot.com/bug?id=bca180abb29567b189efdbdb34cbf7ba851c2a58 Reported-by: Dmitry Vyukov Signed-off-by: Tadeusz Struk Acked-by: Ying Xue Acked-by: Jon Maloy Link: https://lore.kernel.org/r/20211115160143.5099-1-tadeusz.struk@linaro.org Signed-off-by: Jakub Kicinski --- net/tipc/crypto.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'net/tipc') diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c index e701651f6533..b4d9419a015b 100644 --- a/net/tipc/crypto.c +++ b/net/tipc/crypto.c @@ -597,6 +597,10 @@ static int tipc_aead_init(struct tipc_aead **aead, struct tipc_aead_key *ukey, tmp->cloned = NULL; tmp->authsize = TIPC_AES_GCM_TAG_SIZE; tmp->key = kmemdup(ukey, tipc_aead_key_size(ukey), GFP_KERNEL); + if (!tmp->key) { + tipc_aead_free(&tmp->rcu); + return -ENOMEM; + } memcpy(&tmp->salt, ukey->key + keylen, TIPC_AES_GCM_SALT_SIZE); atomic_set(&tmp->users, 0); atomic64_set(&tmp->seqno, 0); -- cgit