[PATCH net v2] tls: keep TX and RX hardware offload on the same device

[PATCH net v2] tls: keep TX and RX hardware offload on the same device

[Rishikesh Jethwani]

After TX is offloaded on device A, a route change, or a change in the
usable-slave set of an 802.3ad or balance-xor (layer3+4) bond, can make
RX setup program device B. tls_device_attach() sees the existing TLS
destructor and keeps ctx->netdev pointing at A. The RX driver state on B
is then untracked: tls_dev_resync() and tls_dev_del() are invoked on A
for a context it never created, and neither socket close nor
NETDEV_DOWN on B calls tls_dev_del() for B, so the driver context leaks.
The reverse setup order has the same problem.

Check the shared device binding under device_offload_lock before each
tls_dev_add(). Permit the first attachment and a second direction on the
same device. Reject a different device, including when the old binding
was cleared by NETDEV_DOWN: the TLS destructor remains installed and
tls_device_attach() cannot establish a new binding. Setup then falls
back to software for the requested direction.

The RX check precedes allocation and software setup. TX uses its existing
resource unwind path without calling the driver. Document the shared
binding and clarify that fallback is per direction.

Fixes: 4799ac81e52a ("tls: Add rx inline crypto offload")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 Documentation/networking/tls-offload.rst | 16 ++++++++++------
 net/tls/tls_device.c                     | 24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/tls-offload.rst b/Documentation/networking/tls-offload.rst
index e5802bcd4d22..cf0e599c06a1 100644
--- a/Documentation/networking/tls-offload.rst
+++ b/Documentation/networking/tls-offload.rst
@@ -71,12 +71,16 @@ During driver initialization device sets the ``NETIF_F_HW_TLS_RX`` and
 pointer in the :c:member:`tlsdev_ops` member of the
 :c:type:`struct net_device <net_device>`.
 
-When TLS cryptographic connection state is installed on a ``ktls`` socket
-(note that it is done twice, once for RX and once for TX direction,
-and the two are completely independent), the kernel checks if the underlying
-network device is offload-capable and attempts the offload. In case offload
-fails the connection is handled entirely in software using the same mechanism
-as if the offload was never tried.
+TLS cryptographic connection state is installed separately for RX and TX
+on a ``ktls`` socket. For each direction, the kernel checks if the underlying
+network device is offload-capable and attempts the offload. If offload fails,
+that direction is handled in software as if offload had never been tried.
+
+Both hardware directions share one device binding. Once one direction is
+offloaded, the other direction must use the same device. If a route change
+selects a different device, or ``NETDEV_DOWN`` has cleared the binding,
+hardware offload of the other direction is declined and it uses software
+instead.
 
 Offload request is performed via the :c:member:`tls_dev_add` callback of
 :c:type:`struct tlsdev_ops <tlsdev_ops>`:
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index f11d0528fc43..9df183d0dd97 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -1022,6 +1022,20 @@ int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx)
 	return tls_device_reencrypt(sk, tls_ctx);
 }
 
+/* Both directions share ctx->netdev. A second direction must use that
+ * device; after NETDEV_DOWN clears it, no new direction can be offloaded.
+ */
+static bool tls_device_can_attach(struct tls_context *ctx, struct sock *sk,
+				  struct net_device *netdev)
+{
+	struct net_device *ctx_netdev;
+
+	ctx_netdev = rcu_dereference_protected(ctx->netdev,
+					       lockdep_is_held(&device_offload_lock));
+	return sk->sk_destruct != tls_device_sk_destruct ||
+	       ctx_netdev == netdev;
+}
+
 static void tls_device_attach(struct tls_context *ctx, struct sock *sk,
 			      struct net_device *netdev)
 {
@@ -1157,6 +1171,11 @@ int tls_set_device_offload(struct sock *sk)
 		goto release_lock;
 	}
 
+	if (!tls_device_can_attach(ctx, sk, netdev)) {
+		rc = -EOPNOTSUPP;
+		goto release_lock;
+	}
+
 	ctx->priv_ctx_tx = offload_ctx;
 	rc = netdev->tlsdev_ops->tls_dev_add(netdev, sk, TLS_OFFLOAD_CTX_DIR_TX,
 					     &ctx->crypto_send.info,
@@ -1227,6 +1246,11 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx)
 		goto release_lock;
 	}
 
+	if (!tls_device_can_attach(ctx, sk, netdev)) {
+		rc = -EOPNOTSUPP;
+		goto release_lock;
+	}
+
 	context = kzalloc_obj(*context);
 	if (!context) {
 		rc = -ENOMEM;
-- 
2.50.1

Re: [PATCH net v2] tls: keep TX and RX hardware offload on the same device

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] tls: keep TX and RX hardware offload on the same device

[Rishikesh Jethwani]

On Tue, Oct 20, 2026 at 2:49 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

This issue was discovered during code review and subsequently reproduced
on hardware using mlx5 and TLS 1.2 AES-GCM-128.

Changing the route between TX and RX setup installed the second direction
on a different device. Driver traces showed successful adds on both devices,
but both deletes at socket close targeted the first device, leaving an orphaned
context on the second. Both setup orders reproduced the leak,
even though all current core TLS context counters returned to zero.
No crash was observed.

With the fix, the same tests correctly fall back to software for the second
direction and leave no orphaned hardware context.

Re: [PATCH net v2] tls: keep TX and RX hardware offload on the same device

[Rishikesh Jethwani]

On Tue, Oct 20, 2026 at 2:49 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

This issue was discovered during code review and subsequently
reproduced on hardware using mlx5 and TLS 1.2 AES-GCM-128.

