diff mbox series

tipc: fix size validations for the MSG_CRYPTO type

Message ID YXbN6S9KPq8S5N0v@kroah.com (mailing list archive)
State Accepted
Commit fa40d9734a57bcbfa79a280189799f76c88f7bb0
Delegated to: Netdev Maintainers
Headers show
Series tipc: fix size validations for the MSG_CRYPTO type | expand

Checks

Context Check Description
netdev/cover_letter success Single patches do not need cover letters
netdev/fixes_present success Fixes tag not required for -next series
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers warning 1 maintainers not CCed: tipc-discussion@lists.sourceforge.net
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Fixes tag looks correct
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success No static functions without inline keyword in header files

Commit Message

Greg Kroah-Hartman Oct. 25, 2021, 3:31 p.m. UTC
From: Max VA <maxv@sentinelone.com>

The function tipc_crypto_key_rcv is used to parse MSG_CRYPTO messages
to receive keys from other nodes in the cluster in order to decrypt any
further messages from them.
This patch verifies that any supplied sizes in the message body are
valid for the received message.

Fixes: 1ef6f7c9390f ("tipc: add automatic session key exchange")
Signed-off-by: Max VA <maxv@sentinelone.com>
Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/tipc/crypto.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

Max's email system doesn't seem to be able to send non-attachment
patches out, so I'm forwarding this on for him.  It's already acked by
one of the tipc maintainers.

Comments

Jon Maloy Oct. 26, 2021, 12:53 a.m. UTC | #1
Acked-by: Jon Maloy <jmaloy@redhat.com>

On 10/25/21 11:31, Greg KH wrote:
> From: Max VA <maxv@sentinelone.com>
>
> The function tipc_crypto_key_rcv is used to parse MSG_CRYPTO messages
> to receive keys from other nodes in the cluster in order to decrypt any
> further messages from them.
> This patch verifies that any supplied sizes in the message body are
> valid for the received message.
>
> Fixes: 1ef6f7c9390f ("tipc: add automatic session key exchange")
> Signed-off-by: Max VA <maxv@sentinelone.com>
> Acked-by: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   net/tipc/crypto.c | 32 +++++++++++++++++++++-----------
>   1 file changed, 21 insertions(+), 11 deletions(-)
>
> Max's email system doesn't seem to be able to send non-attachment
> patches out, so I'm forwarding this on for him.  It's already acked by
> one of the tipc maintainers.
>
> diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c
> index c9391d38de85..dc60c32bb70d 100644
> --- a/net/tipc/crypto.c
> +++ b/net/tipc/crypto.c
> @@ -2285,43 +2285,53 @@ static bool tipc_crypto_key_rcv(struct tipc_crypto *rx, struct tipc_msg *hdr)
>   	u16 key_gen = msg_key_gen(hdr);
>   	u16 size = msg_data_sz(hdr);
>   	u8 *data = msg_data(hdr);
> +	unsigned int keylen;
> +
> +	/* Verify whether the size can exist in the packet */
> +	if (unlikely(size < sizeof(struct tipc_aead_key) + TIPC_AEAD_KEYLEN_MIN)) {
> +		pr_debug("%s: message data size is too small\n", rx->name);
> +		goto exit;
> +	}
> +
> +	keylen = ntohl(*((__be32 *)(data + TIPC_AEAD_ALG_NAME)));
> +
> +	/* Verify the supplied size values */
> +	if (unlikely(size != keylen + sizeof(struct tipc_aead_key) ||
> +		     keylen > TIPC_AEAD_KEY_SIZE_MAX)) {
> +		pr_debug("%s: invalid MSG_CRYPTO key size\n", rx->name);
> +		goto exit;
> +	}
>   
>   	spin_lock(&rx->lock);
>   	if (unlikely(rx->skey || (key_gen == rx->key_gen && rx->key.keys))) {
>   		pr_err("%s: key existed <%p>, gen %d vs %d\n", rx->name,
>   		       rx->skey, key_gen, rx->key_gen);
> -		goto exit;
> +		goto exit_unlock;
>   	}
>   
>   	/* Allocate memory for the key */
>   	skey = kmalloc(size, GFP_ATOMIC);
>   	if (unlikely(!skey)) {
>   		pr_err("%s: unable to allocate memory for skey\n", rx->name);
> -		goto exit;
> +		goto exit_unlock;
>   	}
>   
>   	/* Copy key from msg data */
> -	skey->keylen = ntohl(*((__be32 *)(data + TIPC_AEAD_ALG_NAME)));
> +	skey->keylen = keylen;
>   	memcpy(skey->alg_name, data, TIPC_AEAD_ALG_NAME);
>   	memcpy(skey->key, data + TIPC_AEAD_ALG_NAME + sizeof(__be32),
>   	       skey->keylen);
>   
> -	/* Sanity check */
> -	if (unlikely(size != tipc_aead_key_size(skey))) {
> -		kfree(skey);
> -		skey = NULL;
> -		goto exit;
> -	}
> -
>   	rx->key_gen = key_gen;
>   	rx->skey_mode = msg_key_mode(hdr);
>   	rx->skey = skey;
>   	rx->nokey = 0;
>   	mb(); /* for nokey flag */
>   
> -exit:
> +exit_unlock:
>   	spin_unlock(&rx->lock);
>   
> +exit:
>   	/* Schedule the key attaching on this crypto */
>   	if (likely(skey && queue_delayed_work(tx->wq, &rx->work, 0)))
>   		return true;
patchwork-bot+netdevbpf@kernel.org Oct. 26, 2021, 12:50 p.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon, 25 Oct 2021 17:31:53 +0200 you wrote:
> From: Max VA <maxv@sentinelone.com>
> 
> The function tipc_crypto_key_rcv is used to parse MSG_CRYPTO messages
> to receive keys from other nodes in the cluster in order to decrypt any
> further messages from them.
> This patch verifies that any supplied sizes in the message body are
> valid for the received message.
> 
> [...]

