Re: Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself

[Guillaume Nault]

On Fri, Mar 16, 2018 at 02:49:40PM +0800, xu heng wrote:
> 
>         For testing, in __ppp_channel_push(), disable sending anything from 
> the attached unit, just disable __ppp_xmit_process(ppp) in 
> __ppp_channel_push(). In my opinion, __ppp_xmit_process() should only called 
> by ppp_xmit_process(), because of ppp->xmit_recursion __percpu.
> 
ppp_channel_push() needs to call __ppp_xmit_process() because some
drivers (like ppp_async) need to notify ppp_generic when they can
accept new packets. This is done by ppp_output_wakeup() which then
calls ppp_channel_push(). So we have to handle the unit backlog there.


Please try the following patch (untested).

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index b883af93929c..af22eb11bbaa 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -255,7 +255,7 @@ struct ppp_net {
 /* Prototypes. */
 static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf,
                        struct file *file, unsigned int cmd, unsigned long arg);
-static void ppp_xmit_process(struct ppp *ppp);
+static void ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb);
 static void ppp_send_frame(struct ppp *ppp, struct sk_buff *skb);
 static void ppp_push(struct ppp *ppp);
 static void ppp_channel_push(struct channel *pch);
@@ -511,13 +511,12 @@ static ssize_t ppp_write(struct file *file, const char 
__user *buf,
                goto out;
        }
 
-       skb_queue_tail(&pf->xq, skb);
-
        switch (pf->kind) {
        case INTERFACE:
-               ppp_xmit_process(PF_TO_PPP(pf));
+               ppp_xmit_process(PF_TO_PPP(pf), skb);
                break;
        case CHANNEL:
+               skb_queue_tail(&pf->xq, skb);
                ppp_channel_push(PF_TO_CHANNEL(pf));
                break;
        }
@@ -1260,8 +1259,8 @@ ppp_start_xmit(struct sk_buff *skb, struct net_device 
*dev)
        put_unaligned_be16(proto, pp);
 
        skb_scrub_packet(skb, !net_eq(ppp->ppp_net, dev_net(dev)));
-       skb_queue_tail(&ppp->file.xq, skb);
-       ppp_xmit_process(ppp);
+       ppp_xmit_process(ppp, skb);
+
        return NETDEV_TX_OK;
 
  outf:
@@ -1415,13 +1414,14 @@ static void ppp_setup(struct net_device *dev)
  */
 
 /* Called to do any work queued up on the transmit side that can now be done */
-static void __ppp_xmit_process(struct ppp *ppp)
+static void __ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
 {
-       struct sk_buff *skb;
-
        ppp_xmit_lock(ppp);
        if (!ppp->closing) {
                ppp_push(ppp);
+
+               if (skb)
+                       skb_queue_tail(&ppp->file.xq, skb);
                while (!ppp->xmit_pending &&
                       (skb = skb_dequeue(&ppp->file.xq)))
                        ppp_send_frame(ppp, skb);
@@ -1435,7 +1435,7 @@ static void __ppp_xmit_process(struct ppp *ppp)
        ppp_xmit_unlock(ppp);
 }
 
-static void ppp_xmit_process(struct ppp *ppp)
+static void ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
 {
        local_bh_disable();
 
@@ -1443,7 +1443,7 @@ static void ppp_xmit_process(struct ppp *ppp)
                goto err;
 
        (*this_cpu_ptr(ppp->xmit_recursion))++;
-       __ppp_xmit_process(ppp);
+       __ppp_xmit_process(ppp, skb);
        (*this_cpu_ptr(ppp->xmit_recursion))--;
 
        local_bh_enable();
@@ -1452,6 +1452,7 @@ static void ppp_xmit_process(struct ppp *ppp)
 
 err:
        local_bh_enable();
+       kfree_skb(skb);
 
        if (net_ratelimit())
                netdev_err(ppp->dev, "recursion detected\n");
@@ -1937,7 +1938,7 @@ static void __ppp_channel_push(struct channel *pch)
        if (skb_queue_empty(&pch->file.xq)) {
                ppp = pch->ppp;
                if (ppp)
-                       __ppp_xmit_process(ppp);
+                       __ppp_xmit_process(ppp, NULL);
        }
 }