netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself
@ 2018-03-14 15:02 Stephen Hemminger
  2018-03-15 17:51 ` Guillaume Nault
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2018-03-14 15:02 UTC (permalink / raw)
  To: xeb; +Cc: netdev



Begin forwarded message:

Date: Wed, 14 Mar 2018 06:56:09 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself


https://bugzilla.kernel.org/show_bug.cgi?id=199109

            Bug ID: 199109
           Summary: pptp: kernel printk "recursion detected", and then
                    reboot itself
           Product: Networking
           Version: 2.5
    Kernel Version: 4.9.77
          Hardware: Mips32
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: high
          Priority: P1
         Component: IPV4
          Assignee: stephen@networkplumber.org
          Reporter: xuheng333@zoho.com
        Regression: No

Created attachment 274715
  --> https://bugzilla.kernel.org/attachment.cgi?id=274715&action=edit  
system log

Use openwrt LEDE, (gcc version 5.5.0 (OpenWrt GCC 5.5.0 r5932-9c4fe10) ).
When use pptp, make WAN(eth1) down and then make WAN(eth1) up, pptp will redial
itself. Do this in a loop.

After a while, kernel print "recursion detected" and "net_ratelimit: 49422
callbacks suppressed", print this many times, then system reboot.

When add more printk in driver "driver/net/ppp/ppp_generic.c", fond it fall in
loop in "static void __ppp_xmit_process(struct ppp *ppp)", which called by
"static void __ppp_channel_push(struct channel *pch)". 
__ppp_channel_push() locked ppp->xmit_recursion, and loop for long time( while
(!ppp->xmit_pending && (skb = skb_dequeue(&ppp->file.xq))) ppp_send_frame(ppp,
skb); ). Some other thread want to lock ppp->xmit_recursion, but failed.


static void __ppp_channel_push(struct channel *pch)
{
        struct sk_buff *skb;
        struct ppp *ppp;

        spin_lock_bh(&pch->downl);
        if (pch->chan) {
                while (!skb_queue_empty(&pch->file.xq)) {
                        skb = skb_dequeue(&pch->file.xq);
                        if (!pch->chan->ops->start_xmit(pch->chan, skb)) {
                                /* put the packet back and try again later */
                                skb_queue_head(&pch->file.xq, skb);
                                break;
                        }
                }
        } else {
                /* channel got deregistered */
                skb_queue_purge(&pch->file.xq);
        }
        spin_unlock_bh(&pch->downl);
        /* see if there is anything from the attached unit to be sent */
        if (skb_queue_empty(&pch->file.xq)) {
                ppp = pch->ppp;
                if (ppp)
                        __ppp_xmit_process(ppp);
        }
}

-- 
You are receiving this mail because:
You are the assignee for the bug.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself
  2018-03-14 15:02 Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself Stephen Hemminger
@ 2018-03-15 17:51 ` Guillaume Nault
       [not found]   ` <1622d9239ec.fb8c9df225155.9220077804249612652@zoho.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Nault @ 2018-03-15 17:51 UTC (permalink / raw)
  To: xuheng333, Stephen Hemminger; +Cc: xeb, netdev

On Wed, Mar 14, 2018 at 08:02:17AM -0700, Stephen Hemminger wrote:
> 
> 
> Begin forwarded message:
> 
> Date: Wed, 14 Mar 2018 06:56:09 +0000
> From: bugzilla-daemon@bugzilla.kernel.org
> To: stephen@networkplumber.org
> Subject: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself
> 
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=199109
> 
>             Bug ID: 199109
>            Summary: pptp: kernel printk "recursion detected", and then
>                     reboot itself
>            Product: Networking
>            Version: 2.5
>     Kernel Version: 4.9.77
>           Hardware: Mips32
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: high
>           Priority: P1
>          Component: IPV4
>           Assignee: stephen@networkplumber.org
>           Reporter: xuheng333@zoho.com
>         Regression: No
> 
> Created attachment 274715
>   --> https://bugzilla.kernel.org/attachment.cgi?id=274715&action=edit  
> system log
> 
> Use openwrt LEDE, (gcc version 5.5.0 (OpenWrt GCC 5.5.0 r5932-9c4fe10) ).
> When use pptp, make WAN(eth1) down and then make WAN(eth1) up, pptp will redial
> itself. Do this in a loop.
> 
> After a while, kernel print "recursion detected" and "net_ratelimit: 49422
> callbacks suppressed", print this many times, then system reboot.
> 
> When add more printk in driver "driver/net/ppp/ppp_generic.c", fond it fall in
> loop in "static void __ppp_xmit_process(struct ppp *ppp)", which called by
> "static void __ppp_channel_push(struct channel *pch)". 
> __ppp_channel_push() locked ppp->xmit_recursion, and loop for long time( while
> (!ppp->xmit_pending && (skb = skb_dequeue(&ppp->file.xq))) ppp_send_frame(ppp,
> skb); ). Some other thread want to lock ppp->xmit_recursion, but failed.
> 
> 
Hi Xuheng,

Are you sure that your PPTP packets aren't routed back to your PPP
interface? What's the IP address of the PPTP server?

Regards,

Guillaume

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself
       [not found]   ` <1622d9239ec.fb8c9df225155.9220077804249612652@zoho.com>
