10 Mar, 2017

1 commit

  • Lockdep issues a circular dependency warning when AFS issues an operation
    through AF_RXRPC from a context in which the VFS/VM holds the mmap_sem.

    The theory lockdep comes up with is as follows:

    (1) If the pagefault handler decides it needs to read pages from AFS, it
    calls AFS with mmap_sem held and AFS begins an AF_RXRPC call, but
    creating a call requires the socket lock:

    mmap_sem must be taken before sk_lock-AF_RXRPC

    (2) afs_open_socket() opens an AF_RXRPC socket and binds it. rxrpc_bind()
    binds the underlying UDP socket whilst holding its socket lock.
    inet_bind() takes its own socket lock:

    sk_lock-AF_RXRPC must be taken before sk_lock-AF_INET

    (3) Reading from a TCP socket into a userspace buffer might cause a fault
    and thus cause the kernel to take the mmap_sem, but the TCP socket is
    locked whilst doing this:

    sk_lock-AF_INET must be taken before mmap_sem

    However, lockdep's theory is wrong in this instance because it deals only
    with lock classes and not individual locks. The AF_INET lock in (2) isn't
    really equivalent to the AF_INET lock in (3) as the former deals with a
    socket entirely internal to the kernel that never sees userspace. This is
    a limitation in the design of lockdep.

    Fix the general case by:

    (1) Double up all the locking keys used in sockets so that one set are
    used if the socket is created by userspace and the other set is used
    if the socket is created by the kernel.

    (2) Store the kern parameter passed to sk_alloc() in a variable in the
    sock struct (sk_kern_sock). This informs sock_lock_init(),
    sock_init_data() and sk_clone_lock() as to the lock keys to be used.

    Note that the child created by sk_clone_lock() inherits the parent's
    kern setting.

    (3) Add a 'kern' parameter to ->accept() that is analogous to the one
    passed in to ->create() that distinguishes whether kernel_accept() or
    sys_accept4() was the caller and can be passed to sk_alloc().

    Note that a lot of accept functions merely dequeue an already
    allocated socket. I haven't touched these as the new socket already
    exists before we get the parameter.

    Note also that there are a couple of places where I've made the accepted
    socket unconditionally kernel-based:

    irda_accept()
    rds_rcp_accept_one()
    tcp_accept_from_sock()

    because they follow a sock_create_kern() and accept off of that.

    Whilst creating this, I noticed that lustre and ocfs don't create sockets
    through sock_create_kern() and thus they aren't marked as for-kernel,
    though they appear to be internal. I wonder if these should do that so
    that they use the new set of lock keys.

    Signed-off-by: David Howells
    Signed-off-by: David S. Miller

    David Howells
     

02 Mar, 2017

1 commit


06 Jun, 2015

1 commit


11 May, 2015

1 commit


03 Mar, 2015

1 commit

  • After TIPC doesn't depend on iocb argument in its internal
    implementations of sendmsg() and recvmsg() hooks defined in proto
    structure, no any user is using iocb argument in them at all now.
    Then we can drop the redundant iocb argument completely from kinds of
    implementations of both sendmsg() and recvmsg() in the entire
    networking stack.

    Cc: Christoph Hellwig
    Suggested-by: Al Viro
    Signed-off-by: Ying Xue
    Signed-off-by: David S. Miller

    Ying Xue
     

24 Jan, 2015

1 commit

  • l2cap/rfcomm/sco_sock_accept() are wait loops which may acquire
    sleeping locks. Since both wait loops and sleeping locks use
    task_struct.state to sleep and wake, the nested sleeping locks
    destroy the wait loop state.

    Use the newly-minted wait_woken() and DEFINE_WAIT_FUNC() for the
    wait loop. DEFINE_WAIT_FUNC() allows an alternate wake function
    to be specified; in this case, the predefined scheduler function,
    woken_wake_function(). This wait construct ensures wakeups will
    not be missed without requiring the wait loop to set the
    task state before condition evaluation. How this works:

    CPU 0 | CPU 1
    |
    | is set?
    | no
    set |
    |
    wake_up_interruptible |
    woken_wake_function |
    set WQ_FLAG_WOKEN |
    try_to_wake_up |
    | wait_woken
    | set TASK_INTERRUPTIBLE
    | WQ_FLAG_WOKEN? yes
    | set TASK_RUNNING
    |
    | - loop -
    |
    | is set?
    | yes - exit wait loop

    Fixes "do not call blocking ops when !TASK_RUNNING" warnings
    in l2cap_sock_accept(), rfcomm_sock_accept() and sco_sock_accept().

    Signed-off-by: Peter Hurley
    Signed-off-by: Johan Hedberg

    Peter Hurley
     

