linux-stable.git/drivers/net/ppp, branch v4.9.91

ppp: prevent unregistered channels from connecting to PPP units

2018-03-11T15:21:32+00:00

[ Upstream commit 77f840e3e5f09c6d7d727e85e6e08276dd813d11 ]

PPP units don't hold any reference on the channels connected to it.
It is the channel's responsibility to ensure that it disconnects from
its unit before being destroyed.
In practice, this is ensured by ppp_unregister_channel() disconnecting
the channel from the unit before dropping a reference on the channel.

However, it is possible for an unregistered channel to connect to a PPP
unit: register a channel with ppp_register_net_channel(), attach a
/dev/ppp file to it with ioctl(PPPIOCATTCHAN), unregister the channel
with ppp_unregister_channel() and finally connect the /dev/ppp file to
a PPP unit with ioctl(PPPIOCCONNECT).

Once in this situation, the channel is only held by the /dev/ppp file,
which can be released at anytime and free the channel without letting
the parent PPP unit know. Then the ppp structure ends up with dangling
pointers in its ->channels list.

Prevent this scenario by forbidding unregistered channels from
connecting to PPP units. This maintains the code logic by keeping
ppp_unregister_channel() responsible from disconnecting the channel if
necessary and avoids modification on the reference counting mechanism.

This issue seems to predate git history (successfully reproduced on
Linux 2.6.26 and earlier PPP commits are unrelated).

Signed-off-by: Guillaume Nault 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

ppp: unlock all_ppp_mutex before registering device

2018-01-31T11:55:55+00:00

[ Upstream commit 0171c41835591e9aa2e384b703ef9a6ae367c610 ]

ppp_dev_uninit(), which is the .ndo_uninit() handler of PPP devices,
needs to lock pn->all_ppp_mutex. Therefore we mustn't call
register_netdevice() with pn->all_ppp_mutex already locked, or we'd
deadlock in case register_netdevice() fails and calls .ndo_uninit().

Fortunately, we can unlock pn->all_ppp_mutex before calling
register_netdevice(). This lock protects pn->units_idr, which isn't
used in the device registration process.

However, keeping pn->all_ppp_mutex locked during device registration
did ensure that no device in transient state would be published in
pn->units_idr. In practice, unlocking it before calling
register_netdevice() doesn't change this property: ppp_unit_register()
is called with 'ppp_mutex' locked and all searches done in
pn->units_idr hold this lock too.

Fixes: 8cb775bc0a34 ("ppp: fix device unregistration upon netns deletion")
Reported-and-tested-by: syzbot+367889b9c9e279219175@syzkaller.appspotmail.com
Signed-off-by: Guillaume Nault 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

pppoe: take ->needed_headroom of lower device into account on xmit

2018-01-31T11:55:54+00:00

[ Upstream commit 02612bb05e51df8489db5e94d0cf8d1c81f87b0c ]

In pppoe_sendmsg(), reserving dev->hard_header_len bytes of headroom
was probably fine before the introduction of ->needed_headroom in
commit f5184d267c1a ("net: Allow netdevices to specify needed head/tailroom").

But now, virtual devices typically advertise the size of their overhead
in dev->needed_headroom, so we must also take it into account in
skb_reserve().
Allocation size of skb is also updated to take dev->needed_tailroom
into account and replace the arbitrary 32 bytes with the real size of
a PPPoE header.

This issue was discovered by syzbot, who connected a pppoe socket to a
gre device which had dev->header_ops->create == ipgre_header and
dev->hard_header_len == 0. Therefore, PPPoE didn't reserve any
headroom, and dev_hard_header() crashed when ipgre_header() tried to
prepend its header to skb->data.

skbuff: skb_under_panic: text:000000001d390b3a len:31 put:24
head:00000000d8ed776f data:000000008150e823 tail:0x7 end:0xc0 dev:gre0
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:104!
invalid opcode: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
    (ftrace buffer empty)
