[net,0/7] net: tls: fix some issues with async encryption

Message

Jakub Kicinski Feb. 7, 2024, 1:18 a.m. UTC

Hi!

valis was reporting a race on socket close so I sat down to try to fix it.
I used Sabrina's async crypto debug patch to test... and in the process
run into some of the same issues, and created very similar fixes :(
I didn't realize how many of those patches weren't applied. Once I found
Sabrina's code [1] it turned out to be so similar in fact that I added
her S-o-b's and Co-develop'eds in a semi-haphazard way.

With this series in place all expected tests pass with async crypto.
Sabrina had a few more fixes, but I'll leave those to her, things are
not crashing anymore.

[1] https://lore.kernel.org/netdev/[e-mail]/

Jakub Kicinski (6):
  net: tls: factor out tls_*crypt_async_wait()
  tls: fix race between async notify and socket close
  tls: fix race between tx work scheduling and socket close
  net: tls: handle backlogging of crypto requests
  selftests: tls: use exact comparison in recv_partial
  net: tls: fix returned read length with async decrypt

Sabrina Dubroca (1):
  net: tls: fix use-after-free with partial reads and async decrypt

 include/net/tls.h                 |   5 --
 net/tls/tls_sw.c                  | 135 ++++++++++++++----------------
 tools/testing/selftests/net/tls.c |   8 +-
 3 files changed, 66 insertions(+), 82 deletions(-)

Comments

Sabrina Dubroca Feb. 10, 2024, 9:05 a.m. UTC | #1

2024-02-06, 17:18:17 -0800, Jakub Kicinski wrote:
> Hi!
> 
> valis was reporting a race on socket close so I sat down to try to fix it.
> I used Sabrina's async crypto debug patch to test... and in the process
> run into some of the same issues, and created very similar fixes :(
> I didn't realize how many of those patches weren't applied. Once I found
> Sabrina's code [1] it turned out to be so similar in fact that I added
> her S-o-b's and Co-develop'eds in a semi-haphazard way.
> 
> With this series in place all expected tests pass with async crypto.
> Sabrina had a few more fixes, but I'll leave those to her, things are
> not crashing anymore.

Sorry :(
I got stuck trying to fix a race condition (probably one of those
you're fixing in this series, I tried something similar to patch 3 but
that wasn't enough), and then got distracted. I had a v2 ready and
never posted it :/

Thanks for taking over, and sorry for the duplicate effort. I'll go
back to my old series and see if anything is still relevant on top of
this.

[e-mail] Feb. 10, 2024, 9:40 p.m. UTC | #2

Hello:

This series was applied to netdev/net.git (main)
by David S. Miller <[e-mail]>:

On Tue,  6 Feb 2024 17:18:17 -0800 you wrote:
> Hi!
> 
> valis was reporting a race on socket close so I sat down to try to fix it.
> I used Sabrina's async crypto debug patch to test... and in the process
> run into some of the same issues, and created very similar fixes :(
> I didn't realize how many of those patches weren't applied. Once I found
> Sabrina's code [1] it turned out to be so similar in fact that I added
> her S-o-b's and Co-develop'eds in a semi-haphazard way.
> 
> [...]

Here is the summary with links:
  - [net,1/7] net: tls: factor out tls_*crypt_async_wait()
    https://git.kernel.org/netdev/net/c/c57ca512f3b6
  - [net,2/7] tls: fix race between async notify and socket close
    https://git.kernel.org/netdev/net/c/aec7961916f3
  - [net,3/7] tls: fix race between tx work scheduling and socket close
    https://git.kernel.org/netdev/net/c/e01e3934a1b2
  - [net,4/7] net: tls: handle backlogging of crypto requests
    https://git.kernel.org/netdev/net/c/859054147318
  - [net,5/7] net: tls: fix use-after-free with partial reads and async decrypt
    https://git.kernel.org/netdev/net/c/32b55c5ff910
  - [net,6/7] selftests: tls: use exact comparison in recv_partial
    https://git.kernel.org/netdev/net/c/49d821064c44
  - [net,7/7] net: tls: fix returned read length with async decrypt
    https://git.kernel.org/netdev/net/c/ac437a51ce66

You are awesome, thank you!