28 Mar, 2018

1 commit


27 Mar, 2018

1 commit


23 Mar, 2018

2 commits

  • Fun set of conflict resolutions here...

    For the mac80211 stuff, these were fortunately just parallel
    adds. Trivially resolved.

    In drivers/net/phy/phy.c we had a bug fix in 'net' that moved the
    function phy_disable_interrupts() earlier in the file, whilst in
    'net-next' the phy_error() call from this function was removed.

    In net/ipv4/xfrm4_policy.c, David Ahern's changes to remove the
    'rt_table_id' member of rtable collided with a bug fix in 'net' that
    added a new struct member "rt_mtu_locked" which needs to be copied
    over here.

    The mlxsw driver conflict consisted of net-next separating
    the span code and definitions into separate files, whilst
    a 'net' bug fix made some changes to that moved code.

    The mlx5 infiniband conflict resolution was quite non-trivial,
    the RDMA tree's merge commit was used as a guide here, and
    here are their notes:

    ====================

    Due to bug fixes found by the syzkaller bot and taken into the for-rc
    branch after development for the 4.17 merge window had already started
    being taken into the for-next branch, there were fairly non-trivial
    merge issues that would need to be resolved between the for-rc branch
    and the for-next branch. This merge resolves those conflicts and
    provides a unified base upon which ongoing development for 4.17 can
    be based.

    Conflicts:
    drivers/infiniband/hw/mlx5/main.c - Commit 42cea83f9524
    (IB/mlx5: Fix cleanup order on unload) added to for-rc and
    commit b5ca15ad7e61 (IB/mlx5: Add proper representors support)
    add as part of the devel cycle both needed to modify the
    init/de-init functions used by mlx5. To support the new
    representors, the new functions added by the cleanup patch
    needed to be made non-static, and the init/de-init list
    added by the representors patch needed to be modified to
    match the init/de-init list changes made by the cleanup
    patch.
    Updates:
    drivers/infiniband/hw/mlx5/mlx5_ib.h - Update function
    prototypes added by representors patch to reflect new function
    names as changed by cleanup patch
    drivers/infiniband/hw/mlx5/ib_rep.c - Update init/de-init
    stage list to match new order from cleanup patch
    ====================

    Signed-off-by: David S. Miller

    David S. Miller
     
  • We already detect situations where a PPP channel sends packets back to
    its upper PPP device. While this is enough to avoid deadlocking on xmit
    locks, this doesn't prevent packets from looping between the channel
    and the unit.

    The problem is that ppp_start_xmit() enqueues packets in ppp->file.xq
    before checking for xmit recursion. Therefore, __ppp_xmit_process()
    might dequeue a packet from ppp->file.xq and send it on the channel
    which, in turn, loops it back on the unit. Then ppp_start_xmit()
    queues the packet back to ppp->file.xq and __ppp_xmit_process() picks
    it up and sends it again through the channel. Therefore, the packet
    will loop between __ppp_xmit_process() and ppp_start_xmit() until some
    other part of the xmit path drops it.

    For L2TP, we rapidly fill the skb's headroom and pppol2tp_xmit() drops
    the packet after a few iterations. But PPTP reallocates the headroom
    if necessary, letting the loop run and exhaust the machine resources
    (as reported in https://bugzilla.kernel.org/show_bug.cgi?id=199109).

    Fix this by letting __ppp_xmit_process() enqueue the skb to
    ppp->file.xq, so that we can check for recursion before adding it to
    the queue. Now ppp_xmit_process() can drop the packet when recursion is
    detected.

    __ppp_channel_push() is a bit special. It calls __ppp_xmit_process()
    without having any actual packet to send. This is used by
    ppp_output_wakeup() to re-enable transmission on the parent unit (for
    implementations like ppp_async.c, where the .start_xmit() function
    might not consume the skb, leaving it in ppp->xmit_pending and
    disabling transmission).
    Therefore, __ppp_xmit_process() needs to handle the case where skb is
    NULL, dequeuing as many packets as possible from ppp->file.xq.

    Reported-by: xu heng
    Fixes: 55454a565836 ("ppp: avoid dealock on recursive xmit")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

06 Mar, 2018

1 commit


05 Mar, 2018

1 commit

  • 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

    Guillaume Nault
     

28 Feb, 2018

1 commit

  • These pernet_operations are similar to bond_net_ops. Exit method
    unregisters all net ppp devices, and it looks like another
    pernet_operations are not interested in foreign net ppp list.
    So, it's possible to mark them async.

    Signed-off-by: Kirill Tkhai
    Signed-off-by: David S. Miller

    Kirill Tkhai
     

12 Feb, 2018

1 commit

  • This is the mindless scripted replacement of kernel use of POLL*
    variables as described by Al, done by this script:

    for V in IN OUT PRI ERR RDNORM RDBAND WRNORM WRBAND HUP RDHUP NVAL MSG; do
    L=`git grep -l -w POLL$V | grep -v '^t' | grep -v /um/ | grep -v '^sa' | grep -v '/poll.h$'|grep -v '^D'`
    for f in $L; do sed -i "-es/^\([^\"]*\)\(\\)/\\1E\\2/" $f; done
    done

    with de-mangling cleanups yet to come.

    NOTE! On almost all architectures, the EPOLL* constants have the same
    values as the POLL* constants do. But they keyword here is "almost".
    For various bad reasons they aren't the same, and epoll() doesn't
    actually work quite correctly in some cases due to this on Sparc et al.

    The next patch from Al will sort out the final differences, and we
    should be all done.

    Scripted-by: Al Viro
    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

31 Jan, 2018

1 commit

  • Pull poll annotations from Al Viro:
    "This introduces a __bitwise type for POLL### bitmap, and propagates
    the annotations through the tree. Most of that stuff is as simple as
    'make ->poll() instances return __poll_t and do the same to local
    variables used to hold the future return value'.

    Some of the obvious brainos found in process are fixed (e.g. POLLIN
    misspelled as POLL_IN). At that point the amount of sparse warnings is
    low and most of them are for genuine bugs - e.g. ->poll() instance
    deciding to return -EINVAL instead of a bitmap. I hadn't touched those
    in this series - it's large enough as it is.

    Another problem it has caught was eventpoll() ABI mess; select.c and
    eventpoll.c assumed that corresponding POLL### and EPOLL### were
    equal. That's true for some, but not all of them - EPOLL### are
    arch-independent, but POLL### are not.

    The last commit in this series separates userland POLL### values from
    the (now arch-independent) kernel-side ones, converting between them
    in the few places where they are copied to/from userland. AFAICS, this
    is the least disruptive fix preserving poll(2) ABI and making epoll()
    work on all architectures.

    As it is, it's simply broken on sparc - try to give it EPOLLWRNORM and
    it will trigger only on what would've triggered EPOLLWRBAND on other
    architectures. EPOLLWRBAND and EPOLLRDHUP, OTOH, are never triggered
    at all on sparc. With this patch they should work consistently on all
    architectures"

    * 'misc.poll' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (37 commits)
    make kernel-side POLL... arch-independent
    eventpoll: no need to mask the result of epi_item_poll() again
    eventpoll: constify struct epoll_event pointers
    debugging printk in sg_poll() uses %x to print POLL... bitmap
    annotate poll(2) guts
    9p: untangle ->poll() mess
    ->si_band gets POLL... bitmap stored into a user-visible long field
    ring_buffer_poll_wait() return value used as return value of ->poll()
    the rest of drivers/*: annotate ->poll() instances
    media: annotate ->poll() instances
    fs: annotate ->poll() instances
    ipc, kernel, mm: annotate ->poll() instances
    net: annotate ->poll() instances
    apparmor: annotate ->poll() instances
    tomoyo: annotate ->poll() instances
    sound: annotate ->poll() instances
    acpi: annotate ->poll() instances
    crypto: annotate ->poll() instances
    block: annotate ->poll() instances
    x86: annotate ->poll() instances
    ...

    Linus Torvalds
     

16 Jan, 2018

1 commit

  • 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

    Guillaume Nault
     

29 Nov, 2017

1 commit


14 Nov, 2017

1 commit


01 Nov, 2017

1 commit

  • 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

    Gao Feng
     

29 Oct, 2017

1 commit


22 Oct, 2017

1 commit

  • atomic_t variables are currently used to implement reference
    counters with the following properties:
    - counter is initialized to 1 using atomic_set()
    - a resource is freed upon counter reaching zero
    - once counter reaches zero, its further
    increments aren't allowed
    - counter schema uses basic atomic operations
    (set, inc, inc_not_zero, dec_and_test, etc.)

    Such atomic variables should be converted to a newly provided
    refcount_t type and API that prevents accidental counter overflows
    and underflows. This is important since overflows and underflows
    can lead to use-after-free situation and be exploitable.

    The variable ppp_file.refcnt is used as pure reference counter.
    Convert it to refcount_t and fix up the operations.

    Suggested-by: Kees Cook
    Reviewed-by: David Windsor
    Reviewed-by: Hans Liljestrand
    Signed-off-by: Elena Reshetova
    Signed-off-by: David S. Miller

    Elena Reshetova
     

07 Oct, 2017

1 commit

  • 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

    Guillaume Nault
     

01 Oct, 2017

1 commit

  • Move sparse annotation right after pointer type.

    Fixes sparse warning:
    drivers/net/ppp/ppp_generic.c:1422:13: warning: incorrect type in initializer (different address spaces)
    drivers/net/ppp/ppp_generic.c:1422:13: expected void const [noderef] *__vpp_verify
    drivers/net/ppp/ppp_generic.c:1422:13: got int *
    ...

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

    Guillaume Nault
     

09 Aug, 2017

1 commit

  • 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

    Guillaume Nault
     

19 Jul, 2017

1 commit

  • 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

    Gao Feng
     

27 Jun, 2017

2 commits


16 Jun, 2017

1 commit

  • It seems like a historic accident that these return unsigned char *,
    and in many places that means casts are required, more often than not.

    Make these functions return void * and remove all the casts across
    the tree, adding a (u8 *) cast only where the unsigned char pointer
    was used directly, all done with the following spatch:

    @@
    expression SKB, LEN;
    typedef u8;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    @@
    - *(fn(SKB, LEN))
    + *(u8 *)fn(SKB, LEN)

    @@
    expression E, SKB, LEN;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    type T;
    @@
    - E = ((T *)(fn(SKB, LEN)))
    + E = fn(SKB, LEN)

    @@
    expression SKB, LEN;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    @@
    - fn(SKB, LEN)[0]
    + *(u8 *)fn(SKB, LEN)

    Note that the last part there converts from push(...)[0] to the
    more idiomatic *(u8 *)push(...).

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     

01 Jun, 2017

1 commit

  • Since the commit 55454a565836 ("ppp: avoid dealock on recursive xmit"),
    the PPP xmit path is protected by wrapper functions which disable the
    bh already. So it is unnecessary to disable the bh again in the real
    xmit path.

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

    Gao Feng
     

02 Mar, 2017

1 commit


09 Jan, 2017

1 commit

  • The network device operation for reading statistics is only called
    in one place, and it ignores the return value. Having a structure
    return value is potentially confusing because some future driver could
    incorrectly assume that the return value was used.

    Fix all drivers with ndo_get_stats64 to have a void function.

    Signed-off-by: Stephen Hemminger
    Signed-off-by: David S. Miller

    stephen hemminger
     

18 Nov, 2016

1 commit

  • Make struct pernet_operations::id unsigned.

    There are 2 reasons to do so:

    1)
    This field is really an index into an zero based array and
    thus is unsigned entity. Using negative value is out-of-bound
    access by definition.

    2)
    On x86_64 unsigned 32-bit data which are mixed with pointers
    via array indexing or offsets added or subtracted to pointers
    are preffered to signed 32-bit data.

    "int" being used as an array index needs to be sign-extended
    to 64-bit before being used.

    void f(long *p, int i)
    {
    g(p[i]);
    }

    roughly translates to

    movsx rsi, esi
    mov rdi, [rsi+...]
    call g

    MOVSX is 3 byte instruction which isn't necessary if the variable is
    unsigned because x86_64 is zero extending by default.

    Now, there is net_generic() function which, you guessed it right, uses
    "int" as an array index:

    static inline void *net_generic(const struct net *net, int id)
    {
    ...
    ptr = ng->ptr[id - 1];
    ...
    }

    And this function is used a lot, so those sign extensions add up.

    Patch snipes ~1730 bytes on allyesconfig kernel (without all junk
    messing with code generation):

    add/remove: 0/0 grow/shrink: 70/598 up/down: 396/-2126 (-1730)

    Unfortunately some functions actually grow bigger.
    This is a semmingly random artefact of code generation with register
    allocator being used differently. gcc decides that some variable
    needs to live in new r8+ registers and every access now requires REX
    prefix. Or it is shifted into r12, so [r12+0] addressing mode has to be
    used which is longer than [r8]

    However, overall balance is in negative direction:

    add/remove: 0/0 grow/shrink: 70/598 up/down: 396/-2126 (-1730)
    function old new delta
    nfsd4_lock 3886 3959 +73
    tipc_link_build_proto_msg 1096 1140 +44
    mac80211_hwsim_new_radio 2776 2808 +32
    tipc_mon_rcv 1032 1058 +26
    svcauth_gss_legacy_init 1413 1429 +16
    tipc_bcbase_select_primary 379 392 +13
    nfsd4_exchange_id 1247 1260 +13
    nfsd4_setclientid_confirm 782 793 +11
    ...
    put_client_renew_locked 494 480 -14
    ip_set_sockfn_get 730 716 -14
    geneve_sock_add 829 813 -16
    nfsd4_sequence_done 721 703 -18
    nlmclnt_lookup_host 708 686 -22
    nfsd4_lockt 1085 1063 -22
    nfs_get_client 1077 1050 -27
    tcf_bpf_init 1106 1076 -30
    nfsd4_encode_fattr 5997 5930 -67
    Total: Before=154856051, After=154854321, chg -0.00%

    Signed-off-by: Alexey Dobriyan
    Signed-off-by: David S. Miller

    Alexey Dobriyan
     

01 Sep, 2016

2 commits

  • 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

    Guillaume Nault
     
  • 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

    Guillaume Nault
     

10 Aug, 2016

1 commit

  • Userspace programs generally need to know the name of the ppp devices
    they create. Both ioctl and rtnl interfaces use the ppp sheme
    to name them. But although the suffix used by the ioctl interface can
    be known by userspace (it's the PPP unit identifier returned by the
    PPPIOCGUNIT ioctl), the one used by the rtnl is only known by the
    kernel.

    This patch brings more consistency between ioctl and rtnl based ppp
    devices by generating device names using the PPP unit identifer as
    suffix in both cases. This way, userspace can always infer the name of
    the devices they create.

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

    Guillaume Nault
     

24 Jul, 2016

1 commit


09 Jul, 2016

1 commit

  • Matt reported that we have a NULL pointer dereference
    in ppp_pernet() from ppp_connect_channel(),
    i.e. pch->chan_net is NULL.

    This is due to that a parallel ppp_unregister_channel()
    could happen while we are in ppp_connect_channel(), during
    which pch->chan_net set to NULL. Since we need a reference
    to net per channel, it makes sense to sync the refcnt
    with the life time of the channel, therefore we should
    release this reference when we destroy it.

    Fixes: 1f461dcdd296 ("ppp: take reference on channels netns")
    Reported-by: Matt Bennett
    Cc: Paul Mackerras
    Cc: linux-ppp@vger.kernel.org
    Cc: Guillaume Nault
    Cc: Cyrill Gorcunov
    Signed-off-by: Cong Wang
    Reviewed-by: Cyrill Gorcunov
    Signed-off-by: David S. Miller

    WANG Cong
     

10 Jun, 2016

1 commit


08 Jun, 2016

1 commit

  • Instead of using a single bit (__QDISC___STATE_RUNNING)
    in sch->__state, use a seqcount.

    This adds lockdep support, but more importantly it will allow us
    to sample qdisc/class statistics without having to grab qdisc root lock.

    Signed-off-by: Eric Dumazet
    Cc: Cong Wang
    Cc: Jamal Hadi Salim
    Signed-off-by: David S. Miller

    Eric Dumazet
     

30 Apr, 2016

2 commits

  • Define PPP device handler for use with rtnetlink.
    The only PPP specific attribute is IFLA_PPP_DEV_FD. It is mandatory and
    contains the file descriptor of the associated /dev/ppp instance (the
    file descriptor which would have been used for ioctl(PPPIOCNEWUNIT) in
    the ioctl-based API). The PPP device is removed when this file
    descriptor is released (same behaviour as with ioctl based PPP
    devices).

    PPP devices created with the rtnetlink API behave like the ones created
    with ioctl(PPPIOCNEWUNIT). In particular existing ioctls work the same
    way, no matter how the PPP device was created.
    The rtnl callbacks are also assigned to ioctl based PPP devices. This
    way, rtnl messages have the same effect on any PPP devices.
    The immediate effect is that all PPP devices, even ioctl-based
    ones, can now be removed with "ip link del".

    A minor difference still exists between ioctl and rtnl based PPP
    interfaces: in the device name, the number following the "ppp" prefix
    corresponds to the PPP unit number for ioctl based devices, while it is
    just an unrelated incrementing index for rtnl ones.

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

    Guillaume Nault
     
  • Move PPP device initialisation and registration out of
    ppp_create_interface().
    This prepares code for device registration with rtnetlink.

    While there, simplify the prototype of ppp_create_interface():

    * Since ppp_dev_configure() takes care of setting file->private_data,
    there's no need to return a ppp structure to ppp_unattached_ioctl()
    anymore.

    * The unit parameter is made read/write so that ppp_create_interface()
    can tell which unit number has been assigned.

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

    Guillaume Nault
     

24 Mar, 2016

1 commit

  • Let channels hold a reference on their network namespace.
    Some channel types, like ppp_async and ppp_synctty, can have their
    userspace controller running in a different namespace. Therefore they
    can't rely on them to preclude their netns from being removed from
    under them.

    ==================================================================
    BUG: KASAN: use-after-free in ppp_unregister_channel+0x372/0x3a0 at
    addr ffff880064e217e0
    Read of size 8 by task syz-executor/11581
    =============================================================================
    BUG net_namespace (Not tainted): kasan: bad access detected
    -----------------------------------------------------------------------------

    Disabling lock debugging due to kernel taint
    INFO: Allocated in copy_net_ns+0x6b/0x1a0 age=92569 cpu=3 pid=6906
    [< none >] ___slab_alloc+0x4c7/0x500 kernel/mm/slub.c:2440
    [< none >] __slab_alloc+0x4c/0x90 kernel/mm/slub.c:2469
    [< inline >] slab_alloc_node kernel/mm/slub.c:2532
    [< inline >] slab_alloc kernel/mm/slub.c:2574
    [< none >] kmem_cache_alloc+0x23a/0x2b0 kernel/mm/slub.c:2579
    [< inline >] kmem_cache_zalloc kernel/include/linux/slab.h:597
    [< inline >] net_alloc kernel/net/core/net_namespace.c:325
    [< none >] copy_net_ns+0x6b/0x1a0 kernel/net/core/net_namespace.c:360
    [< none >] create_new_namespaces+0x2f6/0x610 kernel/kernel/nsproxy.c:95
    [< none >] copy_namespaces+0x297/0x320 kernel/kernel/nsproxy.c:150
    [< none >] copy_process.part.35+0x1bf4/0x5760 kernel/kernel/fork.c:1451
    [< inline >] copy_process kernel/kernel/fork.c:1274
    [< none >] _do_fork+0x1bc/0xcb0 kernel/kernel/fork.c:1723
    [< inline >] SYSC_clone kernel/kernel/fork.c:1832
    [< none >] SyS_clone+0x37/0x50 kernel/kernel/fork.c:1826
    [< none >] entry_SYSCALL_64_fastpath+0x16/0x7a kernel/arch/x86/entry/entry_64.S:185

    INFO: Freed in net_drop_ns+0x67/0x80 age=575 cpu=2 pid=2631
    [< none >] __slab_free+0x1fc/0x320 kernel/mm/slub.c:2650
    [< inline >] slab_free kernel/mm/slub.c:2805
    [< none >] kmem_cache_free+0x2a0/0x330 kernel/mm/slub.c:2814
    [< inline >] net_free kernel/net/core/net_namespace.c:341
    [< none >] net_drop_ns+0x67/0x80 kernel/net/core/net_namespace.c:348
    [< none >] cleanup_net+0x4e5/0x600 kernel/net/core/net_namespace.c:448
    [< none >] process_one_work+0x794/0x1440 kernel/kernel/workqueue.c:2036
    [< none >] worker_thread+0xdb/0xfc0 kernel/kernel/workqueue.c:2170
    [< none >] kthread+0x23f/0x2d0 kernel/drivers/block/aoe/aoecmd.c:1303
    [< none >] ret_from_fork+0x3f/0x70 kernel/arch/x86/entry/entry_64.S:468
    INFO: Slab 0xffffea0001938800 objects=3 used=0 fp=0xffff880064e20000
    flags=0x5fffc0000004080
    INFO: Object 0xffff880064e20000 @offset=0 fp=0xffff880064e24200

    CPU: 1 PID: 11581 Comm: syz-executor Tainted: G B 4.4.0+
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
    rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
    00000000ffffffff ffff8800662c7790 ffffffff8292049d ffff88003e36a300
    ffff880064e20000 ffff880064e20000 ffff8800662c77c0 ffffffff816f2054
    ffff88003e36a300 ffffea0001938800 ffff880064e20000 0000000000000000
    Call Trace:
    [< inline >] __dump_stack kernel/lib/dump_stack.c:15
    [] dump_stack+0x6f/0xa2 kernel/lib/dump_stack.c:50
    [] print_trailer+0xf4/0x150 kernel/mm/slub.c:654
    [] object_err+0x2f/0x40 kernel/mm/slub.c:661
    [< inline >] print_address_description kernel/mm/kasan/report.c:138
    [] kasan_report_error+0x215/0x530 kernel/mm/kasan/report.c:236
    [< inline >] kasan_report kernel/mm/kasan/report.c:259
    [] __asan_report_load8_noabort+0x3e/0x40 kernel/mm/kasan/report.c:280
    [< inline >] ? ppp_pernet kernel/include/linux/compiler.h:218
    [] ? ppp_unregister_channel+0x372/0x3a0 kernel/drivers/net/ppp/ppp_generic.c:2392
    [< inline >] ppp_pernet kernel/include/linux/compiler.h:218
    [] ppp_unregister_channel+0x372/0x3a0 kernel/drivers/net/ppp/ppp_generic.c:2392
    [< inline >] ? ppp_pernet kernel/drivers/net/ppp/ppp_generic.c:293
    [] ? ppp_unregister_channel+0xe6/0x3a0 kernel/drivers/net/ppp/ppp_generic.c:2392
    [] ppp_asynctty_close+0xa3/0x130 kernel/drivers/net/ppp/ppp_async.c:241
    [] ? async_lcp_peek+0x5b0/0x5b0 kernel/drivers/net/ppp/ppp_async.c:1000
    [] tty_ldisc_close.isra.1+0x99/0xe0 kernel/drivers/tty/tty_ldisc.c:478
    [] tty_ldisc_kill+0x40/0x170 kernel/drivers/tty/tty_ldisc.c:744
    [] tty_ldisc_release+0x1b3/0x260 kernel/drivers/tty/tty_ldisc.c:772
    [] tty_release+0xac1/0x13e0 kernel/drivers/tty/tty_io.c:1901
    [] ? release_tty+0x320/0x320 kernel/drivers/tty/tty_io.c:1688
    [] __fput+0x236/0x780 kernel/fs/file_table.c:208
    [] ____fput+0x15/0x20 kernel/fs/file_table.c:244
    [] task_work_run+0x16b/0x200 kernel/kernel/task_work.c:115
    [< inline >] exit_task_work kernel/include/linux/task_work.h:21
    [] do_exit+0x8b5/0x2c60 kernel/kernel/exit.c:750
    [] ? debug_check_no_locks_freed+0x290/0x290 kernel/kernel/locking/lockdep.c:4123
    [] ? mm_update_next_owner+0x6f0/0x6f0 kernel/kernel/exit.c:357
    [] ? __dequeue_signal+0x136/0x470 kernel/kernel/signal.c:550
    [] ? recalc_sigpending_tsk+0x13b/0x180 kernel/kernel/signal.c:145
    [] do_group_exit+0x108/0x330 kernel/kernel/exit.c:880
    [] get_signal+0x5e4/0x14f0 kernel/kernel/signal.c:2307
    [< inline >] ? kretprobe_table_lock kernel/kernel/kprobes.c:1113
    [] ? kprobe_flush_task+0xb5/0x450 kernel/kernel/kprobes.c:1158
    [] do_signal+0x83/0x1c90 kernel/arch/x86/kernel/signal.c:712
    [] ? recycle_rp_inst+0x310/0x310 kernel/include/linux/list.h:655
    [] ? setup_sigcontext+0x780/0x780 kernel/arch/x86/kernel/signal.c:165
    [] ? finish_task_switch+0x424/0x5f0 kernel/kernel/sched/core.c:2692
    [< inline >] ? finish_lock_switch kernel/kernel/sched/sched.h:1099
    [] ? finish_task_switch+0x120/0x5f0 kernel/kernel/sched/core.c:2678
    [< inline >] ? context_switch kernel/kernel/sched/core.c:2807
    [] ? __schedule+0x919/0x1bd0 kernel/kernel/sched/core.c:3283
    [] exit_to_usermode_loop+0xf1/0x1a0 kernel/arch/x86/entry/common.c:247
    [< inline >] prepare_exit_to_usermode kernel/arch/x86/entry/common.c:282
    [] syscall_return_slowpath+0x19f/0x210 kernel/arch/x86/entry/common.c:344
    [] int_ret_from_sys_call+0x25/0x9f kernel/arch/x86/entry/entry_64.S:281
    Memory state around the buggy address:
    ffff880064e21680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ffff880064e21700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    >ffff880064e21780: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ^
    ffff880064e21800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ffff880064e21880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ==================================================================

    Fixes: 273ec51dd7ce ("net: ppp_generic - introduce net-namespace functionality v2")
    Reported-by: Baozeng Ding
    Signed-off-by: Guillaume Nault
    Reviewed-by: Cyrill Gorcunov
    Signed-off-by: David S. Miller

    Guillaume Nault
     

17 Mar, 2016

1 commit

  • Locking ppp_mutex must be done before dereferencing file->private_data,
    otherwise it could be modified before ppp_unattached_ioctl() takes the
    lock. This could lead ppp_unattached_ioctl() to override ->private_data,
    thus leaking reference to the ppp_file previously pointed to.

    v2: lock all ppp_ioctl() instead of just checking private_data in
    ppp_unattached_ioctl(), to avoid ambiguous behaviour.

    Fixes: f3ff8a4d80e8 ("ppp: push BKL down into the driver")
    Signed-off-by: Guillaume Nault
    Signed-off-by: David S. Miller

    Guillaume Nault
     

09 Mar, 2016

1 commit


08 Mar, 2016

1 commit


02 Mar, 2016

1 commit

  • ppp_read() and ppp_poll() can be called concurrently with ppp_ioctl().
    In this case, ppp_ioctl() might call ppp_ccp_closed(), which may update
    ppp->flags while ppp_read() or ppp_poll() is reading it.
    The update done by ppp_ccp_closed() isn't atomic due to the bit mask
    operation ('ppp->flags &= ~(SC_CCP_OPEN | SC_CCP_UP)'), so concurrent
    readers might get transient values.
    Reading incorrect ppp->flags may disturb the 'ppp->flags & SC_LOOP_TRAFFIC'
    test in ppp_read() and ppp_poll(), which in turn can lead to improper
    decision on whether the PPP unit file is ready for reading or not.

    Since ppp_ccp_closed() is protected by the Rx and Tx locks (with
    ppp_lock()), taking the Rx lock is enough for ppp_read() and ppp_poll()
    to guarantee that ppp_ccp_closed() won't update ppp->flags
    concurrently.

    The same reasoning applies to ppp->n_channels. The 'n_channels' field
    can also be written to concurrently by ppp_ioctl() (through
    ppp_connect_channel() or ppp_disconnect_channel()). These writes aren't
    atomic (simple increment/decrement), but are protected by both the Rx
    and Tx locks (like in the ppp->flags case). So holding the Rx lock
    before reading ppp->n_channels also prevents concurrent writes.

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

    Guillaume Nault