linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in orinoco_ioctl_set_genie
@ 2009-01-17 15:22 Andrey Borzenkov
  2009-01-19  3:16 ` [Orinoco-devel] " Pavel Roskin
  0 siblings, 1 reply; 4+ messages in thread
From: Andrey Borzenkov @ 2009-01-17 15:22 UTC (permalink / raw)
  To: orinoco-devel, linux-wireless

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

Subject: [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in 
orinoco_ioctl_set_genie
From: Andrey Borzenkov <arvidjaar@mail.ru>

kmalloc is called with interrupt disabled and may not sleep. This
fixes this BUG:

[   56.923623] BUG: sleeping function called from invalid context at 
/home/bor/src/linux-git/mm/slub.c:1599
[   56.923644] in_atomic(): 0, irqs_disabled(): 1, pid: 3031, name: 
wpa_supplicant
[   56.923656] 2 locks held by wpa_supplicant/3031:
[   56.923662]  #0:  (rtnl_mutex){--..}, at: [<c02abd1f>] rtnl_lock+0xf/0x20
[   56.923703]  #1:  (&priv->lock){++..}, at: [<dfc840c2>] 
orinoco_ioctl_set_genie+0x52/0x130 [orinoco]
[   56.923782] irq event stamp: 910
[   56.923788] hardirqs last  enabled at (909): [<c01957db>] 
__kmalloc+0x7b/0x140
[   56.923820] hardirqs last disabled at (910): [<c0309419>] 
_spin_lock_irqsave+0x19/0x80
[   56.923847] softirqs last  enabled at (880): [<c0124f54>] 
__do_softirq+0xc4/0x110
[   56.923865] softirqs last disabled at (871): [<c01049ae>] 
do_softirq+0x8e/0xe0
[   56.923895] Pid: 3031, comm: wpa_supplicant Not tainted 2.6.29-rc2-1avb 
#1
[   56.923905] Call Trace:
[   56.923919]  [<c01049ae>] ? do_softirq+0x8e/0xe0
[   56.923941]  [<c011ad12>] __might_sleep+0xd2/0x100
[   56.923952]  [<c0195837>] __kmalloc+0xd7/0x140
[   56.923963]  [<c030946a>] ? _spin_lock_irqsave+0x6a/0x80
[   56.923981]  [<dfc840e9>] ? orinoco_ioctl_set_genie+0x79/0x130 [orinoco]
[   56.923999]  [<dfc840c2>] ? orinoco_ioctl_set_genie+0x52/0x130 [orinoco]
[   56.924017]  [<dfc840e9>] orinoco_ioctl_set_genie+0x79/0x130 [orinoco]
[   56.924036]  [<c0209325>] ? copy_from_user+0x35/0x130
[   56.924061]  [<c02ffd96>] ioctl_standard_call+0x196/0x380
[   56.924085]  [<c029f945>] ? __dev_get_by_name+0x85/0xb0
[   56.924096]  [<c02ff88f>] wext_handle_ioctl+0x14f/0x230
[   56.924113]  [<dfc84070>] ? orinoco_ioctl_set_genie+0x0/0x130 [orinoco]
[   56.924132]  [<c02a3da5>] dev_ioctl+0x495/0x570
[   56.924155]  [<c0293e05>] ? sys_sendto+0xa5/0xd0
[   56.924171]  [<c0142fe8>] ? mark_held_locks+0x48/0x90
[   56.924183]  [<c0292880>] ? sock_ioctl+0x0/0x280
[   56.924193]  [<c029297d>] sock_ioctl+0xfd/0x280
[   56.924203]  [<c0292880>] ? sock_ioctl+0x0/0x280
[   56.924235]  [<c01a51d0>] vfs_ioctl+0x20/0x80
[   56.924246]  [<c01a53e2>] do_vfs_ioctl+0x72/0x570
[   56.924257]  [<c0293e62>] ? sys_send+0x32/0x40
[   56.924268]  [<c02947c0>] ? sys_socketcall+0x1d0/0x2a0
[   56.924280]  [<c010339f>] ? sysenter_exit+0xf/0x16
[   56.924292]  [<c01a5919>] sys_ioctl+0x39/0x70
[   56.924302]  [<c0103371>] sysenter_do_call+0x12/0x31

Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>

---

Why did not we see it before?

 drivers/net/wireless/orinoco/orinoco.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/drivers/net/wireless/orinoco/orinoco.c 
b/drivers/net/wireless/orinoco/orinoco.c
index c3bb85e..779ab10 100644
--- a/drivers/net/wireless/orinoco/orinoco.c
+++ b/drivers/net/wireless/orinoco/orinoco.c
@@ -5079,7 +5079,7 @@ static int orinoco_ioctl_set_genie(struct net_device 
*dev,
 		return -EBUSY;
 
 	if (wrqu->data.length) {
-		buf = kmalloc(wrqu->data.length, GFP_KERNEL);
+		buf = kmalloc(wrqu->data.length, GFP_ATOMIC);
 		if (buf == NULL) {
 			err = -ENOMEM;
 			goto out;


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Orinoco-devel] [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in orinoco_ioctl_set_genie
  2009-01-17 15:22 [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in orinoco_ioctl_set_genie Andrey Borzenkov
@ 2009-01-19  3:16 ` Pavel Roskin
  2009-01-20 17:26   ` Andrey Borzenkov
  0 siblings, 1 reply; 4+ messages in thread
From: Pavel Roskin @ 2009-01-19  3:16 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: orinoco-devel, linux-wireless

On Sat, 2009-01-17 at 18:22 +0300, Andrey Borzenkov wrote:
> Subject: [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in 
> orinoco_ioctl_set_genie
> From: Andrey Borzenkov <arvidjaar@mail.ru>
> 
> kmalloc is called with interrupt disabled and may not sleep. This
> fixes this BUG:

Please move kmalloc before the lock.  I don't see any reason to allocate
memory with irqs disabled.  The contention happens later, when
priv->wpa_ie is freed and assigned a new value.

-- 
Regards,
Pavel Roskin

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Orinoco-devel] [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in orinoco_ioctl_set_genie
  2009-01-19  3:16 ` [Orinoco-devel] " Pavel Roskin