Modules linked in:
CPU: 1 PID: 3670 Comm: syzkaller801466 Not tainted
4.15.0-rc7-next-20180115+ #97
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:skb_panic+0x162/0x1f0 net/core/skbuff.c:100
RSP: 0018:ffff8801d9bd7840 EFLAGS: 00010282
RAX: 0000000000000083 RBX: ffff8801d4f083c0 RCX: 0000000000000000
RDX: 0000000000000083 RSI: 1ffff1003b37ae92 RDI: ffffed003b37aefc
RBP: ffff8801d9bd78a8 R08: 1ffff1003b37ae8a R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffffffff86200de0
R13: ffffffff84a981ad R14: 0000000000000018 R15: ffff8801d2d34180
FS:  00000000019c4880(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000208bc000 CR3: 00000001d9111001 CR4: 00000000001606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  skb_under_panic net/core/skbuff.c:114 [inline]
  skb_push+0xce/0xf0 net/core/skbuff.c:1714
  ipgre_header+0x6d/0x4e0 net/ipv4/ip_gre.c:879
  dev_hard_header include/linux/netdevice.h:2723 [inline]
  pppoe_sendmsg+0x58e/0x8b0 drivers/net/ppp/pppoe.c:890
  sock_sendmsg_nosec net/socket.c:630 [inline]
  sock_sendmsg+0xca/0x110 net/socket.c:640
  sock_write_iter+0x31a/0x5d0 net/socket.c:909
  call_write_iter include/linux/fs.h:1775 [inline]
  do_iter_readv_writev+0x525/0x7f0 fs/read_write.c:653
  do_iter_write+0x154/0x540 fs/read_write.c:932
  vfs_writev+0x18a/0x340 fs/read_write.c:977
  do_writev+0xfc/0x2a0 fs/read_write.c:1012
  SYSC_writev fs/read_write.c:1085 [inline]
  SyS_writev+0x27/0x30 fs/read_write.c:1082
  entry_SYSCALL_64_fastpath+0x29/0xa0

Admittedly PPPoE shouldn't be allowed to run on non Ethernet-like
interfaces, but reserving space for ->needed_headroom is a more
fundamental issue that needs to be addressed first.

Same problem exists for __pppoe_xmit(), which also needs to take
dev->needed_headroom into account in skb_cow_head().

Fixes: f5184d267c1a ("net: Allow netdevices to specify needed head/tailroom")
Reported-by: syzbot+ed0838d0fa4c4f2b528e20286e6dc63effc7c14d@syzkaller.appspotmail.com
Signed-off-by: Guillaume Nault 
Reviewed-by: Xin Long 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

ppp: Destroy the mutex when cleanup

2017-12-20T09:07:30+00:00

[ Upstream commit f02b2320b27c16b644691267ee3b5c110846f49e ]

The mutex_destroy only makes sense when enable DEBUG_MUTEX. For the
good readbility, it's better to invoke it in exit func when the init
func invokes mutex_init.

Signed-off-by: Gao Feng 
Acked-by: Guillaume Nault 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
Signed-off-by: Greg Kroah-Hartman

ppp: fix race in ppp device destruction

2017-11-18T10:22:23+00:00

[ Upstream commit 6151b8b37b119e8e3a8401b080d532520c95faf4 ]

ppp_release() tries to ensure that netdevices are unregistered before
decrementing the unit refcount and running ppp_destroy_interface().

This is all fine as long as the the device is unregistered by
ppp_release(): the unregister_netdevice() call, followed by
rtnl_unlock(), guarantee that the unregistration process completes
before rtnl_unlock() returns.

However, the device may be unregistered by other means (like
ppp_nl_dellink()). If this happens right before ppp_release() calling
rtnl_lock(), then ppp_release() has to wait for the concurrent
unregistration code to release the lock.
But rtnl_unlock() releases the lock before completing the device
unregistration process. This allows ppp_release() to proceed and
eventually call ppp_destroy_interface() before the unregistration
process completes. Calling free_netdev() on this partially unregistered
device will BUG():

 ------------[ cut here ]------------
 kernel BUG at net/core/dev.c:8141!
 invalid opcode: 0000 [#1] SMP

 CPU: 1 PID: 1557 Comm: pppd Not tainted 4.14.0-rc2+ #4
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1.fc26 04/01/2014

 Call Trace:
  ppp_destroy_interface+0xd8/0xe0 [ppp_generic]
  ppp_disconnect_channel+0xda/0x110 [ppp_generic]
  ppp_unregister_channel+0x5e/0x110 [ppp_generic]
  pppox_unbind_sock+0x23/0x30 [pppox]
  pppoe_connect+0x130/0x440 [pppoe]
  SYSC_connect+0x98/0x110
  ? do_fcntl+0x2c0/0x5d0
  SyS_connect+0xe/0x10
  entry_SYSCALL_64_fastpath+0x1a/0xa5

 RIP: free_netdev+0x107/0x110 RSP: ffffc28a40573d88
 ---[ end trace ed294ff0cc40eeff ]---

We could set the ->needs_free_netdev flag on PPP devices and move the
ppp_destroy_interface() logic in the ->priv_destructor() callback. But
that'd be quite intrusive as we'd first need to unlink from the other
channels and units that depend on the device (the ones that used the
PPPIOCCONNECT and PPPIOCATTACH ioctls).

Instead, we can just let the netdevice hold a reference on its
ppp_file. This reference is dropped in ->priv_destructor(), at the very
end of the unregistration process, so that neither ppp_release() nor
ppp_disconnect_channel() can call ppp_destroy_interface() in the interim.

Reported-by: Beniamino Galvani 
Fixes: 8cb775bc0a34 ("ppp: fix device unregistration upon netns deletion")
Signed-off-by: Guillaume Nault 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

ppp: fix xmit recursion detection on ppp channels

2017-08-13T02:31:21+00:00

[ Upstream commit 0a0e1a85c83775a648041be2b15de6d0a2f2b8eb ]

Commit e5dadc65f9e0 ("ppp: Fix false xmit recursion detect with two ppp
devices") dropped the xmit_recursion counter incrementation in
ppp_channel_push() and relied on ppp_xmit_process() for this task.
But __ppp_channel_push() can also send packets directly (using the
.start_xmit() channel callback), in which case the xmit_recursion
counter isn't incremented anymore. If such packets get routed back to
the parent ppp unit, ppp_xmit_process() won't notice the recursion and
will call ppp_channel_push() on the same channel, effectively creating
the deadlock situation that the xmit_recursion mechanism was supposed
to prevent.

This patch re-introduces the xmit_recursion counter incrementation in
ppp_channel_push(). Since the xmit_recursion variable is now part of
the parent ppp unit, incrementation is skipped if the channel doesn't
have any. This is fine because only packets routed through the parent
unit may enter the channel recursively.

Finally, we have to ensure that pch->ppp is not going to be modified
while executing ppp_channel_push(). Instead of taking this lock only
while calling ppp_xmit_process(), we now have to hold it for the full
ppp_channel_push() execution. This respects the ppp locks ordering
which requires locking ->upl before ->downl.

Fixes: e5dadc65f9e0 ("ppp: Fix false xmit recursion detect with two ppp devices")
Signed-off-by: Guillaume Nault 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

ppp: Fix false xmit recursion detect with two ppp devices

2017-08-13T02:31:21+00:00

[ Upstream commit e5dadc65f9e0177eb649bcd9d333f1ebf871223e ]

The global percpu variable ppp_xmit_recursion is used to detect the ppp
xmit recursion to avoid the deadlock, which is caused by one CPU tries to
lock the xmit lock twice. But it would report false recursion when one CPU
wants to send the skb from two different PPP devices, like one L2TP on the
PPPoE. It is a normal case actually.

Now use one percpu member of struct ppp instead of the gloable variable to
detect the xmit recursion of one ppp device.

Fixes: 55454a565836 ("ppp: avoid dealock on recursive xmit")
Signed-off-by: Gao Feng 
Signed-off-by: Liu Jianying 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman

ppp: declare PPP devices as LLTX

2016-08-31T21:33:09+00:00

ppp_xmit_process() already locks the xmit path. If HARD_TX_LOCK() tries
to hold the _xmit_lock we can get lock inversion.

[  973.726130] ======================================================
[  973.727311] [ INFO: possible circular locking dependency detected ]
[  973.728546] 4.8.0-rc2 #1 Tainted: G           O
[  973.728986] -------------------------------------------------------
[  973.728986] accel-pppd/1806 is trying to acquire lock:
[  973.728986]  (&qdisc_xmit_lock_key){+.-...}, at: [] sch_direct_xmit+0x8d/0x221
[  973.728986]
[  973.728986] but task is already holding lock:
[  973.728986]  (l2tp_sock){+.-...}, at: [] l2tp_xmit_skb+0x1e8/0x5d7 [l2tp_core]
[  973.728986]
[  973.728986] which lock already depends on the new lock.
[  973.728986]
[  973.728986]
[  973.728986] the existing dependency chain (in reverse order) is:
[  973.728986]
-> #3 (l2tp_sock){+.-...}:
[  973.728986]        [] lock_acquire+0x150/0x217
[  973.728986]        [] _raw_spin_lock+0x2d/0x3c
[  973.728986]        [] l2tp_xmit_skb+0x1e8/0x5d7 [l2tp_core]
[  973.728986]        [] pppol2tp_xmit+0x1f2/0x25e [l2tp_ppp]
[  973.728986]        [] ppp_channel_push+0xb5/0x14a [ppp_generic]
[  973.728986]        [] ppp_write+0x104/0x11c [ppp_generic]
[  973.728986]        [] __vfs_write+0x56/0x120
[  973.728986]        [] vfs_write+0xbd/0x11b
[  973.728986]        [] SyS_write+0x5e/0x96
[  973.728986]        [] entry_SYSCALL_64_fastpath+0x18/0xa8
[  973.728986]
-> #2 (&(&pch->downl)->rlock){+.-...}:
[  973.728986]        [] lock_acquire+0x150/0x217
[  973.728986]        [] _raw_spin_lock_bh+0x31/0x40
[  973.728986]        [] ppp_push+0xa7/0x82d [ppp_generic]
[  973.728986]        [] __ppp_xmit_process+0x48/0x877 [ppp_generic]
[  973.728986]        [] ppp_xmit_process+0x4b/0xaf [ppp_generic]
[  973.728986]        [] ppp_write+0x10e/0x11c [ppp_generic]
[  973.728986]        [] __vfs_write+0x56/0x120
[  973.728986]        [] vfs_write+0xbd/0x11b
[  973.728986]        [] SyS_write+0x5e/0x96
[  973.728986]        [] entry_SYSCALL_64_fastpath+0x18/0xa8
[  973.728986]
-> #1 (&(&ppp->wlock)->rlock){+.-...}:
[  973.728986]        [] lock_acquire+0x150/0x217
[  973.728986]        [] _raw_spin_lock_bh+0x31/0x40
[  973.728986]        [] __ppp_xmit_process+0x27/0x877 [ppp_generic]
[  973.728986]        [] ppp_xmit_process+0x4b/0xaf [ppp_generic]
[  973.728986]        [] ppp_start_xmit+0x21b/0x22a [ppp_generic]
[  973.728986]        [] dev_hard_start_xmit+0x1a9/0x43d
[  973.728986]        [] sch_direct_xmit+0xd6/0x221
[  973.728986]        [] __dev_queue_xmit+0x62a/0x912
[  973.728986]        [] dev_queue_xmit+0xb/0xd
[  973.728986]        [] neigh_direct_output+0xc/0xe
[  973.728986]        [] ip6_finish_output2+0x5a9/0x623
[  973.728986]        [] ip6_output+0x15e/0x16a
[  973.728986]        [] dst_output+0x76/0x7f
[  973.728986]        [] mld_sendpack+0x335/0x404
[  973.728986]        [] mld_send_initial_cr.part.21+0x99/0xa2
[  973.728986]        [] ipv6_mc_dad_complete+0x42/0x71
[  973.728986]        [] addrconf_dad_completed+0x1cf/0x2ea
[  973.728986]        [] addrconf_dad_work+0x453/0x520
[  973.728986]        [] process_one_work+0x365/0x6f0
[  973.728986]        [] worker_thread+0x2de/0x421
[  973.728986]        [] kthread+0x121/0x130
[  973.728986]        [] ret_from_fork+0x1f/0x40
[  973.728986]
-> #0 (&qdisc_xmit_lock_key){+.-...}:
[  973.728986]        [] __lock_acquire+0x1118/0x1483
[  973.728986]        [] lock_acquire+0x150/0x217
[  973.728986]        [] _raw_spin_lock+0x2d/0x3c
[  973.728986]        [] sch_direct_xmit+0x8d/0x221
[  973.728986]        [] __dev_queue_xmit+0x62a/0x912
[  973.728986]        [] dev_queue_xmit+0xb/0xd
[  973.728986]        [] neigh_direct_output+0xc/0xe
[  973.728986]        [] ip_finish_output2+0x5db/0x609
[  973.728986]        [] ip_finish_output+0x152/0x15e
[  973.728986]        [] ip_output+0x8c/0x96
[  973.728986]        [] ip_local_out+0x41/0x4a
[  973.728986]        [] ip_queue_xmit+0x5a5/0x609
[  973.728986]        [] l2tp_xmit_skb+0x582/0x5d7 [l2tp_core]
[  973.728986]        [] pppol2tp_xmit+0x1f2/0x25e [l2tp_ppp]
[  973.728986]        [] ppp_channel_push+0xb5/0x14a [ppp_generic]
[  973.728986]        [] ppp_write+0x104/0x11c [ppp_generic]
[  973.728986]        [] __vfs_write+0x56/0x120
[  973.728986]        [] vfs_write+0xbd/0x11b
[  973.728986]        [] SyS_write+0x5e/0x96
[  973.728986]        [] entry_SYSCALL_64_fastpath+0x18/0xa8
[  973.728986]
[  973.728986] other info that might help us debug this:
[  973.728986]
[  973.728986] Chain exists of:
  &qdisc_xmit_lock_key --> &(&pch->downl)->rlock --> l2tp_sock

[  973.728986]  Possible unsafe locking scenario:
[  973.728986]
[  973.728986]        CPU0                    CPU1
[  973.728986]        ----                    ----
[  973.728986]   lock(l2tp_sock);
[  973.728986]                                lock(&(&pch->downl)->rlock);
[  973.728986]                                lock(l2tp_sock);
[  973.728986]   lock(&qdisc_xmit_lock_key);
[  973.728986]
[  973.728986]  *** DEADLOCK ***
[  973.728986]
[  973.728986] 6 locks held by accel-pppd/1806:
[  973.728986]  #0:  (&(&pch->downl)->rlock){+.-...}, at: [] ppp_channel_push+0x56/0x14a [ppp_generic]
[  973.728986]  #1:  (l2tp_sock){+.-...}, at: [] l2tp_xmit_skb+0x1e8/0x5d7 [l2tp_core]
[  973.728986]  #2:  (rcu_read_lock){......}, at: [] rcu_lock_acquire+0x0/0x20
[  973.728986]  #3:  (rcu_read_lock_bh){......}, at: [] rcu_lock_acquire+0x0/0x20
[  973.728986]  #4:  (rcu_read_lock_bh){......}, at: [] rcu_lock_acquire+0x0/0x20
[  973.728986]  #5:  (dev->qdisc_running_key ?: &qdisc_running_key#2){+.....}, at: [] __dev_queue_xmit+0x564/0x912
[  973.728986]
[  973.728986] stack backtrace:
[  973.728986] CPU: 2 PID: 1806 Comm: accel-pppd Tainted: G           O    4.8.0-rc2 #1
[  973.728986] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
[  973.728986]  ffff7fffffffffff ffff88003436f850 ffffffff812a20f4 ffffffff82156e30
[  973.728986]  ffffffff82156920 ffff88003436f890 ffffffff8115c759 ffff88003344ae00
[  973.728986]  ffff88003344b5c0 0000000000000002 0000000000000006 ffff88003344b5e8
[  973.728986] Call Trace:
[  973.728986]  [] dump_stack+0x67/0x90
[  973.728986]  [] print_circular_bug+0x22e/0x23c
[  973.728986]  [] __lock_acquire+0x1118/0x1483
[  973.728986]  [] lock_acquire+0x150/0x217
[  973.728986]  [] ? lock_acquire+0x150/0x217
[  973.728986]  [] ? sch_direct_xmit+0x8d/0x221
[  973.728986]  [] _raw_spin_lock+0x2d/0x3c
[  973.728986]  [] ? sch_direct_xmit+0x8d/0x221
[  973.728986]  [] sch_direct_xmit+0x8d/0x221
[  973.728986]  [] __dev_queue_xmit+0x62a/0x912
[  973.728986]  [] dev_queue_xmit+0xb/0xd
[  973.728986]  [] neigh_direct_output+0xc/0xe
[  973.728986]  [] ip_finish_output2+0x5db/0x609
[  973.728986]  [] ? dst_mtu+0x29/0x2e
[  973.728986]  [] ip_finish_output+0x152/0x15e
[  973.728986]  [] ? ip_output+0x74/0x96
[  973.728986]  [] ip_output+0x8c/0x96
[  973.728986]  [] ip_local_out+0x41/0x4a
[  973.728986]  [] ip_queue_xmit+0x5a5/0x609
[  973.728986]  [] ? udp_set_csum+0x207/0x21e
[  973.728986]  [] l2tp_xmit_skb+0x582/0x5d7 [l2tp_core]
[  973.728986]  [] pppol2tp_xmit+0x1f2/0x25e [l2tp_ppp]
[  973.728986]  [] ppp_channel_push+0xb5/0x14a [ppp_generic]
[  973.728986]  [] ppp_write+0x104/0x11c [ppp_generic]
[  973.728986]  [] __vfs_write+0x56/0x120
[  973.728986]  [] ? fsnotify_perm+0x27/0x95
[  973.728986]  [] ? security_file_permission+0x4d/0x54
[  973.728986]  [] vfs_write+0xbd/0x11b
[  973.728986]  [] SyS_write+0x5e/0x96
[  973.728986]  [] entry_SYSCALL_64_fastpath+0x18/0xa8
[  973.728986]  [] ? trace_hardirqs_off_caller+0x121/0x12f

Signed-off-by: Guillaume Nault 
Signed-off-by: David S. Miller

ppp: avoid dealock on recursive xmit

2016-08-31T21:33:08+00:00

In case of misconfiguration, a virtual PPP channel might send packets
back to their parent PPP interface. This typically happens in
misconfigured L2TP setups, where PPP's peer IP address is set with the
IP of the L2TP peer.
When that happens the system hangs due to PPP trying to recursively
lock its xmit path.

[  243.332155] BUG: spinlock recursion on CPU#1, accel-pppd/926
[  243.333272]  lock: 0xffff880033d90f18, .magic: dead4ead, .owner: accel-pppd/926, .owner_cpu: 1
[  243.334859] CPU: 1 PID: 926 Comm: accel-pppd Not tainted 4.8.0-rc2 #1
[  243.336010] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
[  243.336018]  ffff7fffffffffff ffff8800319a77a0 ffffffff8128de85 ffff880033d90f18
[  243.336018]  ffff880033ad8000 ffff8800319a77d8 ffffffff810ad7c0 ffffffff0000039e
[  243.336018]  ffff880033d90f18 ffff880033d90f60 ffff880033d90f18 ffff880033d90f28
[  243.336018] Call Trace:
[  243.336018]  [] dump_stack+0x4f/0x65
[  243.336018]  [] spin_dump+0xe1/0xeb
[  243.336018]  [] spin_bug+0x26/0x28
[  243.336018]  [] do_raw_spin_lock+0x5c/0x160
[  243.336018]  [] _raw_spin_lock_bh+0x35/0x3c
[  243.336018]  [] ? ppp_push+0xa7/0x82d [ppp_generic]
[  243.336018]  [] ppp_push+0xa7/0x82d [ppp_generic]
[  243.336018]  [] ? do_raw_spin_unlock+0xc2/0xcc
[  243.336018]  [] ? preempt_count_sub+0x13/0xc7
[  243.336018]  [] ? _raw_spin_unlock_irqrestore+0x34/0x49
[  243.336018]  [] ppp_xmit_process+0x48/0x877 [ppp_generic]
[  243.336018]  [] ? preempt_count_sub+0x13/0xc7
[  243.336018]  [] ? skb_queue_tail+0x71/0x7c
[  243.336018]  [] ppp_start_xmit+0x21b/0x22a [ppp_generic]
[  243.336018]  [] dev_hard_start_xmit+0x15e/0x32c
[  243.336018]  [] sch_direct_xmit+0xd6/0x221
[  243.336018]  [] __dev_queue_xmit+0x52a/0x820
[  243.336018]  [] dev_queue_xmit+0xb/0xd
[  243.336018]  [] neigh_direct_output+0xc/0xe
[  243.336018]  [] ip_finish_output2+0x4d2/0x548
[  243.336018]  [] ? dst_mtu+0x29/0x2e
[  243.336018]  [] ip_finish_output+0x152/0x15e
[  243.336018]  [] ? ip_output+0x74/0x96
[  243.336018]  [] ip_output+0x8c/0x96
[  243.336018]  [] ip_local_out+0x41/0x4a
[  243.336018]  [] ip_queue_xmit+0x531/0x5c5
[  243.336018]  [] ? udp_set_csum+0x207/0x21e
[  243.336018]  [] l2tp_xmit_skb+0x582/0x5d7 [l2tp_core]
[  243.336018]  [] pppol2tp_xmit+0x1eb/0x257 [l2tp_ppp]
[  243.336018]  [] ppp_channel_push+0x91/0x102 [ppp_generic]
[  243.336018]  [] ppp_write+0x104/0x11c [ppp_generic]
[  243.336018]  [] __vfs_write+0x56/0x120
[  243.336018]  [] ? fsnotify_perm+0x27/0x95
[  243.336018]  [] ? security_file_permission+0x4d/0x54
[  243.336018]  [] vfs_write+0xbd/0x11b
[  243.336018]  [] SyS_write+0x5e/0x96
[  243.336018]  [] entry_SYSCALL_64_fastpath+0x13/0x94

The main entry points for sending packets over a PPP unit are the
.write() and .ndo_start_xmit() callbacks (simplified view):

.write(unit fd) or .ndo_start_xmit()
       \
        CALL ppp_xmit_process()
               \
                LOCK unit's xmit path (ppp->wlock)
                |
                CALL ppp_push()
                       \
                        LOCK channel's xmit path (chan->downl)
                        |
                        CALL lower layer's .start_xmit() callback
                               \
                                ... might recursively call .ndo_start_xmit() ...
                               /
                        RETURN from .start_xmit()
                        |
                        UNLOCK channel's xmit path
                       /
                RETURN from ppp_push()
                |
                UNLOCK unit's xmit path
               /
        RETURN from ppp_xmit_process()

Packets can also be directly sent on channels (e.g. LCP packets):

.write(channel fd) or ppp_output_wakeup()
       \
        CALL ppp_channel_push()
               \
                LOCK channel's xmit path (chan->downl)
                |
                CALL lower layer's .start_xmit() callback
                       \
                        ... might call .ndo_start_xmit() ...
                       /
                RETURN from .start_xmit()
                |
                UNLOCK channel's xmit path
               /
        RETURN from ppp_channel_push()

Key points about the lower layer's .start_xmit() callback:

  * It can be called directly by a channel fd .write() or by
    ppp_output_wakeup() or indirectly by a unit fd .write() or by
    .ndo_start_xmit().

  * In any case, it's always called with chan->downl held.

  * It might route the packet back to its parent unit using
    .ndo_start_xmit() as entry point.

This patch detects and breaks recursion in ppp_xmit_process(). This
function is a good candidate for the task because it's called early
enough after .ndo_start_xmit(), it's always part of the recursion
loop and it's on the path of whatever entry point is used to send
a packet on a PPP unit.

Recursion detection is done using the per-cpu ppp_xmit_recursion
variable.

Since ppp_channel_push() too locks the channel's xmit path and calls
the lower layer's .start_xmit() callback, we need to also increment
ppp_xmit_recursion there. However there's no need to check for
recursion, as it's out of the recursion loop.

Reported-by: Feng Gao 
Signed-off-by: Guillaume Nault 
Signed-off-by: David S. Miller

pptp: Refactor the struct and macros of PPTP codes

2016-08-15T17:55:53+00:00

1. Use struct gre_base_hdr directly in pptp_gre_header instead of
duplicated members;
2. Use existing macros like GRE_KEY, GRE_SEQ, and so on instead of
duplicated macros defined by PPTP;
3. Add new macros like GRE_IS_ACK/SEQ and so on instead of
PPTP_GRE_IS_A/S and so on;

Signed-off-by: Gao Feng 
Reviewed-by: Philip Prindeville 
Signed-off-by: David S. Miller