From 27393f8c6efc03b8e0b64134721b0d337fca0a80 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 4 Jun 2019 12:00:11 -0700 Subject: Revert "net/tls: avoid NULL-deref on resync during device removal" This reverts commit 38030d7cb77963ba84cdbe034806e2b81245339f. Unfortunately the RX resync may get called from soft IRQ, so we can't take the rwsem to protect from the device disappearing. Signed-off-by: Jakub Kicinski Signed-off-by: David S. Miller --- net/tls/tls_device.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) (limited to 'net/tls/tls_device.c') diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c index b95c408fd771..49b3a2ff8ef3 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -553,8 +553,8 @@ void tls_device_write_space(struct sock *sk, struct tls_context *ctx) void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn) { struct tls_context *tls_ctx = tls_get_ctx(sk); + struct net_device *netdev = tls_ctx->netdev; struct tls_offload_context_rx *rx_ctx; - struct net_device *netdev; u32 is_req_pending; s64 resync_req; u32 req_seq; @@ -568,15 +568,10 @@ void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn) is_req_pending = resync_req; if (unlikely(is_req_pending) && req_seq == seq && - atomic64_try_cmpxchg(&rx_ctx->resync_req, &resync_req, 0)) { - seq += TLS_HEADER_SIZE - 1; - down_read(&device_offload_lock); - netdev = tls_ctx->netdev; - if (netdev) - netdev->tlsdev_ops->tls_dev_resync_rx(netdev, sk, seq, - rcd_sn); - up_read(&device_offload_lock); - } + atomic64_try_cmpxchg(&rx_ctx->resync_req, &resync_req, 0)) + netdev->tlsdev_ops->tls_dev_resync_rx(netdev, sk, + seq + TLS_HEADER_SIZE - 1, + rcd_sn); } static int tls_device_reencrypt(struct sock *sk, struct sk_buff *skb) -- cgit v1.2.3-59-g8ed1b From e52972c11d6b1262964db96d65934196db621685 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 4 Jun 2019 12:00:12 -0700 Subject: net/tls: replace the sleeping lock around RX resync with a bit lock Commit 38030d7cb779 ("net/tls: avoid NULL-deref on resync during device removal") tried to fix a potential NULL-dereference by taking the context rwsem. Unfortunately the RX resync may get called from soft IRQ, so we can't use the rwsem to protect from the device disappearing. Because we are guaranteed there can be only one resync at a time (it's called from strparser) use a bit to indicate resync is busy and make device removal wait for the bit to get cleared. Note that there is a leftover "flags" field in struct tls_context already. Fixes: 4799ac81e52a ("tls: Add rx inline crypto offload") Signed-off-by: Jakub Kicinski Signed-off-by: David S. Miller --- include/net/tls.h | 4 ++++ net/tls/tls_device.c | 27 +++++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) (limited to 'net/tls/tls_device.c') diff --git a/include/net/tls.h b/include/net/tls.h index 39ea62f0c1f6..4a55ce6a303f 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -209,6 +209,10 @@ struct tls_offload_context_tx { (ALIGN(sizeof(struct tls_offload_context_tx), sizeof(void *)) + \ TLS_DRIVER_STATE_SIZE) +enum tls_context_flags { + TLS_RX_SYNC_RUNNING = 0, +}; + struct cipher_context { char *iv; char *rec_seq; diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c index 49b3a2ff8ef3..1f9cf57d9754 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -550,10 +550,22 @@ void tls_device_write_space(struct sock *sk, struct tls_context *ctx) } } +static void tls_device_resync_rx(struct tls_context *tls_ctx, + struct sock *sk, u32 seq, u64 rcd_sn) +{ + struct net_device *netdev; + + if (WARN_ON(test_and_set_bit(TLS_RX_SYNC_RUNNING, &tls_ctx->flags))) + return; + netdev = READ_ONCE(tls_ctx->netdev); + if (netdev) + netdev->tlsdev_ops->tls_dev_resync_rx(netdev, sk, seq, rcd_sn); + clear_bit_unlock(TLS_RX_SYNC_RUNNING, &tls_ctx->flags); +} + void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn) { struct tls_context *tls_ctx = tls_get_ctx(sk); - struct net_device *netdev = tls_ctx->netdev; struct tls_offload_context_rx *rx_ctx; u32 is_req_pending; s64 resync_req; @@ -568,10 +580,10 @@ void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn) is_req_pending = resync_req; if (unlikely(is_req_pending) && req_seq == seq && - atomic64_try_cmpxchg(&rx_ctx->resync_req, &resync_req, 0)) - netdev->tlsdev_ops->tls_dev_resync_rx(netdev, sk, - seq + TLS_HEADER_SIZE - 1, - rcd_sn); + atomic64_try_cmpxchg(&rx_ctx->resync_req, &resync_req, 0)) { + seq += TLS_HEADER_SIZE - 1; + tls_device_resync_rx(tls_ctx, sk, seq, rcd_sn); + } } static int tls_device_reencrypt(struct sock *sk, struct sk_buff *skb) @@ -972,7 +984,10 @@ static int tls_device_down(struct net_device *netdev) if (ctx->rx_conf == TLS_HW) netdev->tlsdev_ops->tls_dev_del(netdev, ctx, TLS_OFFLOAD_CTX_DIR_RX); - ctx->netdev = NULL; + WRITE_ONCE(ctx->netdev, NULL); + smp_mb__before_atomic(); /* pairs with test_and_set_bit() */ + while (test_bit(TLS_RX_SYNC_RUNNING, &ctx->flags)) + usleep_range(10, 200); dev_put(netdev); list_del_init(&ctx->list); -- cgit v1.2.3-59-g8ed1b