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  linux-cve-announce  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]
Message-ID: <ae854fd5-dda1-416a-9327-ac8f9f7d25ba@nvidia.com>
Date: Wed, 22 Oct 2025 14:38:17 +0300
From: Shahar Shitrit <shshitrit@...dia.com>
To: Sabrina Dubroca <sd@...asysnail.net>, Tariq Toukan <tariqt@...dia.com>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S. Miller" <davem@...emloft.net>, Saeed Mahameed <saeedm@...dia.com>,
 Leon Romanovsky <leon@...nel.org>, Mark Bloch <mbloch@...dia.com>,
 John Fastabend <john.fastabend@...il.com>, netdev@...r.kernel.org,
 linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
 Gal Pressman <gal@...dia.com>
Subject: Re: [PATCH net V2 2/3] net: tls: Cancel RX async resync request on
 rdc_delta overflow



On 21/10/2025 18:28, Sabrina Dubroca wrote:
> nit if you end up respinning, there's a typo in the subject:
> s/rdc_delta/rcd_delta/
> 
> 
> 2025-10-20, 10:05:53 +0300, Tariq Toukan wrote:
>> From: Shahar Shitrit <shshitrit@...dia.com>
>>
>> When a netdev issues a RX async resync request for a TLS connection,
>> the TLS module handles it by logging record headers and attempting to
>> match them to the tcp_sn provided by the device. If a match is found,
>> the TLS module approves the tcp_sn for resynchronization.
>>
>> While waiting for a device response, the TLS module also increments
>> rcd_delta each time a new TLS record is received, tracking the distance
>> from the original resync request.
>>
>> However, if the device response is delayed or fails (e.g due to
>> unstable connection and device getting out of tracking, hardware
>> errors, resource exhaustion etc.), the TLS module keeps logging and
>> incrementing, which can lead to a WARN() when rcd_delta exceeds the
>> threshold.
>>
>> To address this, introduce tls_offload_rx_resync_async_request_cancel()
>> to explicitly cancel resync requests when a device response failure is
>> detected. Call this helper also as a final safeguard when rcd_delta
>> crosses its threshold, as reaching this point implies that earlier
>> cancellation did not occur.
>>
>> Fixes: 138559b9f99d ("net/tls: Fix wrong record sn in async mode of device resync")
> 
> The patch itself looks good, but what issue is fixed within this
> patch? The helper will be useful in the next patch, but right now
> we're only resetting the resync_async status. The only change I see
> (without patch 3) is that we won't call tls_device_rx_resync_async()
> next time we decrypt a record in SW, but it wouldn't have done
> anything.
> 
> Actually, also in patch 1/3, there is no "fix" is in that patch.
> 

I agree about patch 1/3 so I'll remove the fixes tag.

For this patch, indeed at this point the WARN() was already fired,
however, the bug being addressed is the unnecessary work the TLS module
continues to do. For my liking, the wasted CPU cycles and resources
alone justify the fix, even if we've already issued a warning.
What do you think?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