Skip to content

Commit

Permalink
net: lapb: Add locking to the lapb module
Browse files Browse the repository at this point in the history 
In the lapb module, the timers may run concurrently with other code in
this module, and there is currently no locking to prevent the code from
racing on "struct lapb_cb". This patch adds locking to prevent racing.

1. Add "spinlock_t lock" to "struct lapb_cb"; Add "spin_lock_bh" and
"spin_unlock_bh" to APIs, timer functions and notifier functions.

2. Add "bool t1timer_stop, t2timer_stop" to "struct lapb_cb" to make us
able to ask running timers to abort; Modify "lapb_stop_t1timer" and
"lapb_stop_t2timer" to make them able to abort running timers;
Modify "lapb_t2timer_expiry" and "lapb_t1timer_expiry" to make them
abort after they are stopped by "lapb_stop_t1timer", "lapb_stop_t2timer",
and "lapb_start_t1timer", "lapb_start_t2timer".

3. Let lapb_unregister wait for other API functions and running timers
to stop.

4. The lapb_device_event function calls lapb_disconnect_request. In
order to avoid trying to hold the lock twice, add a new function named
"__lapb_disconnect_request" which assumes the lock is held, and make
it called by lapb_disconnect_request and lapb_device_event.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: Martin Schiller <[email protected]>
Signed-off-by: Xie He <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jakub Kicinski <[email protected]>
  • Loading branch information
@kuba-moo
Xie He authored and kuba-moo committed Jan 27, 2021
1 parent f0947d0 commit b491e6a
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 20 deletions.
2 changes: 2 additions & 0 deletions include/net/lapb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ struct lapb_cb {
unsigned short n2, n2count;
unsigned short t1, t2;
struct timer_list t1timer, t2timer;
bool t1timer_stop, t2timer_stop;

/* Internal control information */
struct sk_buff_head write_queue;
Expand All @@ -103,6 +104,7 @@ struct lapb_cb {
struct lapb_frame frmr_data;
unsigned char frmr_type;

spinlock_t lock;
refcount_t refcnt;
};

Expand Down
70 changes: 54 additions & 16 deletions net/lapb/lapb_iface.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,17 @@ static struct lapb_cb *lapb_create_cb(void)

timer_setup(&lapb->t1timer, NULL, 0);
timer_setup(&lapb->t2timer, NULL, 0);
lapb->t1timer_stop = true;
lapb->t2timer_stop = true;

lapb->t1 = LAPB_DEFAULT_T1;
lapb->t2 = LAPB_DEFAULT_T2;
lapb->n2 = LAPB_DEFAULT_N2;
lapb->mode = LAPB_DEFAULT_MODE;
lapb->window = LAPB_DEFAULT_WINDOW;
lapb->state = LAPB_STATE_0;

spin_lock_init(&lapb->lock);
refcount_set(&lapb->refcnt, 1);
out:
return lapb;
Expand Down Expand Up @@ -178,11 +182,23 @@ int lapb_unregister(struct net_device *dev)
goto out;
lapb_put(lapb);

/* Wait for other refs to "lapb" to drop */
while (refcount_read(&lapb->refcnt) > 2)
usleep_range(1, 10);

spin_lock_bh(&lapb->lock);

lapb_stop_t1timer(lapb);
lapb_stop_t2timer(lapb);

lapb_clear_queues(lapb);

spin_unlock_bh(&lapb->lock);

/* Wait for running timers to stop */
del_timer_sync(&lapb->t1timer);
del_timer_sync(&lapb->t2timer);

__lapb_remove_cb(lapb);

lapb_put(lapb);
Expand All @@ -201,6 +217,8 @@ int lapb_getparms(struct net_device *dev, struct lapb_parms_struct *parms)
if (!lapb)
goto out;

spin_lock_bh(&lapb->lock);

parms->t1 = lapb->t1 / HZ;
parms->t2 = lapb->t2 / HZ;
parms->n2 = lapb->n2;
Expand All @@ -219,6 +237,7 @@ int lapb_getparms(struct net_device *dev, struct lapb_parms_struct *parms)
else
parms->t2timer = (lapb->t2timer.expires - jiffies) / HZ;

spin_unlock_bh(&lapb->lock);
lapb_put(lapb);
rc = LAPB_OK;
out:
Expand All @@ -234,6 +253,8 @@ int lapb_setparms(struct net_device *dev, struct lapb_parms_struct *parms)
if (!lapb)
goto out;

spin_lock_bh(&lapb->lock);

rc = LAPB_INVALUE;
if (parms->t1 < 1 || parms->t2 < 1 || parms->n2 < 1)
goto out_put;
Expand All @@ -256,6 +277,7 @@ int lapb_setparms(struct net_device *dev, struct lapb_parms_struct *parms)

rc = LAPB_OK;
out_put:
spin_unlock_bh(&lapb->lock);
lapb_put(lapb);
out:
return rc;
Expand All @@ -270,6 +292,8 @@ int lapb_connect_request(struct net_device *dev)
if (!lapb)
goto out;

spin_lock_bh(&lapb->lock);

rc = LAPB_OK;
if (lapb->state == LAPB_STATE_1)
goto out_put;
Expand All @@ -285,37 +309,29 @@ int lapb_connect_request(struct net_device *dev)

rc = LAPB_OK;
out_put:
spin_unlock_bh(&lapb->lock);
lapb_put(lapb);
out:
return rc;
}
EXPORT_SYMBOL(lapb_connect_request);

