lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 20 May 2020 13:34:28 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Tariq Toukan <tariqt@...lanox.com>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        Boris Pismenny <borisp@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>
Subject: Re: [PATCH net] net/tls: Fix driver request resync

On Wed, 20 May 2020 18:14:08 +0300 Tariq Toukan wrote:
> From: Boris Pismenny <borisp@...lanox.com>
> 
> In driver request resync, the hardware requests a resynchronization
> request at some TCP sequence number. If that TCP sequence number does
> not point to a TLS record header, then the resync attempt has failed.
> 
> Failed resync should reset the resync request to avoid spurious resyncs
> after the TCP sequence number has wrapped around.
> 
> Fix this by resetting the resync request when the TLS record header
> sequence number is not before the requested sequence number.
> As a result, drivers may be called with a sequence number that is not
> equal to the requested sequence number.
> 
> Fixes: f953d33ba122 ("net/tls: add kernel-driven TLS RX resync")
> Signed-off-by: Boris Pismenny <borisp@...lanox.com>
> Signed-off-by: Tariq Toukan <tariqt@...lanox.com>
> Reviewed-by: Saeed Mahameed <saeedm@...lanox.com>
> ---
>  net/tls/tls_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index a562ebaaa33c..cbb13001b4a9 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -714,7 +714,7 @@ void tls_device_rx_resync_new_rec(struct sock *sk, u32 rcd_len, u32 seq)
>  		seq += TLS_HEADER_SIZE - 1;
>  		is_req_pending = resync_req;
>  
> -		if (likely(!is_req_pending) || req_seq != seq ||
> +		if (likely(!is_req_pending) || before(seq, req_seq) ||

So the kernel is going to send the sync message to the device with at
sequence number the device never asked about? 

Kernel usually can't guarantee that the notification will happen,
(memory allocation errors, etc.) so the device needs to do the
restarting itself. The notification should not be necessary.

>  		    !atomic64_try_cmpxchg(&rx_ctx->resync_req, &resync_req, 0))
>  			return;
>  		break;

Powered by blists - more mailing lists