diff mbox series

[tls,2/5] net/tls: sleeping function from invalid context

Message ID 20181211102009.3561-1-atul.gupta@chelsio.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series [tls,1/5] net/tls: Init routines in create_ctx | expand

Commit Message

Atul Gupta Dec. 11, 2018, 10:20 a.m. UTC
HW unhash within mutex for registered tls devices cause sleep
when called from tcp_set_state for TCP_CLOSE. Release lock and
re-acquire after function call with ref count incr/dec.
defined kref and fp release for tls_device to ensure device
is not released outside lock.

BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:748
in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: swapper/7
INFO: lockdep is turned off.
CPU: 7 PID: 0 Comm: swapper/7 Tainted: G        W  O
Call Trace:
 <IRQ>
 dump_stack+0x5e/0x8b
 ___might_sleep+0x222/0x260
 __mutex_lock+0x5c/0xa50
 ? vprintk_emit+0x1f3/0x440
 ? kmem_cache_free+0x22d/0x2a0
 ? tls_hw_unhash+0x2f/0x80
 ? printk+0x52/0x6e
 ? tls_hw_unhash+0x2f/0x80
 tls_hw_unhash+0x2f/0x80
 tcp_set_state+0x5f/0x180
 tcp_done+0x2e/0xe0
 tcp_rcv_state_process+0x92c/0xdd3
 ? lock_acquire+0xf5/0x1f0
 ? tcp_v4_rcv+0xa7c/0xbe0
 ? tcp_v4_do_rcv+0x70/0x1e0

Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
 drivers/crypto/chelsio/chtls/chtls_main.c | 55 ++++++++++++++++++-------------
 include/net/tls.h                         |  6 ++++
 net/tls/tls_main.c                        | 36 ++++++++++++--------
 3 files changed, 61 insertions(+), 36 deletions(-)

Comments

David Miller Dec. 14, 2018, 9:41 p.m. UTC | #1
From: Atul Gupta <atul.gupta@chelsio.com>
Date: Tue, 11 Dec 2018 02:20:09 -0800

> HW unhash within mutex for registered tls devices cause sleep
> when called from tcp_set_state for TCP_CLOSE. Release lock and
> re-acquire after function call with ref count incr/dec.
> defined kref and fp release for tls_device to ensure device
> is not released outside lock.
 ...
> Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>

Applied.
diff mbox series

Patch

diff --git a/drivers/crypto/chelsio/chtls/chtls_main.c b/drivers/crypto/chelsio/chtls/chtls_main.c
index f472c51..db40ab6 100644
--- a/drivers/crypto/chelsio/chtls/chtls_main.c
+++ b/drivers/crypto/chelsio/chtls/chtls_main.c
@@ -149,6 +149,30 @@  static void chtls_destroy_hash(struct tls_device *dev, struct sock *sk)
 		chtls_stop_listen(sk);
 }
 
+static void chtls_free_uld(struct chtls_dev *cdev)
+{
+	int i;
+
+	tls_unregister_device(&cdev->tlsdev);
+	kvfree(cdev->kmap.addr);
+	idr_destroy(&cdev->hwtid_idr);
+	for (i = 0; i < (1 << RSPQ_HASH_BITS); i++)
+		kfree_skb(cdev->rspq_skb_cache[i]);
+	kfree(cdev->lldi);
+	kfree_skb(cdev->askb);
+	kfree(cdev);
+}
+
+static inline void chtls_dev_release(struct kref *kref)
+{
+	struct chtls_dev *cdev;
+	struct tls_device *dev;
+
+	dev = container_of(kref, struct tls_device, kref);
+	cdev = to_chtls_dev(dev);
+	chtls_free_uld(cdev);
+}
+
 static void chtls_register_dev(struct chtls_dev *cdev)
 {
 	struct tls_device *tlsdev = &cdev->tlsdev;
@@ -159,15 +183,12 @@  static void chtls_register_dev(struct chtls_dev *cdev)
 	tlsdev->feature = chtls_inline_feature;
 	tlsdev->hash = chtls_create_hash;
 	tlsdev->unhash = chtls_destroy_hash;
-	tls_register_device(&cdev->tlsdev);
+	tlsdev->release = chtls_dev_release;
+	kref_init(&tlsdev->kref);
+	tls_register_device(tlsdev);
 	cdev->cdev_state = CHTLS_CDEV_STATE_UP;
 }
 
