lkml.org 
[lkml]   [2000]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectPatch: syncppp.c locking
	Hello all,

I have found a potential race condition in the syncppp.c code:
The spppq list is modified without any lock, causing possible troubles
when two devices call sppp_attach() and/or sppp_detach() simultaneously.
I have added the spppq_lock spinlock to protect the modifications of
spppq.

I am not sure about the sppp_keepalive() function, which walks
through the spppq list and protects itself using a global cli().
I think it would be better to change this to use the spinlock as well.

Is it OK to add another spinlock, or should I use some existing
spinlock instead?

Patch is relative to 2.3.46.

-Yenya

--- syncppp.c.orig Mon Feb 21 16:27:19 2000
+++ syncppp.c Mon Feb 21 16:53:45 2000
@@ -50,6 +50,7 @@
#include <linux/random.h>
#include <linux/pkt_sched.h>
#include <asm/byteorder.h>
+#include <linux/spinlock.h>
#include "syncppp.h"

#define MAXALIVECNT 6 /* max. alive packets */
@@ -126,6 +127,7 @@

static struct sppp *spppq;
static struct timer_list sppp_keepalive_timer;
+static spinlock_t spppq_lock;

static void sppp_keepalive (unsigned long dummy);
static void sppp_cp_send (struct sppp *sp, u16 proto, u8 type,
@@ -359,8 +361,8 @@
{
struct sppp *sp;
unsigned long flags;
- save_flags(flags);
- cli();
+
+ spin_lock_irqsave(&spppq_lock, flags);

for (sp=spppq; sp; sp=sp->pp_next)
{
@@ -402,7 +404,7 @@
sp->lcp.echoid, 4, &nmagic);
}
}
- restore_flags(flags);
+ spin_unlock_irqrestore(&spppq_lock, flags);
sppp_keepalive_timer.expires=jiffies+10*HZ;
add_timer(&sppp_keepalive_timer);
}
@@ -915,7 +917,9 @@
{
struct net_device *dev = pd->dev;
struct sppp *sp = &pd->sppp;
-
+ unsigned long flags;
+
+ spin_lock_irqsave(&spppq_lock, flags);
/* Initialize keepalive handler. */
if (! spppq)
{
@@ -927,6 +931,7 @@
/* Insert new entry into the keepalive list. */
sp->pp_next = spppq;
spppq = sp;
+ spin_unlock_irqrestore(&spppq_lock, flags);

sp->pp_loopcnt = 0;
sp->pp_alivecnt = 0;
@@ -971,7 +976,9 @@
void sppp_detach (struct net_device *dev)
{
struct sppp **q, *p, *sp = (struct sppp *)sppp_of(dev);
+ unsigned long flags;

+ spin_lock_irqsave(&spppq_lock, flags);
/* Remove the entry from the keepalive list. */
for (q = &spppq; (p = *q); q = &p->pp_next)
if (p == sp) {
@@ -983,6 +990,7 @@
if (! spppq)
del_timer(&sppp_keepalive_timer);
sppp_clear_timeout (sp);
+ spin_unlock_irqrestore(&spppq_lock, flags);
}

EXPORT_SYMBOL(sppp_detach);
@@ -1292,6 +1300,7 @@
{
printk(KERN_INFO "Cronyx Ltd, Synchronous PPP and CISCO HDLC (c) 1994\n");
printk(KERN_INFO "Linux port (c) 1998 Building Number Three Ltd & Jan \"Yenya\" Kasprzak.\n");
+ spin_lock_init(&spppq_lock);
sppp_packet_type.type=htons(ETH_P_WAN_PPP);
dev_add_pack(&sppp_packet_type);
}
--
\ Jan "Yenya" Kasprzak <kas at fi.muni.cz> http://www.fi.muni.cz/~kas/
\\ PGP: finger kas at aisa.fi.muni.cz 0D99A7FB206605D7 8B35FCDE05B18A5E //
\\\ Czech Linux Homepage: http://www.linux.cz/ ///
Its purely bandwidth. If it was 40 instances of Miguel reading web pages
flat out over 100baseT you would definitely be right. But its not... (Alan)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:56    [W:0.050 / U:0.224 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site