Changing the route between TX and RX setup installed the second
direction on a different device. Driver traces showed successful adds
on both devices, but both deletes at socket close targeted the first
device, leaving an orphaned context on the second. Both setup orders
reproduced the leak, even though all current core TLS context counters
returned to zero. No crash was observed.

With the fix, the same tests correctly fall back to software for the
second direction and leave no orphaned hardware context.

Re: [PATCH net v2] tls: keep TX and RX hardware offload on the same device

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 1 · High: 0 (1) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] tls: type confusion and UAF in tls_device_down() when TX is
  not TLS_HW

Pre-existing issues:
- [High] tls_set_sw_offload() (net/tls/tls_sw.c:2594) sets the shared
  ctx->push_pending_record = tls_sw_push_pending_record unconditionally,…

--

tls: keep TX and RX hardware offload on the same device

This patch adds tls_device_can_attach(), which is checked under
device_offload_lock before each tls_dev_add() in tls_set_device_offload()
and tls_set_device_offload_rx(). A second direction is only offloaded if
it resolves to the netdev already bound to the context; otherwise setup
falls back to software for that direction.

> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index f11d0528fc431..9df183d0dd972 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c

[ ... ]

> @@ -1227,6 +1246,11 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx)
>  		goto release_lock;
>  	}
>  
> +	if (!tls_device_can_attach(ctx, sk, netdev)) {
> +		rc = -EOPNOTSUPP;
> +		goto release_lock;
> +	}
> +

[Severity: Critical]
This isn't introduced by this patch, but the patch now deliberately
steers sockets into the state that hits it, so it seems worth raising
here.

When RX is on hardware and TX is declined by tls_device_can_attach(),
do_tls_setsockopt_conf() falls back to TLS_SW for TX (or TX is simply
never configured and stays TLS_BASE), while ctx stays on
tls_device_list bound to the RX device. If that device later goes down,
tls_device_down() does this for every matching ctx:

	WRITE_ONCE(ctx->sk->sk_validate_xmit_skb,
		   tls_validate_xmit_skb_sw);

without looking at ctx->tx_conf. The tls_dev_del() calls further down
are guarded by tx_conf == TLS_HW and rx_conf == TLS_HW, but this store
is not.

>From then on every skb with TCP payload on the socket goes through
validate_xmit_skb() -> tls_validate_xmit_skb_sw() -> tls_sw_fallback(),
which casts tls_ctx->priv_ctx_tx with tls_offload_ctx_tx() and passes
the result to fill_sg_in(), which takes ctx->lock and walks
ctx->records_list through tls_get_record().

With tx_conf == TLS_BASE, priv_ctx_tx is NULL. Isn't this a NULL
pointer dereference on the first data segment sent after the device
goes down? With tx_conf == TLS_SW, priv_ctx_tx is the struct
tls_sw_context_tx from init_ctx_tx(), so fill_sg_in() would take a
spinlock and walk a list that are really unrelated fields of that
structure.

Close looks worse still. tls_sk_proto_close() frees priv_ctx_tx via
tls_sw_free_ctx_tx() before calling ctx->sk_proto->close(), but with
rx_conf == TLS_HW the tls_context and the sk_validate_xmit_skb hook stay
around until tls_device_sk_destruct(). Data pushed out by tcp_close()
or retransmitted afterwards would run tls_sw_fallback() on freed
memory.

The updated tls-offload.rst says a declined direction "uses software
instead", and NETDEV_DOWN clearing the binding is now an explicit
reason to decline TX, so the patch relies on this combination working.

Should tls_device_down() only switch sk_validate_xmit_skb when TX is
really device offloaded? ctx->tx_conf is only set by
do_tls_setsockopt_conf() after tls_set_device_offload() has dropped
device_offload_lock, so I'm not sure tx_conf alone is a reliable test
there. Would comparing against tls_validate_xmit_skb, or having
tls_validate_xmit_skb_sw() bail out when TX is not TLS_HW, work better?

[Severity: High]
This isn't a bug introduced by this patch, but it is related to the new
tls-offload.rst wording that a declined direction "is handled in software
as if offload had never been tried".

Suppose TX is already on hardware and RX is rejected here. Then
do_tls_setsockopt_conf() falls back to tls_set_sw_offload(sk, 0, ...),
and that function sets the shared TX callback whatever the direction:

net/tls/tls_sw.c:tls_set_sw_offload() {
    ...
	ctx->push_pending_record = tls_sw_push_pending_record;
    ...
}

This replaces the callback that tls_set_device_offload() installed:

	ctx->push_pending_record = tls_device_push_pending_record;

The hardware RX success path also calls tls_set_sw_offload(sk, 0, NULL),
so this could already happen before this patch.

Now suppose device TX leaves an open record, for example because
sendmsg() with MSG_MORE set pending_open_record_frags. A later sendmsg()
with a TLS_SET_RECORD_TYPE cmsg then takes this path:

tls_device_sendmsg()
  tls_process_cmsg()
    tls_handle_open_record()
      ctx->push_pending_record() -> tls_sw_push_pending_record()

tls_sk_proto_cleanup() reaches the same callback on close when
sk_write_pending is set. tls_sw_push_pending_record() does:

	struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
	struct tls_rec *rec = ctx->open_rec;

but priv_ctx_tx is really a struct tls_offload_context_tx here.

Doesn't this read an unrelated field of tls_offload_context_tx as a
struct tls_rec pointer?

If that field is NULL, the open device record is not flushed before the
record type changes. Data that is already buffered then goes out under
the new record type. If the field is non-NULL, rec->msg_plaintext is
read through a garbage pointer.

This seems to go back to 3c4d7559159b (2017). Could tls_set_sw_offload()
set push_pending_record only when tx is set? Then RX setup, hardware or
software fallback, would leave the device TX callback in place.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261020214500.972308-1-rjethwani%40purestorage.com