Here is the summary with links:
  - tipc: fix size validations for the MSG_CRYPTO type
    https://git.kernel.org/netdev/net/c/fa40d9734a57

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c
index c9391d38de85..dc60c32bb70d 100644
--- a/net/tipc/crypto.c
+++ b/net/tipc/crypto.c
@@ -2285,43 +2285,53 @@  static bool tipc_crypto_key_rcv(struct tipc_crypto *rx, struct tipc_msg *hdr)
 	u16 key_gen = msg_key_gen(hdr);
 	u16 size = msg_data_sz(hdr);
 	u8 *data = msg_data(hdr);
+	unsigned int keylen;
+
+	/* Verify whether the size can exist in the packet */
+	if (unlikely(size < sizeof(struct tipc_aead_key) + TIPC_AEAD_KEYLEN_MIN)) {
+		pr_debug("%s: message data size is too small\n", rx->name);
+		goto exit;
+	}
+
+	keylen = ntohl(*((__be32 *)(data + TIPC_AEAD_ALG_NAME)));
+
+	/* Verify the supplied size values */
+	if (unlikely(size != keylen + sizeof(struct tipc_aead_key) ||
+		     keylen > TIPC_AEAD_KEY_SIZE_MAX)) {
+		pr_debug("%s: invalid MSG_CRYPTO key size\n", rx->name);
+		goto exit;
+	}
 
 	spin_lock(&rx->lock);
 	if (unlikely(rx->skey || (key_gen == rx->key_gen && rx->key.keys))) {
 		pr_err("%s: key existed <%p>, gen %d vs %d\n", rx->name,
 		       rx->skey, key_gen, rx->key_gen);
-		goto exit;
+		goto exit_unlock;
 	}
 
 	/* Allocate memory for the key */
 	skey = kmalloc(size, GFP_ATOMIC);
 	if (unlikely(!skey)) {
 		pr_err("%s: unable to allocate memory for skey\n", rx->name);
-		goto exit;
+		goto exit_unlock;
 	}
 
 	/* Copy key from msg data */
-	skey->keylen = ntohl(*((__be32 *)(data + TIPC_AEAD_ALG_NAME)));
+	skey->keylen = keylen;
 	memcpy(skey->alg_name, data, TIPC_AEAD_ALG_NAME);
 	memcpy(skey->key, data + TIPC_AEAD_ALG_NAME + sizeof(__be32),
 	       skey->keylen);
 
-	/* Sanity check */
-	if (unlikely(size != tipc_aead_key_size(skey))) {
-		kfree(skey);
-		skey = NULL;
-		goto exit;
-	}
-
 	rx->key_gen = key_gen;
 	rx->skey_mode = msg_key_mode(hdr);
 	rx->skey = skey;
 	rx->nokey = 0;
 	mb(); /* for nokey flag */
 
-exit:
+exit_unlock:
 	spin_unlock(&rx->lock);
 
+exit:
 	/* Schedule the key attaching on this crypto */
 	if (likely(skey && queue_delayed_work(tx->wq, &rx->work, 0)))
 		return true;