12 Jan, 2015

1 commit


24 Nov, 2014

1 commit


17 Jul, 2014

1 commit

  • If the current process is exiting, lingering on socket close will make
    it unkillable, so we should avoid it.

    Reproducer:

    #include
    #include

    #define BTPROTO_L2CAP 0
    #define BTPROTO_SCO 2
    #define BTPROTO_RFCOMM 3

    int main()
    {
    int fd;
    struct linger ling;

    fd = socket(PF_BLUETOOTH, SOCK_STREAM, BTPROTO_RFCOMM);
    //or: fd = socket(PF_BLUETOOTH, SOCK_DGRAM, BTPROTO_L2CAP);
    //or: fd = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_SCO);

    ling.l_onoff = 1;
    ling.l_linger = 1000000000;
    setsockopt(fd, SOL_SOCKET, SO_LINGER, &ling, sizeof(ling));

    return 0;
    }

    Signed-off-by: Vladimir Davydov
    Signed-off-by: Marcel Holtmann
    Cc: stable@vger.kernel.org

    Vladimir Davydov
     

12 Apr, 2014

1 commit

  • Several spots in the kernel perform a sequence like:

    skb_queue_tail(&sk->s_receive_queue, skb);
    sk->sk_data_ready(sk, skb->len);

    But at the moment we place the SKB onto the socket receive queue it
    can be consumed and freed up. So this skb->len access is potentially
    to freed up memory.

    Furthermore, the skb->len can be modified by the consumer so it is
    possible that the value isn't accurate.

    And finally, no actual implementation of this callback actually uses
    the length argument. And since nobody actually cared about it's
    value, lots of call sites pass arbitrary values in such as '0' and
    even '1'.

    So just remove the length argument from the callback, that way there
    is no confusion whatsoever and all of these use-after-free cases get
    fixed as a side effect.

    Based upon a patch by Eric Dumazet and his suggestion to audit this
    issue tree-wide.

    Signed-off-by: David S. Miller

    David S. Miller
     

27 Mar, 2014

1 commit

  • We should let user space request the peer address also in the pending
    connect states, i.e. BT_CONNECT and BT_CONNECT2. There is existing user
    space code that tries to do this and will fail without extending the set
    of allowed states for the peer address information.

    This patch adds the two states to the allowed ones in the L2CAP and
    RFCOMM sock_getname functions, thereby preventing ENOTCONN from being
    returned.

    Reported-by: Andrzej Kaczmarek
    Signed-off-by: Johan Hedberg
    Tested-by: Andrzej Kaczmarek
    Signed-off-by: Marcel Holtmann

    Johan Hedberg
     

21 Feb, 2014

1 commit

  • When binding RFCOMM socket with non-zero channel we're checking if
    there is already any other socket which has the same channel number
    assigned and then fail. This check does not consider situation where
    we have another socket connected to remote device on given channel
    number in which case we still should be able to bind local socket.

    This patch changes __rfcomm_get_sock_by_addr() to return only sockets
    in either BT_BOUND or BT_LISTEN states, also name is updated to better
    describe what this function does now.

    Signed-off-by: Andrzej Kaczmarek
    Signed-off-by: Marcel Holtmann

    Andrzej Kaczmarek
     

13 Feb, 2014

2 commits


21 Nov, 2013

2 commits


16 Nov, 2013

1 commit


13 Nov, 2013

