[PATCH net v2] tls: restrict device-down TX fallback to hardware TX sockets

[PATCH net v2] tls: restrict device-down TX fallback to hardware TX sockets

[Rishikesh Jethwani]

tls_device_down() installs tls_validate_xmit_skb_sw() on every socket with
hardware offload in either direction, but tls_sw_fallback() needs a
hardware TX context in priv_ctx_tx. An RX-only socket's NULL priv_ctx_tx
crashes the fallback on its next transmit, for instance after the device
comes back up:

  BUG: kernel NULL pointer dereference, address: 0000000000000008
  RIP: 0010:_raw_spin_lock_irqsave+0x31/0x60
   tls_sw_fallback+0x11b/0x450
   validate_xmit_skb.isra.0+0x14f/0x420
   __dev_queue_xmit+0x5e8/0xf30

A software-TX socket's priv_ctx_tx is a tls_sw_context_tx, which the
fallback would walk as a hardware context.

Only install the fallback on hardware TX sockets. Today tx_conf alone
cannot identify them from tls_device_down(): tls_set_device_offload()
drops device_offload_lock before it publishes the validator, and
do_tls_setsockopt_conf() stores tx_conf = TLS_HW later still, under
lock_sock, which tls_device_down() does not take. That leaves two
windows. A NETDEV_DOWN before the validator store installs the software
fallback, only for the hardware validator to overwrite it with
ctx->netdev already cleared. A NETDEV_DOWN after the validator store
leaves the fallback in place. Both read tx_conf as TLS_BASE and skip
tls_dev_del(TX) today, which the destructor never retries
(tls_device_queue_ctx_destruction() only queues it while ctx->netdev is
set), so the driver TX context leaks. Gating the fallback on tx_conf
alone would make the second window a regression: the hardware validator
would stay in place with ctx->netdev cleared, so
tls_is_skb_tx_device_offloaded() keeps reporting offloaded.

Set tx_conf to TLS_HW in tls_set_device_offload() before dropping
device_offload_lock, and publish the validator there too, so
tls_device_down() sees both or neither. rx_conf has the same window and
leaks the driver RX context, as tls_device_offload_cleanup_rx() skips
tls_dev_del() once ctx->netdev is NULL, so set it the same way in
tls_set_device_offload_rx().

The unconditional fallback install, and its overwrite by the hardware
validator, date from c55dcdd435aa; the TX and RX leaks from 4799ac81e52a.

The NULL dereference was reproduced on an RX-only socket, which now
survives an interface down/up cycle and keeps receiving TLS records. The
software-TX case and the setsockopt/NETDEV_DOWN races are from code
inspection only.

Fixes: c55dcdd435aa ("net/tls: Fix use-after-free after the TLS device goes down and up")
Fixes: 4799ac81e52a ("tls: Add rx inline crypto offload")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 net/tls/tls_device.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 1a5f328d80a5..9e0863dd2f96 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -1176,13 +1176,15 @@ int tls_set_device_offload(struct sock *sk)
 		goto release_lock;
 
 	tls_device_attach(ctx, sk, netdev);
-	up_read(&device_offload_lock);
 
 	/* following this assignment tls_is_skb_tx_device_offloaded
 	 * will return true and the context might be accessed
-	 * by the netdev's xmit function.
+	 * by the netdev's xmit function. Publish it and tx_conf under
+	 * device_offload_lock so tls_device_down() sees both or neither.
 	 */
 	smp_store_release(&sk->sk_validate_xmit_skb, tls_validate_xmit_skb);
+	ctx->tx_conf = TLS_HW;
+	up_read(&device_offload_lock);
 	dev_put(netdev);
 
 	return 0;
@@ -1258,6 +1260,11 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx)
 		goto free_sw_resources;
 
 	tls_device_attach(ctx, sk, netdev);
+	/* do_tls_setsockopt_conf() stores rx_conf too, but only after
+	 * device_offload_lock is dropped. Set it here so tls_device_down()
+	 * sees TLS_HW and releases the driver RX context.
+	 */
+	ctx->rx_conf = TLS_HW;
 	up_read(&device_offload_lock);
 
 	dev_put(netdev);
@@ -1326,10 +1333,17 @@ static int tls_device_down(struct net_device *netdev)
 	spin_unlock_irqrestore(&tls_device_lock, flags);
 
 	list_for_each_entry_safe(ctx, tmp, &list, list)	{
+		bool tx_hw = ctx->tx_conf == TLS_HW;
+
 		/* Stop offloaded TX and switch to the fallback.
 		 * tls_is_skb_tx_device_offloaded will return false.
+		 * Only HW TX sockets have the context the fallback needs.
+		 * The single read above keeps the fallback and tls_dev_del()
+		 * below in agreement.
 		 */
-		WRITE_ONCE(ctx->sk->sk_validate_xmit_skb, tls_validate_xmit_skb_sw);
+		if (tx_hw)
+			WRITE_ONCE(ctx->sk->sk_validate_xmit_skb,
+				   tls_validate_xmit_skb_sw);
 
 		/* Stop the RX and TX resync.
 		 * tls_dev_resync must not be called after tls_dev_del.
@@ -1346,7 +1360,7 @@ static int tls_device_down(struct net_device *netdev)
 		synchronize_net();
 
 		/* Release the offload context on the driver side. */
-		if (ctx->tx_conf == TLS_HW)
+		if (tx_hw)
 			netdev->tlsdev_ops->tls_dev_del(netdev, ctx,
 							TLS_OFFLOAD_CTX_DIR_TX);
 		if (ctx->rx_conf == TLS_HW &&
-- 
2.50.1