Skip to contentSkip to conversation

tls: rx: fix return value for async crypto

PATCH Applied Jakub Kicinski 4 participants 4 messages Feb 27, 2023 – Feb 28, 2023

Subsystems (3)
Networking Network drivers Networking TLS
lore
Share & tools
lore

Jakub Kicinski

author maintainer
#0 lore raw

PATCH 1/1 tls: rx: fix return value for async crypto

Gaurav reports that TLS Rx is broken with async crypto accelerators. The commit under fixes missed updating the retval byte counting logic when updating how records are stored. Even tho both before and after the change 'decrypted' was updated inside the main loop, it was completely overwritten when processing the async completions. Now that the rx_list only holds non-zero-copy records we need to add, not overwrite.

Reported-and-bisected-by Gaurav Jain <[email protected]>
Fixes cbbdee9918a2 ("tls: rx: async: don't put async zc on the list")
Link https://bugzilla.kernel.org/show_bug.cgi?id=217064
Signed-off-by Jakub Kicinski <[email protected]>

---
CC: [email protected]
CC: [email protected]

---
 net/tls/tls_sw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/tls/tls_sw.c +1 -1
index 782d3701b86f..021d760f9133 100644
@@ -2127,7 +2127,7 @@ int tls_sw_recvmsg(struct sock *sk,
 		else
 			err = process_rx_list(ctx, msg, &control, 0,
 					      async_copy_bytes, is_peek);
-		decrypted = max(err, 0);
+		decrypted += max(err, 0);
 	}
 
 	copied += decrypted;
Signature · 1 line
2.39.2
Tested-by on v1 1/1
#1 lore raw
Tested-by Gaurav Jain <[email protected]>
Quoted 51 lines

-----Original Message-----
From: Jakub Kicinski <[email protected]>
Sent: Monday, February 27, 2023 11:42 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; Gaurav Jain <[email protected]>; Jakub Kicinski <[email protected]>; [email protected]; [email protected]
Subject: [EXT] [PATCH net] tls: rx: fix return value for async crypto

Caution: EXT Email

Gaurav reports that TLS Rx is broken with async crypto accelerators. The commit under fixes missed updating the retval byte counting logic when updating how records are stored. Even tho both before and after the change 'decrypted' was updated inside the main loop, it was completely overwritten when processing the async completions. Now that the rx_list only holds non-zero-copy records we need to add, not overwrite.

Reported-and-bisected-by Gaurav Jain <[email protected]>
Fixes cbbdee9918a2 ("tls: rx: async: don't put async zc on the list")

Link:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.k
ernel.org%2Fshow_bug.cgi%3Fid%3D217064&data=05%7C01%7Cgaurav.jain%4 0nxp.com%7C77f145ef30784d369bca08db18ee26ab%7C686ea1d3bc2b4c6fa92 cd99c5c301635%7C0%7C0%7C638131183344515410%7CUnknown%7CTWFpbG Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn 0%3D%7C3000%7C%7C%7C&sdata=PQE9yFQ85lN8RvbgLwLIZmRbCzZOVcF8VB MEWTgi%2B4Q%3D&reserved=0

Signed-off-by Jakub Kicinski <[email protected]>

---
CC: [email protected]
CC: [email protected]

---
 net/tls/tls_sw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/tls/tls_sw.c index renamed from net/tls/tls_sw.c +0 -0

782d3701b86f..021d760f9133 100644

net/tls/tls_sw.c +1 -2
@@ -2127,7 +2127,7 @@ int tls_sw_recvmsg(struct sock *sk,
                else
                        err = process_rx_list(ctx, msg, &control, 0,
                                              async_copy_bytes, is_peek);
-               decrypted = max(err, 0);
+               decrypted += max(err, 0);
        }
 
        copied += decrypted;
--

2.39.2

Reviewed-by on v1 1/1
#2 lore raw
In reply to Jakub Kicinski
Quoted 9 lines · Jakub Kicinski

Gaurav reports that TLS Rx is broken with async crypto accelerators. The commit under fixes missed updating the retval byte counting logic when updating how records are stored. Even tho both before and after the change 'decrypted' was updated inside the main loop, it was completely overwritten when processing the async completions. Now that the rx_list only holds non-zero-copy records we need to add, not overwrite.

Quoted 4 lines
Reported-and-bisected-by Gaurav Jain <[email protected]>
Fixes cbbdee9918a2 ("tls: rx: async: don't put async zc on the list")
Link https://bugzilla.kernel.org/show_bug.cgi?id=217064
Signed-off-by Jakub Kicinski <[email protected]>
Reviewed-by Simon Horman <[email protected]>
CI Applied
#3 lore raw
In reply to Jakub Kicinski
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 10 lines · On Mon, 27 Feb 2023 10:12:01 -0800 you wrote:

Gaurav reports that TLS Rx is broken with async crypto accelerators. The commit under fixes missed updating the retval byte counting logic when updating how records are stored. Even tho both before and after the change 'decrypted' was updated inside the main loop, it was completely overwritten when processing the async completions. Now that the rx_list only holds non-zero-copy records we need to add, not overwrite.

[...]

Here is the summary with links:
- [net] tls: rx: fix return value for async crypto

    https://git.kernel.org/netdev/net/c/4d42cd6bc2ac

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html