[PATCH] staging: usbip: fix the usage of kthread_stop()

From: Oleg Nesterov
Date: Tue Mar 13 2012 - 13:55:20 EST


stub_shutdown_connection() and vhci_shutdown_connection() use
task_is_dead() before kthread_stop(). This buys nothing and wrong.
kthread_stop() is fine even if this thread is dead. However, if it
is dead nothing protects this task_struct, we shouldn't touch this
memory.

Change the code to do the necessary get_task_struct/put_task_struct.

This patch assumes that

- xxx_shutdown_connection() is always called, so we can't
leak the task_struct.

- kthread_stop_put() can't be called twice on the same task.

Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
---
drivers/staging/usbip/stub_dev.c | 12 ++++++------
drivers/staging/usbip/usbip_common.h | 17 +++++++++++++++++
drivers/staging/usbip/vhci_hcd.c | 8 ++++----
drivers/staging/usbip/vhci_sysfs.c | 4 ++--
4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/usbip/stub_dev.c b/drivers/staging/usbip/stub_dev.c
index 03420e2..fc6ceca 100644
--- a/drivers/staging/usbip/stub_dev.c
+++ b/drivers/staging/usbip/stub_dev.c
@@ -113,8 +113,8 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr,

spin_unlock(&sdev->ud.lock);

- sdev->ud.tcp_rx = kthread_run(stub_rx_loop, &sdev->ud, "stub_rx");
- sdev->ud.tcp_tx = kthread_run(stub_tx_loop, &sdev->ud, "stub_tx");
+ sdev->ud.tcp_rx = kthread_get_run(stub_rx_loop, &sdev->ud, "stub_rx");
+ sdev->ud.tcp_tx = kthread_get_run(stub_tx_loop, &sdev->ud, "stub_tx");

spin_lock(&sdev->ud.lock);
sdev->ud.status = SDEV_ST_USED;
@@ -187,10 +187,10 @@ static void stub_shutdown_connection(struct usbip_device *ud)
}

/* 1. stop threads */
- if (ud->tcp_rx && !task_is_dead(ud->tcp_rx))
- kthread_stop(ud->tcp_rx);
- if (ud->tcp_tx && !task_is_dead(ud->tcp_tx))
- kthread_stop(ud->tcp_tx);
+ if (ud->tcp_rx)
+ kthread_stop_put(ud->tcp_rx);
+ if (ud->tcp_tx)
+ kthread_stop_put(ud->tcp_tx);

/*
* 2. close the socket
diff --git a/drivers/staging/usbip/usbip_common.h b/drivers/staging/usbip/usbip_common.h
index b8f8c48..0d52234 100644
--- a/drivers/staging/usbip/usbip_common.h
+++ b/drivers/staging/usbip/usbip_common.h
@@ -292,6 +292,23 @@ struct usbip_device {
} eh_ops;
};

+#define kthread_get_run(threadfn, data, namefmt, ...) \
+({ \
+ struct task_struct *__k \
+ = kthread_create(threadfn, data, namefmt, ## __VA_ARGS__); \
+ if (!IS_ERR(__k)) { \
+ get_task_struct(__k); \
+ wake_up_process(__k); \
+ } \
+ __k; \
+})
+
+#define kthread_stop_put(k) \
+ do { \
+ kthread_stop(k); \
+ put_task_struct(k); \
+ } while (0)
+
/* usbip_common.c */
void usbip_dump_urb(struct urb *purb);
void usbip_dump_header(struct usbip_header *pdu);
diff --git a/drivers/staging/usbip/vhci_hcd.c b/drivers/staging/usbip/vhci_hcd.c
index 2ee97e2..fb50fdc 100644
--- a/drivers/staging/usbip/vhci_hcd.c
+++ b/drivers/staging/usbip/vhci_hcd.c
@@ -860,10 +860,10 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
}

/* kill threads related to this sdev, if v.c. exists */
- if (vdev->ud.tcp_rx && !task_is_dead(vdev->ud.tcp_rx))
- kthread_stop(vdev->ud.tcp_rx);
- if (vdev->ud.tcp_tx && !task_is_dead(vdev->ud.tcp_tx))
- kthread_stop(vdev->ud.tcp_tx);
+ if (vdev->ud.tcp_rx)
+ kthread_stop_put(vdev->ud.tcp_rx);
+ if (vdev->ud.tcp_tx)
+ kthread_stop_put(vdev->ud.tcp_tx);

pr_info("stop threads\n");

diff --git a/drivers/staging/usbip/vhci_sysfs.c b/drivers/staging/usbip/vhci_sysfs.c
index 0cd039b..7ce9c2f 100644
--- a/drivers/staging/usbip/vhci_sysfs.c
+++ b/drivers/staging/usbip/vhci_sysfs.c
@@ -222,8 +222,8 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
spin_unlock(&the_controller->lock);
/* end the lock */

- vdev->ud.tcp_rx = kthread_run(vhci_rx_loop, &vdev->ud, "vhci_rx");
- vdev->ud.tcp_tx = kthread_run(vhci_tx_loop, &vdev->ud, "vhci_tx");
+ vdev->ud.tcp_rx = kthread_get_run(vhci_rx_loop, &vdev->ud, "vhci_rx");
+ vdev->ud.tcp_tx = kthread_get_run(vhci_tx_loop, &vdev->ud, "vhci_tx");

rh_port_connect(rhport, speed);

--
1.5.5.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/