Re: Urgent: BUG: PPP ioctl Transport endpoint is not connected

From: Martin Zaharinov
Date: Wed Dec 09 2020 - 12:30:46 EST

I make diff linux 4.14.211 and 4.15 kernel

And changes is:

atomic_inc to refcount_inc

And on other part of code in ppp_generic.c remove skb_free ….



You see diff down :


--- linux-4.14.211/drivers/net/ppp/ppp_generic.c 2020-12-08 09:17:35.000000000 +0000
+++ linux-4.15/drivers/net/ppp/ppp_generic.c 2018-01-28 21:20:33.000000000 +0000
@@ -51,6 +51,7 @@
#include <asm/unaligned.h>
#include <net/slhc_vj.h>
#include <linux/atomic.h>
+#include <linux/refcount.h>

#include <linux/nsproxy.h>
#include <net/net_namespace.h>
@@ -84,7 +85,7 @@ struct ppp_file {
struct sk_buff_head xq; /* pppd transmit queue */
struct sk_buff_head rq; /* receive queue for pppd */
wait_queue_head_t rwait; /* for poll on reading /dev/ppp */
- atomic_t refcnt; /* # refs (incl /dev/ppp attached) */
+ refcount_t refcnt; /* # refs (incl /dev/ppp attached) */
int hdrlen; /* space to leave for headers */
int index; /* interface unit / channel number */
int dead; /* unit/channel has been shut down */
@@ -256,7 +257,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, struct sk_buff *skb);
+static void ppp_xmit_process(struct ppp *ppp);
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);
@@ -389,7 +390,7 @@ static int ppp_open(struct inode *inode,
/*
* This could (should?) be enforced by the permissions on /dev/ppp.
*/
- if (!capable(CAP_NET_ADMIN))
+ if (!ns_capable(file->f_cred->user_ns, CAP_NET_ADMIN))
return -EPERM;
return 0;
}
@@ -408,7 +409,7 @@ static int ppp_release(struct inode *unu
unregister_netdevice(ppp->dev);
rtnl_unlock();
}
- if (atomic_dec_and_test(&pf->refcnt)) {
+ if (refcount_dec_and_test(&pf->refcnt)) {
switch (pf->kind) {
case INTERFACE:
ppp_destroy_interface(PF_TO_PPP(pf));
@@ -512,12 +513,13 @@ static ssize_t ppp_write(struct file *fi
goto out;
}

+ skb_queue_tail(&pf->xq, skb);
+
switch (pf->kind) {
case INTERFACE:
- ppp_xmit_process(PF_TO_PPP(pf), skb);
+ ppp_xmit_process(PF_TO_PPP(pf));
break;
case CHANNEL:
- skb_queue_tail(&pf->xq, skb);
ppp_channel_push(PF_TO_CHANNEL(pf));
break;
}
@@ -880,7 +882,7 @@ static int ppp_unattached_ioctl(struct n
mutex_lock(&pn->all_ppp_mutex);
ppp = ppp_find_unit(pn, unit);
if (ppp) {
- atomic_inc(&ppp->file.refcnt);
+ refcount_inc(&ppp->file.refcnt);
file->private_data = &ppp->file;
err = 0;
}
@@ -895,7 +897,7 @@ static int ppp_unattached_ioctl(struct n
spin_lock_bh(&pn->all_channels_lock);
chan = ppp_find_channel(pn, unit);
if (chan) {
- atomic_inc(&chan->file.refcnt);
+ refcount_inc(&chan->file.refcnt);
file->private_data = &chan->file;
err = 0;
}
@@ -960,6 +962,8 @@ static __net_exit void ppp_exit_net(stru

mutex_destroy(&pn->all_ppp_mutex);
idr_destroy(&pn->units_idr);
+ WARN_ON_ONCE(!list_empty(&pn->all_channels));
+ WARN_ON_ONCE(!list_empty(&pn->new_channels));
}

static struct pernet_operations ppp_net_ops = {
@@ -1263,8 +1267,8 @@ ppp_start_xmit(struct sk_buff *skb, stru
put_unaligned_be16(proto, pp);

skb_scrub_packet(skb, !net_eq(ppp->ppp_net, dev_net(dev)));
- ppp_xmit_process(ppp, skb);
-
+ skb_queue_tail(&ppp->file.xq, skb);
+ ppp_xmit_process(ppp);
return NETDEV_TX_OK;

outf:
@@ -1349,7 +1353,7 @@ static int ppp_dev_init(struct net_devic
* that ppp_destroy_interface() won't run before the device gets
* unregistered.
*/
- atomic_inc(&ppp->file.refcnt);
+ refcount_inc(&ppp->file.refcnt);

return 0;
}
@@ -1378,7 +1382,7 @@ static void ppp_dev_priv_destructor(stru
struct ppp *ppp;

ppp = netdev_priv(dev);
- if (atomic_dec_and_test(&ppp->file.refcnt))
+ if (refcount_dec_and_test(&ppp->file.refcnt))
ppp_destroy_interface(ppp);
}

@@ -1416,14 +1420,13 @@ static void ppp_setup(struct net_device
*/

/* Called to do any work queued up on the transmit side that can now be done */
-static void __ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
+static void __ppp_xmit_process(struct ppp *ppp)
{
+ 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);
@@ -1433,13 +1436,11 @@ static void __ppp_xmit_process(struct pp
netif_wake_queue(ppp->dev);
else
netif_stop_queue(ppp->dev);
- } else {
- kfree_skb(skb);
}
ppp_xmit_unlock(ppp);
}

-static void ppp_xmit_process(struct ppp *ppp, struct sk_buff *skb)
+static void ppp_xmit_process(struct ppp *ppp)
{
local_bh_disable();

@@ -1447,7 +1448,7 @@ static void ppp_xmit_process(struct ppp
goto err;

(*this_cpu_ptr(ppp->xmit_recursion))++;
- __ppp_xmit_process(ppp, skb);
+ __ppp_xmit_process(ppp);
(*this_cpu_ptr(ppp->xmit_recursion))--;

local_bh_enable();
@@ -1457,8 +1458,6 @@ static void ppp_xmit_process(struct ppp
err:
local_bh_enable();

- kfree_skb(skb);
-
if (net_ratelimit())
netdev_err(ppp->dev, "recursion detected\n");
}
@@ -1943,7 +1942,7 @@ static void __ppp_channel_push(struct ch
if (skb_queue_empty(&pch->file.xq)) {
ppp = pch->ppp;
if (ppp)
- __ppp_xmit_process(ppp, NULL);
+ __ppp_xmit_process(ppp);
}
}

@@ -2682,7 +2681,7 @@ ppp_unregister_channel(struct ppp_channe

pch->file.dead = 1;
wake_up_interruptible(&pch->file.rwait);
- if (atomic_dec_and_test(&pch->file.refcnt))
+ if (refcount_dec_and_test(&pch->file.refcnt))
ppp_destroy_channel(pch);
}

@@ -3052,7 +3051,7 @@ init_ppp_file(struct ppp_file *pf, int k
pf->kind = kind;
skb_queue_head_init(&pf->xq);
skb_queue_head_init(&pf->rq);
- atomic_set(&pf->refcnt, 1);
+ refcount_set(&pf->refcnt, 1);
init_waitqueue_head(&pf->rwait);
}

@@ -3162,15 +3161,6 @@ ppp_connect_channel(struct channel *pch,
goto outl;

ppp_lock(ppp);
- spin_lock_bh(&pch->downl);
- if (!pch->chan) {
- /* Don't connect unregistered channels */
- spin_unlock_bh(&pch->downl);
- ppp_unlock(ppp);
- ret = -ENOTCONN;
- goto outl;
- }
- spin_unlock_bh(&pch->downl);
if (pch->file.hdrlen > ppp->file.hdrlen)
ppp->file.hdrlen = pch->file.hdrlen;
hdrlen = pch->file.hdrlen + 2; /* for protocol bytes */
@@ -3179,7 +3169,7 @@ ppp_connect_channel(struct channel *pch,
list_add_tail(&pch->clist, &ppp->channels);
++ppp->n_channels;
pch->ppp = ppp;
- atomic_inc(&ppp->file.refcnt);
+ refcount_inc(&ppp->file.refcnt);
ppp_unlock(ppp);
ret = 0;

@@ -3210,7 +3200,7 @@ ppp_disconnect_channel(struct channel *p
if (--ppp->n_channels == 0)
wake_up_interruptible(&ppp->file.rwait);
ppp_unlock(ppp);
- if (atomic_dec_and_test(&ppp->file.refcnt))
+ if (refcount_dec_and_test(&ppp->file.refcnt))
ppp_destroy_interface(ppp);
err = 0;
}

> On 9 Dec 2020, at 18:57, Martin Zaharinov <micron10@xxxxxxxxx> wrote:
>
> Hi Nault
>
>
>
>> On 9 Dec 2020, at 18:40, Guillaume Nault <gnault@xxxxxxxxxx> wrote:
>>
>> On Wed, Dec 09, 2020 at 04:47:52PM +0200, Martin Zaharinov wrote:
>>> Hi All
>>>
>>> I have problem with latest kernel release
>>> And the problem is base on this late problem :
>>>
>>>
>>> https://www.mail-archive.com/search?l=netdev@xxxxxxxxxxxxxxx&q=subject:%22Re%5C%3A+ppp%5C%2Fpppoe%2C+still+panic+4.15.3+in+ppp_push%22&o=newest&f=1
>>>
>>> I have same problem in kernel 5.6 > now I use kernel 5.9.13 and have same problem.
>>>
>>>
>>> In kernel 5.9.13 now don’t have any crashes in dimes but in one moment accel service stop with defunct and in log have many of this line :
>>>
>>>
>>> error: vlan608: ioctl(PPPIOCCONNECT): Transport endpoint is not connected
>>> error: vlan617: ioctl(PPPIOCCONNECT): Transport endpoint is not connected
>>> error: vlan679: ioctl(PPPIOCCONNECT): Transport endpoint is not connected
>>>
>>> In one moment connected user bump double or triple and after that service defunct and need wait to drop all session to start .
>>>
>>> I talk with accel-ppp team and they said this is kernel related problem and to back to kernel 4.14 there is not this problem.
>>>
>>> Problem is come after kernel 4.15 > and not have solution to this moment.
>>
>> I'm sorry, I don't understand.
>> Do you mean that v4.14 worked fine (no crash, no ioctl() error)?
>> Did the problem start appearing in v4.15? Or did v4.15 work and the
>> problem appeared in v4.16?
>
> In Telegram group I talk with Sergey and Dimka and told my the problem is come after changes from 4.14 to 4.15
> Sergey write this : "as I know, there was a similar issue in kernel 4.15 so maybe it is still not fixed”
>
> I don’t have options to test with this old kernel 4.14.xxx i don’t have support for them.
>
>
>>
>>> Please help to find the problem.
>>>
>>> Last time in link I see is make changes in ppp_generic.c
>>>
>>> ppp_lock(ppp);
>>> spin_lock_bh(&pch->downl);
>>> if (!pch->chan) {
>>> /* Don't connect unregistered channels */
>>> spin_unlock_bh(&pch->downl);
>>> ppp_unlock(ppp);
>>> ret = -ENOTCONN;
>>> goto outl;
>>> }
>>> spin_unlock_bh(&pch->downl);
>>>
>>>
>>> But this fix only to don’t display error and freeze system
>>> The problem is stay and is to big.
>>
>> Do you use accel-ppp's unit-cache option? Does the problem go away if
>> you stop using it?
>>
>
> No I don’t use unit-cache , if I set unit-cache accel-ppp defunct same but user Is connect and disconnet more fast.
>
> The problem is same with unit and without .
> Only after this patch I don’t see error in dimes but this is not solution.
> In network have customer what have power cut problem, when drop 600 user and back Is normal but in this moment kernel is locking and start to make this :
> sessions:
> starting: 4235
> active: 3882
> finishing: 378
>
> The problem is starting session is not real user normal user in this server is ~4k customers .
>
> I use pppd_compat .
>
> Any idea ?
>
>>>
>>> Please help to fix.
> Martin