Skip to content

Commit c55dcdd

Browse files
nvmmaxdavem330
authored andcommitted
net/tls: Fix use-after-free after the TLS device goes down and up
When a netdev with active TLS offload goes down, tls_device_down is called to stop the offload and tear down the TLS context. However, the socket stays alive, and it still points to the TLS context, which is now deallocated. If a netdev goes up, while the connection is still active, and the data flow resumes after a number of TCP retransmissions, it will lead to a use-after-free of the TLS context. This commit addresses this bug by keeping the context alive until its normal destruction, and implements the necessary fallbacks, so that the connection can resume in software (non-offloaded) kTLS mode. On the TX side tls_sw_fallback is used to encrypt all packets. The RX side already has all the necessary fallbacks, because receiving non-decrypted packets is supported. The thing needed on the RX side is to block resync requests, which are normally produced after receiving non-decrypted packets. The necessary synchronization is implemented for a graceful teardown: first the fallbacks are deployed, then the driver resources are released (it used to be possible to have a tls_dev_resync after tls_dev_del). A new flag called TLS_RX_DEV_DEGRADED is added to indicate the fallback mode. It's used to skip the RX resync logic completely, as it becomes useless, and some objects may be released (for example, resync_async, which is allocated and freed by the driver). Fixes: e8f6979 ("net/tls: Add generic NIC offload infrastructure") Signed-off-by: Maxim Mikityanskiy <[email protected]> Reviewed-by: Tariq Toukan <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 05fc8b6 commit c55dcdd

File tree

4 files changed

+64
-5
lines changed

4 files changed

+64
-5
lines changed