-static void chtls_unregister_dev(struct chtls_dev *cdev)
-{
-	tls_unregister_device(&cdev->tlsdev);
-}
-
 static void process_deferq(struct work_struct *task_param)
 {
 	struct chtls_dev *cdev = container_of(task_param,
@@ -262,28 +283,16 @@  static void *chtls_uld_add(const struct cxgb4_lld_info *info)
 	return NULL;
 }
 
-static void chtls_free_uld(struct chtls_dev *cdev)
-{
-	int i;
-
-	chtls_unregister_dev(cdev);
-	kvfree(cdev->kmap.addr);
-	idr_destroy(&cdev->hwtid_idr);
-	for (i = 0; i < (1 << RSPQ_HASH_BITS); i++)
-		kfree_skb(cdev->rspq_skb_cache[i]);
-	kfree(cdev->lldi);
-	kfree_skb(cdev->askb);
-	kfree(cdev);
-}
-
 static void chtls_free_all_uld(void)
 {
 	struct chtls_dev *cdev, *tmp;
 
 	mutex_lock(&cdev_mutex);
 	list_for_each_entry_safe(cdev, tmp, &cdev_list, list) {
-		if (cdev->cdev_state == CHTLS_CDEV_STATE_UP)
-			chtls_free_uld(cdev);
+		if (cdev->cdev_state == CHTLS_CDEV_STATE_UP) {
+			list_del(&cdev->list);
+			kref_put(&cdev->tlsdev.kref, cdev->tlsdev.release);
+		}
 	}
 	mutex_unlock(&cdev_mutex);
 }
@@ -304,7 +313,7 @@  static int chtls_uld_state_change(void *handle, enum cxgb4_state new_state)
 		mutex_lock(&cdev_mutex);
 		list_del(&cdev->list);
 		mutex_unlock(&cdev_mutex);
-		chtls_free_uld(cdev);
+		kref_put(&cdev->tlsdev.kref, cdev->tlsdev.release);
 		break;
 	default:
 		break;
diff --git a/include/net/tls.h b/include/net/tls.h
index bab5627..3cbcd12 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -76,6 +76,10 @@ 
  *
  * void (*unhash)(struct tls_device *device, struct sock *sk);
  *     This function cleans listen state set by Inline TLS driver
+ *
+ * void (*release)(struct kref *kref);
+ *     Release the registered device and allocated resources
+ * @kref: Number of reference to tls_device
  */
 struct tls_device {
 	char name[TLS_DEVICE_NAME_MAX];
@@ -83,6 +87,8 @@  struct tls_device {
 	int  (*feature)(struct tls_device *device);
 	int  (*hash)(struct tls_device *device, struct sock *sk);
 	void (*unhash)(struct tls_device *device, struct sock *sk);
+	void (*release)(struct kref *kref);
+	struct kref kref;
 };
 
 enum {
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 4920803..1428bd7 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -56,7 +56,7 @@  enum {
 static struct proto *saved_tcpv6_prot;
 static DEFINE_MUTEX(tcpv6_prot_mutex);
 static LIST_HEAD(device_list);
-static DEFINE_MUTEX(device_mutex);
+static DEFINE_SPINLOCK(device_spinlock);
 static struct proto tls_prots[TLS_NUM_PROTS][TLS_NUM_CONFIG][TLS_NUM_CONFIG];
 static struct proto_ops tls_sw_proto_ops;
 
@@ -555,7 +555,7 @@  static int tls_hw_prot(struct sock *sk)
 	struct tls_device *dev;
 	int rc = 0;
 
-	mutex_lock(&device_mutex);
+	spin_lock_bh(&device_spinlock);
 	list_for_each_entry(dev, &device_list, dev_list) {
 		if (dev->feature && dev->feature(dev)) {
 			ctx = create_ctx(sk);
@@ -573,7 +573,7 @@  static int tls_hw_prot(struct sock *sk)
 		}
 	}
 out:
-	mutex_unlock(&device_mutex);
+	spin_unlock_bh(&device_spinlock);
 	return rc;
 }
 
@@ -582,12 +582,17 @@  static void tls_hw_unhash(struct sock *sk)
 	struct tls_context *ctx = tls_get_ctx(sk);
 	struct tls_device *dev;
 
-	mutex_lock(&device_mutex);
+	spin_lock_bh(&device_spinlock);
 	list_for_each_entry(dev, &device_list, dev_list) {
-		if (dev->unhash)
+		if (dev->unhash) {
+			kref_get(&dev->kref);
+			spin_unlock_bh(&device_spinlock);
 			dev->unhash(dev, sk);
+			kref_put(&dev->kref, dev->release);
+			spin_lock_bh(&device_spinlock);
+		}
 	}
-	mutex_unlock(&device_mutex);
+	spin_unlock_bh(&device_spinlock);
 	ctx->unhash(sk);
 }
 
@@ -598,12 +603,17 @@  static int tls_hw_hash(struct sock *sk)
 	int err;
 
 	err = ctx->hash(sk);
-	mutex_lock(&device_mutex);
+	spin_lock_bh(&device_spinlock);
 	list_for_each_entry(dev, &device_list, dev_list) {
-		if (dev->hash)
+		if (dev->hash) {
+			kref_get(&dev->kref);
+			spin_unlock_bh(&device_spinlock);
 			err |= dev->hash(dev, sk);
+			kref_put(&dev->kref, dev->release);
+			spin_lock_bh(&device_spinlock);
+		}
 	}
-	mutex_unlock(&device_mutex);
+	spin_unlock_bh(&device_spinlock);
 
 	if (err)
 		tls_hw_unhash(sk);
@@ -699,17 +709,17 @@  static int tls_init(struct sock *sk)
 
 void tls_register_device(struct tls_device *device)
 {
-	mutex_lock(&device_mutex);
+	spin_lock_bh(&device_spinlock);
 	list_add_tail(&device->dev_list, &device_list);
-	mutex_unlock(&device_mutex);
+	spin_unlock_bh(&device_spinlock);
 }
 EXPORT_SYMBOL(tls_register_device);
 
 void tls_unregister_device(struct tls_device *device)
 {
-	mutex_lock(&device_mutex);
+	spin_lock_bh(&device_spinlock);
 	list_del(&device->dev_list);
-	mutex_unlock(&device_mutex);
+	spin_unlock_bh(&device_spinlock);
 }
 EXPORT_SYMBOL(tls_unregister_device);