1 commit

  • The commit 94a86df01082557e2de45865e538d7fb6c46231c seem to have
    uncovered a long standing bug that did not trigger so far.

    BUG: unable to handle kernel paging request at 00000009dd503502
    IP: [] rfcomm_sock_getsockopt+0x128/0x200
    PGD 0
    Oops: 0000 [#1] SMP
    Modules linked in: ath5k ath mac80211 cfg80211
    CPU: 2 PID: 1459 Comm: bluetoothd Not tainted 3.11.0-133163-gcebd830 #2
    Hardware name: System manufacturer System Product Name/P6T DELUXE V2, BIOS
    1202 12/22/2010
    task: ffff8803304106a0 ti: ffff88033046a000 task.ti: ffff88033046a000
    RIP: 0010:[] []
    rfcomm_sock_getsockopt+0x128/0x200
    RSP: 0018:ffff88033046bed8 EFLAGS: 00010246
    RAX: 00000009dd503502 RBX: 0000000000000003 RCX: 00007fffa2ed5548
    RDX: 0000000000000003 RSI: 0000000000000012 RDI: ffff88032fd37480
    RBP: ffff88033046bf28 R08: 00007fffa2ed554c R09: ffff88032f5707d8
    R10: 00007fffa2ed5548 R11: 0000000000000202 R12: ffff880330bbd000
    R13: 00007fffa2ed5548 R14: 0000000000000003 R15: 00007fffa2ed554c
    FS: 00007fc44cfac700(0000) GS:ffff88033fc80000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 00000009dd503502 CR3: 00000003304c2000 CR4: 00000000000007e0
    Stack:
    ffff88033046bf28 ffffffff815b0f2f ffff88033046bf18 0002ffff81105ef6
    0000000600000000 ffff88032fd37480 0000000000000012 00007fffa2ed5548
    0000000000000003 00007fffa2ed554c ffff88033046bf78 ffffffff814c0380
    Call Trace:
    [] ? rfcomm_sock_setsockopt+0x5f/0x190
    [] SyS_getsockopt+0x60/0xb0
    [] system_call_fastpath+0x16/0x1b
    Code: 02 00 00 00 0f 47 d0 4c 89 ef e8 74 13 cd ff 83 f8 01 19 c9 f7 d1 83 e1
    f2 e9 4b ff ff ff 0f 1f 44 00 00 49 8b 84 24 70 02 00 00 8b 30 4c 89 c0 e8
    2d 19 cd ff 85 c0 49 89 d7 b9 f2 ff ff ff
    RIP [] rfcomm_sock_getsockopt+0x128/0x200
    RSP
    CR2: 00000009dd503502

    It triggers in the following segment of the code:

    0x1313 is in rfcomm_sock_getsockopt (net/bluetooth/rfcomm/sock.c:743).
    738
    739 static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char __user *optval, int __user *optlen)
    740 {
    741 struct sock *sk = sock->sk;
    742 struct rfcomm_conninfo cinfo;
    743 struct l2cap_conn *conn = l2cap_pi(sk)->chan->conn;
    744 int len, err = 0;
    745 u32 opt;
    746
    747 BT_DBG("sk %p", sk);

    The l2cap_pi(sk) is wrong here since it should have been rfcomm_pi(sk),
    but that socket of course does not contain the low-level connection
    details requested here.

    Tracking down the actual offending commit, it seems that this has been
    introduced when doing some L2CAP refactoring:

    commit 8c1d787be4b62d2d1b6f04953eca4bcf7c839d44
    Author: Gustavo F. Padovan
    Date: Wed Apr 13 20:23:55 2011 -0300

    @@ -743,6 +743,7 @@ static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char __u
    struct sock *sk = sock->sk;
    struct sock *l2cap_sk;
    struct rfcomm_conninfo cinfo;
    + struct l2cap_conn *conn = l2cap_pi(sk)->chan->conn;
    int len, err = 0;
    u32 opt;

    @@ -787,8 +788,8 @@ static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char __u

    l2cap_sk = rfcomm_pi(sk)->dlc->session->sock->sk;

    - cinfo.hci_handle = l2cap_pi(l2cap_sk)->conn->hcon->handle;
    - memcpy(cinfo.dev_class, l2cap_pi(l2cap_sk)->conn->hcon->dev_class, 3);
    + cinfo.hci_handle = conn->hcon->handle;
    + memcpy(cinfo.dev_class, conn->hcon->dev_class, 3);

    The l2cap_sk got accidentally mixed into the sk (which is RFCOMM) and
    now causing a problem within getsocketopt() system call. To fix this,
    just re-introduce l2cap_sk and make sure the right socket is used for
    the low-level connection details.

    Reported-by: Fabio Rossi
    Reported-by: Janusz Dziedzic
    Tested-by: Janusz Dziedzic
    Signed-off-by: Marcel Holtmann
    Signed-off-by: Johan Hedberg

    Marcel Holtmann
     

18 Oct, 2013

1 commit


14 Oct, 2013

1 commit


19 Sep, 2013

1 commit

  • In the case of blocking sockets we should not proceed with sendmsg() if
    the socket has the BT_SK_SUSPEND flag set. So far the code was only
    ensuring that POLLOUT doesn't get set for non-blocking sockets using
    poll() but there was no code in place to ensure that blocking sockets do
    the right thing when writing to them.

    This patch adds a new bt_sock_wait_ready helper function to sleep in the
    sendmsg call if the BT_SK_SUSPEND flag is set, and wake up as soon as it
    is unset. It also updates the L2CAP and RFCOMM sendmsg callbacks to take
    advantage of this new helper function.

    Signed-off-by: Johan Hedberg
    Acked-by: Marcel Holtmann
    Signed-off-by: Gustavo Padovan

    Johan Hedberg
     

02 May, 2013

1 commit

  • Pull VFS updates from Al Viro,

    Misc cleanups all over the place, mainly wrt /proc interfaces (switch
    create_proc_entry to proc_create(), get rid of the deprecated
    create_proc_read_entry() in favor of using proc_create_data() and
    seq_file etc).

    7kloc removed.

    * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (204 commits)
    don't bother with deferred freeing of fdtables
    proc: Move non-public stuff from linux/proc_fs.h to fs/proc/internal.h
    proc: Make the PROC_I() and PDE() macros internal to procfs
    proc: Supply a function to remove a proc entry by PDE
    take cgroup_open() and cpuset_open() to fs/proc/base.c
    ppc: Clean up scanlog
    ppc: Clean up rtas_flash driver somewhat
    hostap: proc: Use remove_proc_subtree()
    drm: proc: Use remove_proc_subtree()
    drm: proc: Use minor->index to label things, not PDE->name
    drm: Constify drm_proc_list[]
    zoran: Don't print proc_dir_entry data in debug
    reiserfs: Don't access the proc_dir_entry in r_open(), r_start() r_show()
    proc: Supply an accessor for getting the data from a PDE's parent
    airo: Use remove_proc_subtree()
    rtl8192u: Don't need to save device proc dir PDE
    rtl8187se: Use a dir under /proc/net/r8180/
    proc: Add proc_mkdir_data()
    proc: Move some bits from linux/proc_fs.h to linux/{of.h,signal.h,tty.h}
    proc: Move PDE_NET() to fs/proc/proc_net.c
    ...

    Linus Torvalds
     

10 Apr, 2013

2 commits


08 Apr, 2013

1 commit

  • If RFCOMM_DEFER_SETUP is set in the flags, rfcomm_sock_recvmsg() returns
    early with 0 without updating the possibly set msg_namelen member. This,
    in turn, leads to a 128 byte kernel stack leak in net/socket.c.

    Fix this by updating msg_namelen in this case. For all other cases it
    will be handled in bt_sock_stream_recvmsg().

    Cc: Marcel Holtmann
    Cc: Gustavo Padovan
    Cc: Johan Hedberg
    Signed-off-by: Mathias Krause
    Signed-off-by: David S. Miller

    Mathias Krause
     

08 Mar, 2013

1 commit

  • After we successfully registered a socket via bt_sock_register() there is
    no reason to ever check the return code of bt_sock_unregister(). If
    bt_sock_unregister() fails, it means the socket _is_ already unregistered
    so we have what we want, don't we?

    Also, to get bt_sock_unregister() to fail, another part of the kernel has
    to unregister _our_ socket. This is sooo _wrong_ that it will break way
    earlier than when we unregister our socket.

    Signed-off-by: David Herrmann
    Signed-off-by: Gustavo Padovan

    David Herrmann
     

28 Feb, 2013

1 commit

  • I'm not sure why, but the hlist for each entry iterators were conceived

    list_for_each_entry(pos, head, member)

    The hlist ones were greedy and wanted an extra parameter:

    hlist_for_each_entry(tpos, pos, head, member)

    Why did they need an extra pos parameter? I'm not quite sure. Not only
    they don't really need it, it also prevents the iterator from looking
    exactly like the list iterator, which is unfortunate.

    Besides the semantic patch, there was some manual work required:

    - Fix up the actual hlist iterators in linux/list.h
    - Fix up the declaration of other iterators based on the hlist ones.
    - A very small amount of places were using the 'node' parameter, this
    was modified to use 'obj->member' instead.
    - Coccinelle didn't handle the hlist_for_each_entry_safe iterator
    properly, so those had to be fixed up manually.

    The semantic patch which is mostly the work of Peter Senna Tschudin is here:

    @@
    iterator name hlist_for_each_entry, hlist_for_each_entry_continue, hlist_for_each_entry_from, hlist_for_each_entry_rcu, hlist_for_each_entry_rcu_bh, hlist_for_each_entry_continue_rcu_bh, for_each_busy_worker, ax25_uid_for_each, ax25_for_each, inet_bind_bucket_for_each, sctp_for_each_hentry, sk_for_each, sk_for_each_rcu, sk_for_each_from, sk_for_each_safe, sk_for_each_bound, hlist_for_each_entry_safe, hlist_for_each_entry_continue_rcu, nr_neigh_for_each, nr_neigh_for_each_safe, nr_node_for_each, nr_node_for_each_safe, for_each_gfn_indirect_valid_sp, for_each_gfn_sp, for_each_host;

    type T;
    expression a,c,d,e;
    identifier b;
    statement S;
    @@

    -T b;

    [akpm@linux-foundation.org: drop bogus change from net/ipv4/raw.c]
    [akpm@linux-foundation.org: drop bogus hunk from net/ipv6/raw.c]
    [akpm@linux-foundation.org: checkpatch fixes]
    [akpm@linux-foundation.org: fix warnings]
    [akpm@linux-foudnation.org: redo intrusive kvm changes]
    Tested-by: Peter Senna Tschudin
    Acked-by: Paul E. McKenney
    Signed-off-by: Sasha Levin
    Cc: Wu Fengguang
    Cc: Marcelo Tosatti
    Cc: Gleb Natapov
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Sasha Levin
     

04 Dec, 2012

1 commit

  • This patch fixes the following report, it happens when accepting rfcomm
    connections:

    [ 228.165378] =============================================
    [ 228.165378] [ INFO: possible recursive locking detected ]
    [ 228.165378] 3.7.0-rc1-00536-gc1d5dc4 #120 Tainted: G W
    [ 228.165378] ---------------------------------------------
    [ 228.165378] bluetoothd/1341 is trying to acquire lock:
    [ 228.165378] (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+...}, at:
    [] bt_accept_dequeue+0xa0/0x180 [bluetooth]
    [ 228.165378]
    [ 228.165378] but task is already holding lock:
    [ 228.165378] (sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM){+.+...}, at:
    [] rfcomm_sock_accept+0x58/0x2d0 [rfcomm]
    [ 228.165378]
    [ 228.165378] other info that might help us debug this:
    [ 228.165378] Possible unsafe locking scenario:
    [ 228.165378]
    [ 228.165378] CPU0
    [ 228.165378] ----
    [ 228.165378] lock(sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM);
    [ 228.165378] lock(sk_lock-AF_BLUETOOTH-BTPROTO_RFCOMM);
    [ 228.165378]
    [ 228.165378] *** DEADLOCK ***
    [ 228.165378]
    [ 228.165378] May be due to missing lock nesting notation

    Cc: stable@vger.kernel.org
    Signed-off-by: Gustavo Padovan

    Gustavo Padovan
     

20 Oct, 2012

1 commit


28 Sep, 2012

2 commits


23 Aug, 2012

1 commit


16 Aug, 2012

2 commits

  • The RFCOMM code fails to initialize the trailing padding byte of struct
    sockaddr_rc added for alignment. It that for leaks one byte kernel stack
    via the getsockname() syscall. Add an explicit memset(0) before filling
    the structure to avoid the info leak.

    Signed-off-by: Mathias Krause
    Cc: Marcel Holtmann
    Cc: Gustavo Padovan
    Cc: Johan Hedberg
    Signed-off-by: David S. Miller

    Mathias Krause
     
  • The RFCOMM code fails to initialize the key_size member of struct
    bt_security before copying it to userland -- that for leaking one
    byte kernel stack. Initialize key_size with 0 to avoid the info
    leak.

    Signed-off-by: Mathias Krause
    Cc: Marcel Holtmann
    Cc: Gustavo Padovan
    Cc: Johan Hedberg
    Signed-off-by: David S. Miller

    Mathias Krause
     

07 Aug, 2012

1 commit


05 Jun, 2012

1 commit


17 May, 2012

1 commit


29 Mar, 2012

1 commit


15 Feb, 2012

1 commit

  • Since bluetooth uses multiple protocols types, to avoid lockdep
    warnings, we need to use different lockdep classes (one for each
    protocol type).

    This is already done in bt_sock_create but it misses a couple of cases
    when new connections are created. This patch corrects that to fix the
    following warning:

    [ 1864.732366] =======================================================
    [ 1864.733030] [ INFO: possible circular locking dependency detected ]
    [ 1864.733544] 3.0.16-mid3-00007-gc9a0f62 #3
    [ 1864.733883] -------------------------------------------------------
    [ 1864.734408] t.android.btclc/4204 is trying to acquire lock:
    [ 1864.734869] (rfcomm_mutex){+.+.+.}, at: [] rfcomm_dlc_close+0x15/0x30
    [ 1864.735541]
    [ 1864.735549] but task is already holding lock:
    [ 1864.736045] (sk_lock-AF_BLUETOOTH){+.+.+.}, at: [] lock_sock+0xa/0xc
    [ 1864.736732]
    [ 1864.736740] which lock already depends on the new lock.
    [ 1864.736750]
    [ 1864.737428]
    [ 1864.737437] the existing dependency chain (in reverse order) is:
    [ 1864.738016]
    [ 1864.738023] -> #1 (sk_lock-AF_BLUETOOTH){+.+.+.}:
    [ 1864.738549] [] lock_acquire+0x104/0x140
    [ 1864.738977] [] lock_sock_nested+0x58/0x68
    [ 1864.739411] [] l2cap_sock_sendmsg+0x3e/0x76
    [ 1864.739858] [] __sock_sendmsg+0x50/0x59
    [ 1864.740279] [] sock_sendmsg+0x94/0xa8
    [ 1864.740687] [] kernel_sendmsg+0x28/0x37
    [ 1864.741106] [] rfcomm_send_frame+0x30/0x38
    [ 1864.741542] [] rfcomm_send_ua+0x58/0x5a
    [ 1864.741959] [] rfcomm_run+0x441/0xb52
    [ 1864.742365] [] kthread+0x63/0x68
    [ 1864.742742] [] kernel_thread_helper+0x6/0xd
    [ 1864.743187]
    [ 1864.743193] -> #0 (rfcomm_mutex){+.+.+.}:
    [ 1864.743667] [] __lock_acquire+0x988/0xc00
    [ 1864.744100] [] lock_acquire+0x104/0x140
    [ 1864.744519] [] __mutex_lock_common+0x3b/0x33f
    [ 1864.744975] [] mutex_lock_nested+0x2d/0x36
    [ 1864.745412] [] rfcomm_dlc_close+0x15/0x30
    [ 1864.745842] [] __rfcomm_sock_close+0x5f/0x6b
    [ 1864.746288] [] rfcomm_sock_shutdown+0x2f/0x62
    [ 1864.746737] [] sys_socketcall+0x1db/0x422
    [ 1864.747165] [] syscall_call+0x7/0xb

    Signed-off-by: Octavian Purdila
    Acked-by: Marcel Holtmann
    Signed-off-by: Johan Hedberg

    Octavian Purdila
     

03 Jan, 2012

1 commit