* [PATCH] 2.4.18 Eicon ISDN driver fix.
@ 2002-02-26 19:26 ` petter wahlman
2002-02-26 19:49 ` petter wahlman
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: petter wahlman @ 2002-02-26 19:26 UTC (permalink / raw)
To: linux-kernel; +Cc: info
The following code is calling a possibly blocking operation while
holding a spinlock.
Petter Wahlman
--- linux-2.4.18/drivers/isdn/eicon/eicon_mod.c Fri Dec 21 18:41:54 2001
+++ linux-2.4.18-pw/drivers/isdn/eicon/eicon_mod.c Mon Feb 25
23:45:05 2002
@@ -665,8 +665,11 @@
else
cnt = skb->len;
- if (user)
+ if (user) {
+ spin_unlock_irqrestore(&eicon_lock,
flags);
copy_to_user(p, skb->data, cnt);
+ spin_lock_irqsave(&eicon_lock, flags);
+ }
else
memcpy(p, skb->data, cnt);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 2.4.18 Eicon ISDN driver fix.
2002-02-26 19:26 ` petter wahlman
@ 2002-02-26 19:49 ` petter wahlman
2002-02-26 20:50 ` Alan Cox
2002-02-27 7:58 ` Armin Schindler
2 siblings, 0 replies; 8+ messages in thread
From: petter wahlman @ 2002-02-26 19:49 UTC (permalink / raw)
To: Dave Jones; +Cc: linux-kernel, info
On Tue, 2002-02-26 at 20:54, Dave Jones wrote:
> On Tue, Feb 26, 2002 at 08:26:18PM +0100, petter wahlman wrote:
> > +++ linux-2.4.18-pw/drivers/isdn/eicon/eicon_mod.c Mon Feb 25
>
> > - if (user)
> > + if (user) {
> > + spin_unlock_irqrestore(&eicon_lock,
> > flags);
> > copy_to_user(p, skb->data, cnt);
> > + spin_lock_irqsave(&eicon_lock, flags);
> > + }
>
> What happens if something else adds/removes to card->statq, or
> frees the skb after you drop the lock? I'm not familiar with
> this code, but from a quick look, it looks like this introduces
> a race no ?
>
Yes, it will introduce a new race.
I did not actually intend to send this unfinished patch, but fscked up
:)
Anyway, calling copy_to_user while holding a spinlock is defiantly a bad
idea.
> --
> | Dave Jones. http://www.codemonkey.org.uk
> | SuSE Labs
>
Petter Wahlman.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 2.4.18 Eicon ISDN driver fix.
2002-02-26 19:26 ` petter wahlman
@ 2002-02-26 19:54 Dave Jones
2002-02-26 19:26 ` petter wahlman
2 siblings, 1 reply; 8+ messages in thread
From: Dave Jones @ 2002-02-26 19:54 UTC (permalink / raw)
To: petter wahlman; +Cc: linux-kernel, info
On Tue, Feb 26, 2002 at 08:26:18PM +0100, petter wahlman wrote:
> +++ linux-2.4.18-pw/drivers/isdn/eicon/eicon_mod.c Mon Feb 25
> - if (user)
> + if (user) {
> + spin_unlock_irqrestore(&eicon_lock,
> flags);
> copy_to_user(p, skb->data, cnt);
> + spin_lock_irqsave(&eicon_lock, flags);
> + }
What happens if something else adds/removes to card->statq, or
frees the skb after you drop the lock? I'm not familiar with
this code, but from a quick look, it looks like this introduces
a race no ?
--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 2.4.18 Eicon ISDN driver fix.
2002-02-26 19:26 ` petter wahlman
2002-02-26 19:49 ` petter wahlman
@ 2002-02-26 20:50 ` Alan Cox
2002-02-27 7:58 ` Armin Schindler
2 siblings, 0 replies; 8+ messages in thread
From: Alan Cox @ 2002-02-26 20:50 UTC (permalink / raw)
To: petter wahlman; +Cc: linux-kernel, info
> The following code is calling a possibly blocking operation while
> holding a spinlock.
Definitely a bug - but can you prove dropping the lock there is safe ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 2.4.18 Eicon ISDN driver fix.
2002-02-26 19:26 ` petter wahlman
2002-02-26 19:49 ` petter wahlman
2002-02-26 20:50 ` Alan Cox
@ 2002-02-27 7:58 ` Armin Schindler
2002-02-28 17:41 ` Marcelo Tosatti
2 siblings, 1 reply; 8+ messages in thread
From: Armin Schindler @ 2002-02-27 7:58 UTC (permalink / raw)
To: Marcelo Tosatti, Alan Cox; +Cc: Linux Kernel Mailinglist, info, petter wahlman
The patch below fixes the race condition with copy_to_user and will
not introduce a new race. What can happen is that two reader-processes
may get mixed-up messages, but more than one reader isn't allowed here
anyway.
Please apply this patch to 2.4 and 2.2, it works for both.
Thanx,
Armin
On 26 Feb 2002, petter wahlman wrote:
> The following code is calling a possibly blocking operation while
> holding a spinlock.
>
>
> Petter Wahlman
--- linux-2.4.18/drivers/isdn/eicon/eicon_mod.c Fri Dec 21 18:41:54 2001
+++ linux-2.4.18-pw/drivers/isdn/eicon/eicon_mod.c Mon Feb 25
23:45:05 2002
@@ -665,8 +665,11 @@
else
cnt = skb->len;
- if (user)
+ if (user) {
+ spin_unlock_irqrestore(&eicon_lock,
flags);
copy_to_user(p, skb->data, cnt);
+ spin_lock_irqsave(&eicon_lock, flags);
+ }
else
memcpy(p, skb->data, cnt);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 2.4.18 Eicon ISDN driver fix.
2002-02-27 7:58 ` Armin Schindler
@ 2002-02-28 17:41 ` Marcelo Tosatti
2002-02-28 21:14 ` petter wahlman
2002-03-01 7:30 ` Armin Schindler
0 siblings, 2 replies; 8+ messages in thread
From: Marcelo Tosatti @ 2002-02-28 17:41 UTC (permalink / raw)
To: Armin Schindler; +Cc: Alan Cox, Linux Kernel Mailinglist, info, petter wahlman
On Wed, 27 Feb 2002, Armin Schindler wrote:
> The patch below fixes the race condition with copy_to_user and will
> not introduce a new race. What can happen is that two reader-processes
> may get mixed-up messages, but more than one reader isn't allowed here
> anyway.
>
> Please apply this patch to 2.4 and 2.2, it works for both.
Armin,
Your patch does not apply cleanly against my tree.
Please regenerate it.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 2.4.18 Eicon ISDN driver fix.
2002-02-28 17:41 ` Marcelo Tosatti
@ 2002-02-28 21:14 ` petter wahlman
2002-03-01 7:30 ` Armin Schindler
1 sibling, 0 replies; 8+ messages in thread
From: petter wahlman @ 2002-02-28 21:14 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Armin Schindler, Alan Cox, Linux Kernel Mailinglist, info
On Thu, 2002-02-28 at 18:41, Marcelo Tosatti wrote:
>
>
> On Wed, 27 Feb 2002, Armin Schindler wrote:
>
> > The patch below fixes the race condition with copy_to_user and will
> > not introduce a new race. What can happen is that two reader-processes
> > may get mixed-up messages, but more than one reader isn't allowed here
> > anyway.
> >
> > Please apply this patch to 2.4 and 2.2, it works for both.
>
> Armin,
>
> Your patch does not apply cleanly against my tree.
>
> Please regenerate it.
>
--- linux/drivers/isdn/eicon/eicon_mod.c Fri Dec 21 18:41:54 2001
+++ linux-2.4.18-pw/drivers/isdn/eicon/eicon_mod.c Thu Feb 28
21:48:32 2002
@@ -665,8 +665,11 @@
else
cnt = skb->len;
- if (user)
+ if (user) {
+ spin_unlock_irqrestore(&eicon_lock,
flags);
copy_to_user(p, skb->data, cnt);
+ spin_lock_irqsave(&eicon_lock, flags);
+ }
else
memcpy(p, skb->data, cnt);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] 2.4.18 Eicon ISDN driver fix.
2002-02-28 17:41 ` Marcelo Tosatti
2002-02-28 21:14 ` petter wahlman
@ 2002-03-01 7:30 ` Armin Schindler
1 sibling, 0 replies; 8+ messages in thread
From: Armin Schindler @ 2002-03-01 7:30 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Alan Cox, Linux Kernel Mailinglist, info, petter wahlman
On Thu, 28 Feb 2002, Marcelo Tosatti wrote:
>
>
> On Wed, 27 Feb 2002, Armin Schindler wrote:
>
> > The patch below fixes the race condition with copy_to_user and will
> > not introduce a new race. What can happen is that two reader-processes
> > may get mixed-up messages, but more than one reader isn't allowed here
> > anyway.
> >
> > Please apply this patch to 2.4 and 2.2, it works for both.
>
> Armin,
>
> Your patch does not apply cleanly against my tree.
>
> Please regenerate it.
Hi Marcelo,
I don't why the patch does not apply, maybe whitespace problems ?
Okay, here it is again. I tested it with 2.4.18.
Thanks Armin
diff -Nurb pristine/drivers/isdn/eicon/eicon_mod.c linux/drivers/isdn/eicon/eicon_mod.c
--- pristine/drivers/isdn/eicon/eicon_mod.c Fri Dec 21 18:41:54 2001
+++ linux/drivers/isdn/eicon/eicon_mod.c Fri Mar 1 08:07:44 2002
@@ -665,8 +665,11 @@
else
cnt = skb->len;
- if (user)
+ if (user) {
+ spin_unlock_irqrestore(&eicon_lock, flags);
copy_to_user(p, skb->data, cnt);
+ spin_lock_irqsave(&eicon_lock, flags);
+ }
else
memcpy(p, skb->data, cnt);
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2002-03-01 7:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-02-26 19:54 [PATCH] 2.4.18 Eicon ISDN driver fix Dave Jones
2002-02-26 19:26 ` petter wahlman
2002-02-26 19:49 ` petter wahlman
2002-02-26 20:50 ` Alan Cox
2002-02-27 7:58 ` Armin Schindler
2002-02-28 17:41 ` Marcelo Tosatti
2002-02-28 21:14 ` petter wahlman
2002-03-01 7:30 ` Armin Schindler
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).