int lapb_disconnect_request(struct net_device *dev)
static int __lapb_disconnect_request(struct lapb_cb *lapb)
{
struct lapb_cb *lapb = lapb_devtostruct(dev);
int rc = LAPB_BADTOKEN;

if (!lapb)
goto out;

switch (lapb->state) {
case LAPB_STATE_0:
rc = LAPB_NOTCONNECTED;
goto out_put;
return LAPB_NOTCONNECTED;

case LAPB_STATE_1:
lapb_dbg(1, "(%p) S1 TX DISC(1)\n", lapb->dev);
lapb_dbg(0, "(%p) S1 -> S0\n", lapb->dev);
lapb_send_control(lapb, LAPB_DISC, LAPB_POLLON, LAPB_COMMAND);
lapb->state = LAPB_STATE_0;
lapb_start_t1timer(lapb);
rc = LAPB_NOTCONNECTED;
goto out_put;
return LAPB_NOTCONNECTED;

case LAPB_STATE_2:
rc = LAPB_OK;
goto out_put;
return LAPB_OK;
}

lapb_clear_queues(lapb);
Expand All @@ -328,8 +344,22 @@ int lapb_disconnect_request(struct net_device *dev)
lapb_dbg(1, "(%p) S3 DISC(1)\n", lapb->dev);
lapb_dbg(0, "(%p) S3 -> S2\n", lapb->dev);

rc = LAPB_OK;
out_put:
return LAPB_OK;
}