@ 2009-01-20 17:26   ` Andrey Borzenkov
  2009-01-21 16:24     ` John W. Linville
  0 siblings, 1 reply; 4+ messages in thread
From: Andrey Borzenkov @ 2009-01-20 17:26 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: orinoco-devel, linux-wireless

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

On 19 января 2009 06:16:04 Pavel Roskin wrote:
> On Sat, 2009-01-17 at 18:22 +0300, Andrey Borzenkov wrote:
> > Subject: [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in
> > orinoco_ioctl_set_genie
> > From: Andrey Borzenkov <arvidjaar@mail.ru>
> >
> > kmalloc is called with interrupt disabled and may not sleep. This
> > fixes this BUG:
>
> Please move kmalloc before the lock.  I don't see any reason to
> allocate memory with irqs disabled.  The contention happens later,
> when priv->wpa_ie is freed and assigned a new value.

OK.

This looks like -stable candidate.

---

Subject: [PATCH] orinoco: move kmalloc(..., GFP_KERNEL) outside spinlock in 
orinoco_ioctl_set_genie
From: Andrey Borzenkov <arvidjaar@mail.ru>

As pointed out by Pavel Roskin, it need not be in critical section. Move
it outside of spinlock. This fixes this BUG:

[   56.923623] BUG: sleeping function called from invalid context at 
/home/bor/src/linux-git/mm/slub.c:1599
[   56.923644] in_atomic(): 0, irqs_disabled(): 1, pid: 3031, name: 
wpa_supplicant
[   56.923656] 2 locks held by wpa_supplicant/3031:
[   56.923662]  #0:  (rtnl_mutex){--..}, at: [<c02abd1f>] rtnl_lock+0xf/0x20
[   56.923703]  #1:  (&priv->lock){++..}, at: [<dfc840c2>] 
orinoco_ioctl_set_genie+0x52/0x130 [orinoco]
[   56.923782] irq event stamp: 910
[   56.923788] hardirqs last  enabled at (909): [<c01957db>] 
__kmalloc+0x7b/0x140
[   56.923820] hardirqs last disabled at (910): [<c0309419>] 
_spin_lock_irqsave+0x19/0x80
[   56.923847] softirqs last  enabled at (880): [<c0124f54>] 
__do_softirq+0xc4/0x110
[   56.923865] softirqs last disabled at (871): [<c01049ae>] 
do_softirq+0x8e/0xe0
[   56.923895] Pid: 3031, comm: wpa_supplicant Not tainted 2.6.29-rc2-1avb 
#1
[   56.923905] Call Trace:
[   56.923919]  [<c01049ae>] ? do_softirq+0x8e/0xe0
[   56.923941]  [<c011ad12>] __might_sleep+0xd2/0x100
[   56.923952]  [<c0195837>] __kmalloc+0xd7/0x140
[   56.923963]  [<c030946a>] ? _spin_lock_irqsave+0x6a/0x80
[   56.923981]  [<dfc840e9>] ? orinoco_ioctl_set_genie+0x79/0x130 [orinoco]
[   56.923999]  [<dfc840c2>] ? orinoco_ioctl_set_genie+0x52/0x130 [orinoco]
[   56.924017]  [<dfc840e9>] orinoco_ioctl_set_genie+0x79/0x130 [orinoco]
[   56.924036]  [<c0209325>] ? copy_from_user+0x35/0x130
[   56.924061]  [<c02ffd96>] ioctl_standard_call+0x196/0x380
[   56.924085]  [<c029f945>] ? __dev_get_by_name+0x85/0xb0
[   56.924096]  [<c02ff88f>] wext_handle_ioctl+0x14f/0x230
[   56.924113]  [<dfc84070>] ? orinoco_ioctl_set_genie+0x0/0x130 [orinoco]
[   56.924132]  [<c02a3da5>] dev_ioctl+0x495/0x570
[   56.924155]  [<c0293e05>] ? sys_sendto+0xa5/0xd0
[   56.924171]  [<c0142fe8>] ? mark_held_locks+0x48/0x90
[   56.924183]  [<c0292880>] ? sock_ioctl+0x0/0x280
[   56.924193]  [<c029297d>] sock_ioctl+0xfd/0x280
[   56.924203]  [<c0292880>] ? sock_ioctl+0x0/0x280
[   56.924235]  [<c01a51d0>] vfs_ioctl+0x20/0x80
[   56.924246]  [<c01a53e2>] do_vfs_ioctl+0x72/0x570
[   56.924257]  [<c0293e62>] ? sys_send+0x32/0x40
[   56.924268]  [<c02947c0>] ? sys_socketcall+0x1d0/0x2a0
[   56.924280]  [<c010339f>] ? sysenter_exit+0xf/0x16
[   56.924292]  [<c01a5919>] sys_ioctl+0x39/0x70
[   56.924302]  [<c0103371>] sysenter_do_call+0x12/0x31

Signed-off-by: Andrey Borzenkov <arvidjaar@mail.ru>

---

 drivers/net/wireless/orinoco/orinoco.c |   30 
+++++++++++++-----------------
 1 files changed, 13 insertions(+), 17 deletions(-)


diff --git a/drivers/net/wireless/orinoco/orinoco.c 
b/drivers/net/wireless/orinoco/orinoco.c
index c3bb85e..39eab39 100644
--- a/drivers/net/wireless/orinoco/orinoco.c
+++ b/drivers/net/wireless/orinoco/orinoco.c
@@ -5068,33 +5068,30 @@ static int orinoco_ioctl_set_genie(struct net_device 
*dev,
 	struct orinoco_private *priv = netdev_priv(dev);
 	u8 *buf;
 	unsigned long flags;
-	int err = 0;
 
 	/* cut off at IEEE80211_MAX_DATA_LEN */
 	if ((wrqu->data.length > IEEE80211_MAX_DATA_LEN) ||
 	    (wrqu->data.length && (extra == NULL)))
 		return -EINVAL;
 
-	if (orinoco_lock(priv, &flags) != 0)
-		return -EBUSY;
-
 	if (wrqu->data.length) {
 		buf = kmalloc(wrqu->data.length, GFP_KERNEL);
-		if (buf == NULL) {
-			err = -ENOMEM;
-			goto out;
-		}
+		if (buf == NULL)
+			return -ENOMEM;
 
 		memcpy(buf, extra, wrqu->data.length);
-		kfree(priv->wpa_ie);
-		priv->wpa_ie = buf;
-		priv->wpa_ie_len = wrqu->data.length;
-	} else {
-		kfree(priv->wpa_ie);
-		priv->wpa_ie = NULL;
-		priv->wpa_ie_len = 0;
+	} else
+		buf = NULL;
+
+	if (orinoco_lock(priv, &flags) != 0) {
+		kfree(buf);
+		return -EBUSY;
 	}
 
+	kfree(priv->wpa_ie);
+	priv->wpa_ie = buf;
+	priv->wpa_ie_len = wrqu->data.length;
+
 	if (priv->wpa_ie) {
 		/* Looks like wl_lkm wants to check the auth alg, and
 		 * somehow pass it to the firmware.
@@ -5103,9 +5100,8 @@ static int orinoco_ioctl_set_genie(struct net_device 
*dev,
 		 */
 	}
 
-out:
 	orinoco_unlock(priv, &flags);
-	return err;
+	return 0;
 }
 
 static int orinoco_ioctl_get_genie(struct net_device *dev,


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Orinoco-devel] [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in orinoco_ioctl_set_genie
  2009-01-20 17:26   ` Andrey Borzenkov
@ 2009-01-21 16:24     ` John W. Linville
  0 siblings, 0 replies; 4+ messages in thread
From: John W. Linville @ 2009-01-21 16:24 UTC (permalink / raw)
  To: Andrey Borzenkov; +Cc: Pavel Roskin, orinoco-devel, linux-wireless

On Tue, Jan 20, 2009 at 08:26:46PM +0300, Andrey Borzenkov wrote:
> On 19 =D1=8F=D0=BD=D0=B2=D0=B0=D1=80=D1=8F 2009 06:16:04 Pavel Roskin=
 wrote:
> > On Sat, 2009-01-17 at 18:22 +0300, Andrey Borzenkov wrote:
> > > Subject: [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc =
in
> > > orinoco_ioctl_set_genie
> > > From: Andrey Borzenkov <arvidjaar@mail.ru>
> > >
> > > kmalloc is called with interrupt disabled and may not sleep. This
> > > fixes this BUG:
> >
> > Please move kmalloc before the lock.  I don't see any reason to
> > allocate memory with irqs disabled.  The contention happens later,
> > when priv->wpa_ie is freed and assigned a new value.
>=20
> OK.
>=20
> This looks like -stable candidate.

Just FYI, the best way to indicate is to add a line like this after
your Signed-off-by:

Cc: stable@kernel.org

I'll take care of this one.

Thanks,

John
--=20
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-01-21 16:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-17 15:22 [PATCH] [2.6.29-rc2] orinoco: use GFP_ATOMIC in kmalloc in orinoco_ioctl_set_genie Andrey Borzenkov
2009-01-19  3:16 ` [Orinoco-devel] " Pavel Roskin
2009-01-20 17:26   ` Andrey Borzenkov
2009-01-21 16:24     ` John W. Linville

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).