[PATCH] mac80211: fix deadlocks in debugfs_netdev.c

[PATCH] mac80211: fix deadlocks in debugfs_netdev.c

[Luis Carlos Cobo]

The bug shows up with CONFIG_PREEMPT enabled. Pointed out by Andrew Morton.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: John Linville <linville@tuxdriver.com>
Signed-off-by: Luis Carlos Cobo <luisca@cozybit.com>
---
 net/mac80211/debugfs_netdev.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index 107b0fe..7454d0c 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -31,11 +31,13 @@ static ssize_t ieee80211_if_read(
 	ssize_t ret = -EINVAL;
 
 	read_lock(&dev_base_lock);
-	if (sdata->dev->reg_state == NETREG_REGISTERED) {
+	if (sdata->dev->reg_state == NETREG_REGISTERED)
 		ret = (*format)(sdata, buf, sizeof(buf));
-		ret = simple_read_from_buffer(userbuf, count, ppos, buf, ret);
-	}
 	read_unlock(&dev_base_lock);
+
+	if (ret != -EINVAL)
+		ret = simple_read_from_buffer(userbuf, count, ppos, buf, ret);
+
 	return ret;
 }
 
@@ -51,13 +53,13 @@ static ssize_t ieee80211_if_write(
 
 	memset(buf, 0x00, sizeof(buf));
 	buf_size = min(count, (sizeof(buf)-1));
-	read_lock(&dev_base_lock);
 	if (copy_from_user(buf, userbuf, buf_size))
-		goto endwrite;
+		return count;
+	read_lock(&dev_base_lock);
 	if (sdata->dev->reg_state == NETREG_REGISTERED)
 		(*format)(sdata, buf);
-endwrite:
 	read_unlock(&dev_base_lock);
+
 	return count;
 }
 #endif
-- 
1.5.4.3

Re: [PATCH] mac80211: fix deadlocks in debugfs_netdev.c

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]


On Mon, 2008-03-31 at 15:10 -0700, Luis Carlos Cobo wrote:
> The bug shows up with CONFIG_PREEMPT enabled. Pointed out by Andrew Morton.

> @@ -51,13 +53,13 @@ static ssize_t ieee80211_if_write(
>  
>  	memset(buf, 0x00, sizeof(buf));
>  	buf_size = min(count, (sizeof(buf)-1));
> -	read_lock(&dev_base_lock);
>  	if (copy_from_user(buf, userbuf, buf_size))

Do we actually still need all this ieee80211_if_write code? It seems
that nl80211 can fully replace it now.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] mac80211: fix deadlocks in debugfs_netdev.c

[Luis Carlos Cobo]

On Tue, 2008-04-01 at 14:02 +0200, Johannes Berg wrote:
> On Mon, 2008-03-31 at 15:10 -0700, Luis Carlos Cobo wrote:
> > The bug shows up with CONFIG_PREEMPT enabled. Pointed out by Andrew Morton.
> 
> > @@ -51,13 +53,13 @@ static ssize_t ieee80211_if_write(
> >  
> >  	memset(buf, 0x00, sizeof(buf));
> >  	buf_size = min(count, (sizeof(buf)-1));
> > -	read_lock(&dev_base_lock);
> >  	if (copy_from_user(buf, userbuf, buf_size))
> 
> Do we actually still need all this ieee80211_if_write code? It seems
> that nl80211 can fully replace it now.

Yes we do. We cannot tune mesh parameters (Mesh TTL, auto open peer
links, retries, timeouts, and other dozen parameters) through nl80211. 

As the values for those parameters are not yet defined, these knobs are
very useful for performance analysis.

-- 
Luis Carlos Cobo Rus       GnuPG ID: 44019B60
cozybit Inc.