int lapb_disconnect_request(struct net_device *dev)
{
struct lapb_cb *lapb = lapb_devtostruct(dev);
int rc = LAPB_BADTOKEN;

if (!lapb)
goto out;

spin_lock_bh(&lapb->lock);

rc = __lapb_disconnect_request(lapb);

spin_unlock_bh(&lapb->lock);
lapb_put(lapb);
out:
return rc;
Expand All @@ -344,6 +374,8 @@ int lapb_data_request(struct net_device *dev, struct sk_buff *skb)
if (!lapb)
goto out;

spin_lock_bh(&lapb->lock);

rc = LAPB_NOTCONNECTED;
if (lapb->state != LAPB_STATE_3 && lapb->state != LAPB_STATE_4)
goto out_put;
Expand All @@ -352,6 +384,7 @@ int lapb_data_request(struct net_device *dev, struct sk_buff *skb)
lapb_kick(lapb);
rc = LAPB_OK;
out_put:
spin_unlock_bh(&lapb->lock);
lapb_put(lapb);
out:
return rc;
Expand All @@ -364,7 +397,9 @@ int lapb_data_received(struct net_device *dev, struct sk_buff *skb)
int rc = LAPB_BADTOKEN;

if (lapb) {
spin_lock_bh(&lapb->lock);
lapb_data_input(lapb, skb);
spin_unlock_bh(&lapb->lock);
lapb_put(lapb);
rc = LAPB_OK;
}
Expand Down Expand Up @@ -435,6 +470,8 @@ static int lapb_device_event(struct notifier_block *this, unsigned long event,
if (!lapb)
return NOTIFY_DONE;

spin_lock_bh(&lapb->lock);

switch (event) {
case NETDEV_UP:
lapb_dbg(0, "(%p) Interface up: %s\n", dev, dev->name);
Expand All @@ -454,7 +491,7 @@ static int lapb_device_event(struct notifier_block *this, unsigned long event,
break;
case NETDEV_GOING_DOWN:
if (netif_carrier_ok(dev))
lapb_disconnect_request(dev);
__lapb_disconnect_request(lapb);
break;
case NETDEV_DOWN:
lapb_dbg(0, "(%p) Interface down: %s\n", dev, dev->name);
Expand Down Expand Up @@ -489,6 +526,7 @@ static int lapb_device_event(struct notifier_block *this, unsigned long event,
break;
}

spin_unlock_bh(&lapb->lock);
lapb_put(lapb);
return NOTIFY_DONE;
}
Expand Down
30 changes: 26 additions & 4 deletions net/lapb/lapb_timer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ void lapb_start_t1timer(struct lapb_cb *lapb)
lapb->t1timer.function = lapb_t1timer_expiry;
lapb->t1timer.expires = jiffies + lapb->t1;

lapb->t1timer_stop = false;
add_timer(&lapb->t1timer);
}

Expand All @@ -50,16 +51,19 @@ void lapb_start_t2timer(struct lapb_cb *lapb)
lapb->t2timer.function = lapb_t2timer_expiry;
lapb->t2timer.expires = jiffies + lapb->t2;

lapb->t2timer_stop = false;
add_timer(&lapb->t2timer);
}

void lapb_stop_t1timer(struct lapb_cb *lapb)
{
lapb->t1timer_stop = true;
del_timer(&lapb->t1timer);
}

void lapb_stop_t2timer(struct lapb_cb *lapb)
{
lapb->t2timer_stop = true;
del_timer(&lapb->t2timer);
}

Expand All @@ -72,16 +76,31 @@ static void lapb_t2timer_expiry(struct timer_list *t)
{
struct lapb_cb *lapb = from_timer(lapb, t, t2timer);

spin_lock_bh(&lapb->lock);
if (timer_pending(&lapb->t2timer)) /* A new timer has been set up */
goto out;
if (lapb->t2timer_stop) /* The timer has been stopped */
goto out;

if (lapb->condition & LAPB_ACK_PENDING_CONDITION) {
lapb->condition &= ~LAPB_ACK_PENDING_CONDITION;
lapb_timeout_response(lapb);
}

out:
spin_unlock_bh(&lapb->lock);
}

static void lapb_t1timer_expiry(struct timer_list *t)
{
struct lapb_cb *lapb = from_timer(lapb, t, t1timer);

spin_lock_bh(&lapb->lock);
if (timer_pending(&lapb->t1timer)) /* A new timer has been set up */
goto out;
if (lapb->t1timer_stop) /* The timer has been stopped */
goto out;

switch (lapb->state) {

/*
Expand All @@ -108,7 +127,7 @@ static void lapb_t1timer_expiry(struct timer_list *t)
lapb->state = LAPB_STATE_0;
lapb_disconnect_indication(lapb, LAPB_TIMEDOUT);
lapb_dbg(0, "(%p) S1 -> S0\n", lapb->dev);
return;
goto out;
} else {
lapb->n2count++;
if (lapb->mode & LAPB_EXTENDED) {
Expand All @@ -132,7 +151,7 @@ static void lapb_t1timer_expiry(struct timer_list *t)
lapb->state = LAPB_STATE_0;
lapb_disconnect_confirmation(lapb, LAPB_TIMEDOUT);
lapb_dbg(0, "(%p) S2 -> S0\n", lapb->dev);
return;
goto out;
} else {
lapb->n2count++;
lapb_dbg(1, "(%p) S2 TX DISC(1)\n", lapb->dev);
Expand All @@ -150,7 +169,7 @@ static void lapb_t1timer_expiry(struct timer_list *t)
lapb_stop_t2timer(lapb);
lapb_disconnect_indication(lapb, LAPB_TIMEDOUT);
lapb_dbg(0, "(%p) S3 -> S0\n", lapb->dev);
return;
goto out;
} else {
lapb->n2count++;
lapb_requeue_frames(lapb);
Expand All @@ -167,7 +186,7 @@ static void lapb_t1timer_expiry(struct timer_list *t)
lapb->state = LAPB_STATE_0;
lapb_disconnect_indication(lapb, LAPB_TIMEDOUT);
lapb_dbg(0, "(%p) S4 -> S0\n", lapb->dev);
return;
goto out;
} else {
lapb->n2count++;
lapb_transmit_frmr(lapb);
Expand All @@ -176,4 +195,7 @@ static void lapb_t1timer_expiry(struct timer_list *t)
}

lapb_start_t1timer(lapb);

out:
spin_unlock_bh(&lapb->lock);
}

0 comments on commit b491e6a

Please sign in to comment.