@ 2018-03-16 20:02     ` Guillaume Nault
  2018-03-20 15:38       ` Guillaume Nault
  0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Nault @ 2018-03-16 20:02 UTC (permalink / raw)
  To: xu heng; +Cc: Stephen Hemminger, xeb, netdev

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-&gt;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);
 	}
 }
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself
  2018-03-16 20:02     ` Guillaume Nault
@ 2018-03-20 15:38       ` Guillaume Nault
       [not found]         ` <1624615812e.11e307c748584.6445977831797927207@zoho.com>
  0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Nault @ 2018-03-20 15:38 UTC (permalink / raw)
  To: xu heng; +Cc: Stephen Hemminger, xeb, netdev

On Fri, Mar 16, 2018 at 09:02:40PM +0100, Guillaume Nault wrote:
> 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-&gt;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).
> 
FYI, I've now fully tested the patch. I'm going to send it formally.

> 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);
>  	}
>  }
>  
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself
       [not found]         ` <1624615812e.11e307c748584.6445977831797927207@zoho.com>
@ 2018-03-21  8:35           ` Guillaume Nault
  2018-03-22  2:41             ` xu heng
  0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Nault @ 2018-03-21  8:35 UTC (permalink / raw)
  To: xu heng; +Cc: Stephen Hemminger, xeb, netdev

On Wed, Mar 21, 2018 at 09:03:57AM +0800, xu heng wrote:
> Yes, i have tested it for 146390 seconds in my board, it's ok now. Thanks!
> 
Feel free to add your Tested-by tag to the patch if you want to.
Thanks for your report.

Guillaume

BTW, for your future exchanges on the list, please avoid top-posting.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself
  2018-03-21  8:35           ` Guillaume Nault
@ 2018-03-22  2:41             ` xu heng
  0 siblings, 0 replies; 6+ messages in thread
From: xu heng @ 2018-03-22  2:41 UTC (permalink / raw)
  To: Guillaume Nault; +Cc: Stephen Hemminger, xeb, netdev


 ---- On Wed, 21 Mar 2018 16:35:28 +0800 Guillaume Nault <g.nault@alphalink.fr> wrote ---- 
 > On Wed, Mar 21, 2018 at 09:03:57AM +0800, xu heng wrote: 
 > > Yes, i have tested it for 146390 seconds in my board, it's ok now. Thanks! 
 > >  
 > Feel free to add your Tested-by tag to the patch if you want to. 
 > Thanks for your report. 
 >  
 > Guillaume 
 >  
 > BTW, for your future exchanges on the list, please avoid top-posting. 
 > 

I'm sorry for that, will never do that again. Thanks.

xuheng

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-22  2:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-14 15:02 Fw: [Bug 199109] New: pptp: kernel printk "recursion detected", and then reboot itself Stephen Hemminger
2018-03-15 17:51 ` Guillaume Nault
     [not found]   ` <1622d9239ec.fb8c9df225155.9220077804249612652@zoho.com>
2018-03-16 20:02     ` Guillaume Nault
2018-03-20 15:38       ` Guillaume Nault
     [not found]         ` <1624615812e.11e307c748584.6445977831797927207@zoho.com>
2018-03-21  8:35           ` Guillaume Nault
2018-03-22  2:41             ` xu heng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).