Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver

Jarek Poplawski
2008-02-19T23:25:42Z

On Wed, Feb 20, 2008 at 12:06:40AM +0100, Jarek Poplawski wrote:
...
> (testing patch #1)

SORRY!!! ----> take 2 (unlocking fixed)

---

drivers/net/ppp_generic.c | 39 +++++++++++++++++++++++++++++----------
1 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index 4dc5b4b..c4e3808 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -1473,7 +1473,7 @@ void
ppp_input(struct ppp_channel *chan, struct sk_buff *skb)
{
struct channel *pch = chan->ppp;
- int proto;
+ int proto, locked;

if (!pch || skb->len == 0) {
kfree_skb(skb);
@@ -1481,8 +1481,13 @@ ppp_input(struct ppp_channel *chan, struct sk_buff *skb)
}

proto = PPP_PROTO(skb);
- read_lock_bh(&pch->upl);
- if (!pch->ppp || proto >= 0xc000 || proto == PPP_CCPFRAG) {
+ /*
+ * We use trylock to avoid dependency between soft-irq-safe upl lock
+ * and soft-irq-unsafe sk_dst_lock.
+ */
+ local_bh_disable();
+ locked = read_trylock(&pch->upl);
+ if (!locked || !pch->ppp || proto >= 0xc000 || proto == PPP_CCPFRAG) {
/* put it on the channel queue */
skb_queue_tail(&pch->file.rq, skb);
/* drop old frames if queue too long */
@@ -1493,7 +1498,10 @@ ppp_input(struct ppp_channel *chan, struct sk_buff *skb)
} else {
ppp_do_recv(pch->ppp, skb, pch);
}
- read_unlock_bh(&pch->upl);
+
+ if (locked)
+ read_unlock(&pch->upl);
+ local_bh_enable();
}

/* Put a 0-length skb in the receive queue as an error indication */
@@ -1502,12 +1510,14 @@ ppp_input_error(struct ppp_channel *chan, int code)
{
struct channel *pch = chan->ppp;
struct sk_buff *skb;
+ int locked;

if (!pch)
return;

- read_lock_bh(&pch->upl);
- if (pch->ppp) {
+ /* a trylock comment in ppp_input() */
+ local_bh_disable();
+ if ((locked = read_trylock(&pch->upl)) && pch->ppp) {
skb = alloc_skb(0, GFP_ATOMIC);
if (skb) {
skb->len = 0; /* probably unnecessary */
@@ -1515,7 +1525,10 @@ ppp_input_error(struct ppp_channel *chan, int code)
ppp_do_recv(pch->ppp, skb, pch);
}
}
- read_unlock_bh(&pch->upl);
+
+ if (locked)
+ read_unlock(&pch->upl);
+ local_bh_enable();
}

/*
@@ -2044,10 +2057,16 @@ int ppp_unit_number(struct ppp_channel *chan)
int unit = -1;

if (pch) {
- read_lock_bh(&pch->upl);
- if (pch->ppp)
+ int locked;
+
+ /* a trylock comment in ppp_input() */
+ local_bh_disable();
+ if ((locked = read_trylock(&pch->upl)) && pch->ppp)
unit = pch->ppp->file.index;
- read_unlock_bh(&pch->upl);
+
+ if (locked)
+ read_unlock(&pch->upl);
+ local_bh_enable();
}
return unit;
}