linux-kernel - [PATCH 2/3] uml: fix proc-vs-interrupt context spinlock deadlock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 06 Aug 2006 17:47:03 +0200
From:	Paolo 'Blaisorblade' Giarrusso <blaisorblade@...oo.it>
To:	Andrew Morton <akpm@...l.org>
Cc:	Jeff Dike <jdike@...toit.com>,
	user-mode-linux-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: [PATCH 2/3] uml: fix proc-vs-interrupt context spinlock deadlock

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@...oo.it>

This spinlock can be taken on interrupt too, so spin_lock_irq[save] must be used.

However, Documentation/networking/netdevices.txt explains we are called with
rtnl_lock() held - so we don't need to care about other concurrent opens.
Verified also in LDD3 and by direct checking. Also verified that the network
layer (through a state machine) guarantees us that nobody will close the
interface while it's being used. Please correct me if I'm wrong.

Also, we must check we don't sleep with irqs disabled!!! But anyway, this is not
news - we already can't sleep while holding a spinlock. Who says this is
guaranted really by the present code?

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@...oo.it>
---

 arch/um/drivers/net_kern.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/um/drivers/net_kern.c b/arch/um/drivers/net_kern.c
index 6af229c..f5fba74 100644
--- a/arch/um/drivers/net_kern.c
+++ b/arch/um/drivers/net_kern.c
@@ -109,8 +109,6 @@ static int uml_net_open(struct net_devic
 	struct uml_net_private *lp = dev->priv;
 	int err;
 
-	spin_lock(&lp->lock);
-
 	if(lp->fd >= 0){
 		err = -ENXIO;
 		goto out;
@@ -144,8 +142,6 @@ static int uml_net_open(struct net_devic
 	 */
 	while((err = uml_net_rx(dev)) > 0) ;
 
-	spin_unlock(&lp->lock);
-
 	spin_lock(&opened_lock);
 	list_add(&lp->list, &opened);
 	spin_unlock(&opened_lock);
@@ -155,7 +151,6 @@ out_close:
 	if(lp->close != NULL) (*lp->close)(lp->fd, &lp->user);
 	lp->fd = -1;
 out:
-	spin_unlock(&lp->lock);
 	return err;
 }
 
@@ -164,15 +159,12 @@ static int uml_net_close(struct net_devi
 	struct uml_net_private *lp = dev->priv;
 	
 	netif_stop_queue(dev);
-	spin_lock(&lp->lock);
 
 	free_irq(dev->irq, dev);
 	if(lp->close != NULL)
 		(*lp->close)(lp->fd, &lp->user);
 	lp->fd = -1;
 
-	spin_unlock(&lp->lock);
-
 	spin_lock(&opened_lock);
 	list_del(&lp->list);
 	spin_unlock(&opened_lock);
@@ -241,9 +233,9 @@ static int uml_net_set_mac(struct net_de
 	struct uml_net_private *lp = dev->priv;
 	struct sockaddr *hwaddr = addr;
 
-	spin_lock(&lp->lock);
+	spin_lock_irq(&lp->lock);
 	memcpy(dev->dev_addr, hwaddr->sa_data, ETH_ALEN);
-	spin_unlock(&lp->lock);
+	spin_unlock_irq(&lp->lock);
 
 	return(0);
 }
@@ -253,7 +245,7 @@ static int uml_net_change_mtu(struct net
 	struct uml_net_private *lp = dev->priv;
 	int err = 0;
 
-	spin_lock(&lp->lock);
+	spin_lock_irq(&lp->lock);
 
 	new_mtu = (*lp->set_mtu)(new_mtu, &lp->user);
 	if(new_mtu < 0){
@@ -264,7 +256,7 @@ static int uml_net_change_mtu(struct net
 	dev->mtu = new_mtu;
 
  out:
-	spin_unlock(&lp->lock);
+	spin_unlock_irq(&lp->lock);
 	return err;
 }
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/