include/net/tls.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,11 @@ struct tls_offload_context_tx {
193193
(sizeof(struct tls_offload_context_tx) + TLS_DRIVER_STATE_SIZE_TX)
194194

195195
enum tls_context_flags {
196+
/* tls_device_down was called after the netdev went down, device state
197+
* was released, and kTLS works in software, even though rx_conf is
198+
* still TLS_HW (needed for transition).
199+
*/
200+
TLS_RX_DEV_DEGRADED = 0,
196201
/* Unlike RX where resync is driven entirely by the core in TX only
197202
* the driver knows when things went out of sync, so we need the flag
198203
* to be atomic.
@@ -265,6 +270,7 @@ struct tls_context {
265270

266271
/* cache cold stuff */
267272
struct proto *sk_proto;
273+
struct sock *sk;
268274

269275
void (*sk_destruct)(struct sock *sk);
270276

@@ -447,6 +453,9 @@ static inline u16 tls_user_config(struct tls_context *ctx, bool tx)
447453
struct sk_buff *
448454
tls_validate_xmit_skb(struct sock *sk, struct net_device *dev,
449455
struct sk_buff *skb);
456+
struct sk_buff *
457+
tls_validate_xmit_skb_sw(struct sock *sk, struct net_device *dev,
458+
struct sk_buff *skb);
450459

451460
static inline bool tls_is_sk_tx_device_offloaded(struct sock *sk)
452461
{

net/tls/tls_device.c

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ static void tls_device_gc_task(struct work_struct *work);
5050
static DECLARE_WORK(tls_device_gc_work, tls_device_gc_task);
5151
static LIST_HEAD(tls_device_gc_list);
5252
static LIST_HEAD(tls_device_list);
53+
static LIST_HEAD(tls_device_down_list);
5354
static DEFINE_SPINLOCK(tls_device_lock);
5455

5556
static void tls_device_free_ctx(struct tls_context *ctx)
@@ -759,6 +760,8 @@ void tls_device_rx_resync_new_rec(struct sock *sk, u32 rcd_len, u32 seq)
759760

760761
if (tls_ctx->rx_conf != TLS_HW)
761762
return;
763+
if (unlikely(test_bit(TLS_RX_DEV_DEGRADED, &tls_ctx->flags)))
764+
return;
762765

763766
prot = &tls_ctx->prot_info;
764767
rx_ctx = tls_offload_ctx_rx(tls_ctx);
@@ -961,6 +964,17 @@ int tls_device_decrypted(struct sock *sk, struct tls_context *tls_ctx,
961964

962965
ctx->sw.decrypted |= is_decrypted;
963966

967+
if (unlikely(test_bit(TLS_RX_DEV_DEGRADED, &tls_ctx->flags))) {
968+
if (likely(is_encrypted || is_decrypted))
969+
return 0;
970+
971+
/* After tls_device_down disables the offload, the next SKB will
972+
* likely have initial fragments decrypted, and final ones not
973+
* decrypted. We need to reencrypt that single SKB.
974+
*/
975+
return tls_device_reencrypt(sk, skb);
976+
}
977+
964978
/* Return immediately if the record is either entirely plaintext or
965979
* entirely ciphertext. Otherwise handle reencrypt partially decrypted
966980
* record.
@@ -1290,20 +1304,48 @@ static int tls_device_down(struct net_device *netdev)
12901304
spin_unlock_irqrestore(&tls_device_lock, flags);
12911305

12921306
list_for_each_entry_safe(ctx, tmp, &list, list) {
1307+
/* Stop offloaded TX and switch to the fallback.
1308+
* tls_is_sk_tx_device_offloaded will return false.
1309+
*/
1310+
WRITE_ONCE(ctx->sk->sk_validate_xmit_skb, tls_validate_xmit_skb_sw);
1311+
1312+
/* Stop the RX and TX resync.
1313+
* tls_dev_resync must not be called after tls_dev_del.
1314+
*/
1315+
WRITE_ONCE(ctx->netdev, NULL);
1316+
1317+
/* Start skipping the RX resync logic completely. */
1318+
set_bit(TLS_RX_DEV_DEGRADED, &ctx->flags);
1319+
1320+
/* Sync with inflight packets. After this point:
1321+
* TX: no non-encrypted packets will be passed to the driver.
1322+
* RX: resync requests from the driver will be ignored.
1323+
*/
1324+
synchronize_net();
1325+
1326+
/* Release the offload context on the driver side. */
12931327
if (ctx->tx_conf == TLS_HW)
12941328
netdev->tlsdev_ops->tls_dev_del(netdev, ctx,
12951329
TLS_OFFLOAD_CTX_DIR_TX);
12961330
if (ctx->rx_conf == TLS_HW &&
12971331
!test_bit(TLS_RX_DEV_CLOSED, &ctx->flags))
12981332
netdev->tlsdev_ops->tls_dev_del(netdev, ctx,
12991333
TLS_OFFLOAD_CTX_DIR_RX);
1300-
WRITE_ONCE(ctx->netdev, NULL);
1301-
synchronize_net();
1334+
13021335
dev_put(netdev);
1303-
list_del_init(&ctx->list);
13041336

1305-
if (refcount_dec_and_test(&ctx->refcount))
1306-
tls_device_free_ctx(ctx);
1337+
/* Move the context to a separate list for two reasons:
1338+
* 1. When the context is deallocated, list_del is called.
1339+
* 2. It's no longer an offloaded context, so we don't want to
1340+
* run offload-specific code on this context.
1341+
*/
1342+
spin_lock_irqsave(&tls_device_lock, flags);
1343+
list_move_tail(&ctx->list, &tls_device_down_list);
1344+
spin_unlock_irqrestore(&tls_device_lock, flags);
1345+
1346+
/* Device contexts for RX and TX will be freed in on sk_destruct
1347+
* by tls_device_free_ctx. rx_conf and tx_conf stay in TLS_HW.
1348+
*/
13071349
}
13081350

13091351
up_write(&device_offload_lock);

net/tls/tls_device_fallback.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,13 @@ struct sk_buff *tls_validate_xmit_skb(struct sock *sk,
431431
}
432432
EXPORT_SYMBOL_GPL(tls_validate_xmit_skb);
433433

434+
struct sk_buff *tls_validate_xmit_skb_sw(struct sock *sk,
435+
struct net_device *dev,
436+
struct sk_buff *skb)
437+
{
438+
return tls_sw_fallback(sk, skb);
439+
}
440+
434441
struct sk_buff *tls_encrypt_skb(struct sk_buff *skb)
435442
{
436443
return tls_sw_fallback(skb->sk, skb);

net/tls/tls_main.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ struct tls_context *tls_ctx_create(struct sock *sk)
636636
mutex_init(&ctx->tx_lock);
637637
rcu_assign_pointer(icsk->icsk_ulp_data, ctx);
638638
ctx->sk_proto = READ_ONCE(sk->sk_prot);
639+
ctx->sk = sk;
639640
return ctx;
640641
}
641642

0 commit comments

Comments
 (0)