staging: usbip: fix the usage of kthread_stop()
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@redhat.com> Cc: Tobias Klauser <tklauser@distanz.ch> Cc: Matt Mooney <mfm@muteddisk.com>, Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
		
					parent
					
						
							
								96ddcd4398
							
						
					
				
			
			
				commit
				
					
						ba46ce30f1
					
				
			
		
					 4 changed files with 29 additions and 12 deletions
				
			
		| 
						 | 
					@ -113,8 +113,8 @@ static ssize_t store_sockfd(struct device *dev, struct device_attribute *attr,
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		spin_unlock(&sdev->ud.lock);
 | 
							spin_unlock(&sdev->ud.lock);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		sdev->ud.tcp_rx = kthread_run(stub_rx_loop, &sdev->ud, "stub_rx");
 | 
							sdev->ud.tcp_rx = kthread_get_run(stub_rx_loop, &sdev->ud, "stub_rx");
 | 
				
			||||||
		sdev->ud.tcp_tx = kthread_run(stub_tx_loop, &sdev->ud, "stub_tx");
 | 
							sdev->ud.tcp_tx = kthread_get_run(stub_tx_loop, &sdev->ud, "stub_tx");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		spin_lock(&sdev->ud.lock);
 | 
							spin_lock(&sdev->ud.lock);
 | 
				
			||||||
		sdev->ud.status = SDEV_ST_USED;
 | 
							sdev->ud.status = SDEV_ST_USED;
 | 
				
			||||||
| 
						 | 
					@ -187,10 +187,10 @@ static void stub_shutdown_connection(struct usbip_device *ud)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/* 1. stop threads */
 | 
						/* 1. stop threads */
 | 
				
			||||||
	if (ud->tcp_rx && !task_is_dead(ud->tcp_rx))
 | 
						if (ud->tcp_rx)
 | 
				
			||||||
		kthread_stop(ud->tcp_rx);
 | 
							kthread_stop_put(ud->tcp_rx);
 | 
				
			||||||
	if (ud->tcp_tx && !task_is_dead(ud->tcp_tx))
 | 
						if (ud->tcp_tx)
 | 
				
			||||||
		kthread_stop(ud->tcp_tx);
 | 
							kthread_stop_put(ud->tcp_tx);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/*
 | 
						/*
 | 
				
			||||||
	 * 2. close the socket
 | 
						 * 2. close the socket
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -292,6 +292,23 @@ struct usbip_device {
 | 
				
			||||||
	} eh_ops;
 | 
						} 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 */
 | 
					/* usbip_common.c */
 | 
				
			||||||
void usbip_dump_urb(struct urb *purb);
 | 
					void usbip_dump_urb(struct urb *purb);
 | 
				
			||||||
void usbip_dump_header(struct usbip_header *pdu);
 | 
					void usbip_dump_header(struct usbip_header *pdu);
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -821,10 +821,10 @@ static void vhci_shutdown_connection(struct usbip_device *ud)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/* kill threads related to this sdev, if v.c. exists */
 | 
						/* kill threads related to this sdev, if v.c. exists */
 | 
				
			||||||
	if (vdev->ud.tcp_rx && !task_is_dead(vdev->ud.tcp_rx))
 | 
						if (vdev->ud.tcp_rx)
 | 
				
			||||||
		kthread_stop(vdev->ud.tcp_rx);
 | 
							kthread_stop_put(vdev->ud.tcp_rx);
 | 
				
			||||||
	if (vdev->ud.tcp_tx && !task_is_dead(vdev->ud.tcp_tx))
 | 
						if (vdev->ud.tcp_tx)
 | 
				
			||||||
		kthread_stop(vdev->ud.tcp_tx);
 | 
							kthread_stop_put(vdev->ud.tcp_tx);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	pr_info("stop threads\n");
 | 
						pr_info("stop threads\n");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -222,8 +222,8 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
 | 
				
			||||||
	spin_unlock(&the_controller->lock);
 | 
						spin_unlock(&the_controller->lock);
 | 
				
			||||||
	/* end the lock */
 | 
						/* end the lock */
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	vdev->ud.tcp_rx = kthread_run(vhci_rx_loop, &vdev->ud, "vhci_rx");
 | 
						vdev->ud.tcp_rx = kthread_get_run(vhci_rx_loop, &vdev->ud, "vhci_rx");
 | 
				
			||||||
	vdev->ud.tcp_tx = kthread_run(vhci_tx_loop, &vdev->ud, "vhci_tx");
 | 
						vdev->ud.tcp_tx = kthread_get_run(vhci_tx_loop, &vdev->ud, "vhci_tx");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	rh_port_connect(rhport, speed);
 | 
						rh_port_connect(rhport, speed);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue