diff mbox series

[net] tipc: do not write skb_shinfo frags when doing decrytion

Message ID 453b10a48c21d1882bbee21fe2c84197faad75e1.1627080361.git.lucien.xin@gmail.com (mailing list archive)
State Accepted
Commit 3cf4375a090473d240281a0d2b04a3a5aaeac34b
Delegated to: Netdev Maintainers
Headers show
Series [net] tipc: do not write skb_shinfo frags when doing decrytion | 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 fail 1 blamed authors not CCed: ying.xue@windriver.com; 1 maintainers not CCed: ying.xue@windriver.com
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: 0 this patch: 0
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, 20 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Xin Long July 23, 2021, 10:46 p.m. UTC
One skb's skb_shinfo frags are not writable, and they can be shared with
other skbs' like by pskb_copy(). To write the frags may cause other skb's
data crash.

So before doing en/decryption, skb_cow_data() should always be called for
a cloned or nonlinear skb if req dst is using the same sg as req src.
While at it, the likely branch can be removed, as it will be covered
by skb_cow_data().

Note that esp_input() has the same issue, and I will fix it in another
patch. tipc_aead_encrypt() doesn't have this issue, as it only processes
linear data in the unlikely branch.

Fixes: fc1b6d6de220 ("tipc: introduce TIPC encryption & authentication")
Reported-by: Shuang Li <shuali@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Jon Maloy <jmaloy@redhat.com>
---
 net/tipc/crypto.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

patchwork-bot+netdevbpf@kernel.org July 24, 2021, 6:50 p.m. UTC | #1
Hello:

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

On Fri, 23 Jul 2021 18:46:01 -0400 you wrote:
> One skb's skb_shinfo frags are not writable, and they can be shared with
> other skbs' like by pskb_copy(). To write the frags may cause other skb's
> data crash.
> 
> So before doing en/decryption, skb_cow_data() should always be called for
> a cloned or nonlinear skb if req dst is using the same sg as req src.
> While at it, the likely branch can be removed, as it will be covered
> by skb_cow_data().
> 
> [...]

Here is the summary with links:
  - [net] tipc: do not write skb_shinfo frags when doing decrytion
    https://git.kernel.org/netdev/net/c/3cf4375a0904

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 e5c43d4d5a75..c9391d38de85 100644
--- a/net/tipc/crypto.c
+++ b/net/tipc/crypto.c
@@ -898,16 +898,10 @@  static int tipc_aead_decrypt(struct net *net, struct tipc_aead *aead,
 	if (unlikely(!aead))
 		return -ENOKEY;
 
-	/* Cow skb data if needed */
-	if (likely(!skb_cloned(skb) &&
-		   (!skb_is_nonlinear(skb) || !skb_has_frag_list(skb)))) {
-		nsg = 1 + skb_shinfo(skb)->nr_frags;
-	} else {
-		nsg = skb_cow_data(skb, 0, &unused);
-		if (unlikely(nsg < 0)) {
-			pr_err("RX: skb_cow_data() returned %d\n", nsg);
-			return nsg;
-		}
+	nsg = skb_cow_data(skb, 0, &unused);
+	if (unlikely(nsg < 0)) {
+		pr_err("RX: skb_cow_data() returned %d\n", nsg);
+		return nsg;
 	}
 
 	/* Allocate memory for the AEAD operation */