kernel panic in pppoe_release

kernel panic in pppoe_release

[Murat Sezgin]

Hi all,

We are using 3.4.103 kernel on our openwrt router platform. In the event of
interface down, we get kernel panic in the pppoe_release() function, because
the po->pppoe_dev comes to this function as NULL and dev_put() is called
without doing any NULL check.

I see that openwrt community has done an improvement for this issue with the
following commits in 3.18 and 4.0 kernels.

https://dev.openwrt.org/changeset/45653

I back ported these commits to our kernel and they increased the issue
occurrence frequency, but they didn't fix the issue completely. I ended up
that we need a NULL check in the pppoe_release() function as well. It seems
there is still a race between the pppoe_release() and the pppoe_flush_dev().
So, added the below change, but I would like to see whether this may have
side effects or not.

Can somebody please make comments on this?


@@ -589,7 +606,7 @@ static int pppoe_release(struct socket *sock)
 
        po = pppox_sk(sk);
 
-       if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND | PPPOX_ZOMBIE)) {
+       if (po->pppoe_dev && sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND |
PPPOX_ZOMBIE)) {
                dev_put(po->pppoe_dev);
                po->pppoe_dev = NULL;
        }

Regards,
Murat

kernel panic in pppoe_release

[Murat Sezgin]

Hi all,

I have already sent this email to linux-ppp group, but I have not received
any response yet. So, I want to send it to this group as well, because it is
most likely a netdev issue in pppoe kernel driver.

I see the bug in the below email discussion on the kernel that we are
currently using (3.4.103) with our openwrt distribution.

https://www.mail-archive.com/netdev@vger.kernel.org/msg70367.html

I did some debug on this and I see that the ref count of the po->pppoe_dev
doesn’t go to zero before releasing it with dev_put() and set its value to
NULL.

I also found the below patches from openwrt patch site for 3.18 and 4.0
kernels which can be applicable to our kernel. 

https://dev.openwrt.org/changeset/45653

But as described in the netdev mail-archive link above, it doesn’t solve
this issue completely and we still see the crash. I just wonder , if the
proposed patch by “Denys Fedoryshchenko”, which is below, fixes this issue
completely.

        pppox_unbind_sock(sk);
        +/* Signal the death of the socket. */
        +sk->sk_state = PPPOX_DEAD;


Do you have a conclusion on this bug? Is it safe to get this patch along
with the other workqueue patches?

Regards,
Murat

Re: kernel panic in pppoe_release

[Guillaume Nault]

On Thu, Sep 03, 2015 at 12:14:32PM -0700, Murat Sezgin wrote:
> I just wonder , if the
> proposed patch by Denys Fedoryshchenko, which is below, fixes this issue
> completely.
> 
>         pppox_unbind_sock(sk);
>         +/* Signal the death of the socket. */
>         +sk->sk_state = PPPOX_DEAD;
> 
> Do you have a conclusion on this bug? Is it safe to get this patch along
> with the other workqueue patches?
>
You might want to check my recent reply in the original thread:
https://marc.info/?l=linux-netdev&m=144190062507458&w=2