10 Sep, 2020

2 commits


11 Aug, 2020

1 commit

  • [ Upstream commit 65550098c1c4db528400c73acf3e46bfa78d9264 ]

    There's a race between rxrpc_sendmsg setting up a call, but then failing to
    send anything on it due to an error, and recvmsg() seeing the call
    completion occur and trying to return the state to the user.

    An assertion fails in rxrpc_recvmsg() because the call has already been
    released from the socket and is about to be released again as recvmsg deals
    with it. (The recvmsg_q queue on the socket holds a ref, so there's no
    problem with use-after-free.)

    We also have to be careful not to end up reporting an error twice, in such
    a way that both returns indicate to userspace that the user ID supplied
    with the call is no longer in use - which could cause the client to
    malfunction if it recycles the user ID fast enough.

    Fix this by the following means:

    (1) When sendmsg() creates a call after the point that the call has been
    successfully added to the socket, don't return any errors through
    sendmsg(), but rather complete the call and let recvmsg() retrieve
    them. Make sendmsg() return 0 at this point. Further calls to
    sendmsg() for that call will fail with ESHUTDOWN.

    Note that at this point, we haven't send any packets yet, so the
    server doesn't yet know about the call.

    (2) If sendmsg() returns an error when it was expected to create a new
    call, it means that the user ID wasn't used.

    (3) Mark the call disconnected before marking it completed to prevent an
    oops in rxrpc_release_call().

    (4) recvmsg() will then retrieve the error and set MSG_EOR to indicate
    that the user ID is no longer known by the kernel.

    An oops like the following is produced:

    kernel BUG at net/rxrpc/recvmsg.c:605!
    ...
    RIP: 0010:rxrpc_recvmsg+0x256/0x5ae
    ...
    Call Trace:
    ? __init_waitqueue_head+0x2f/0x2f
    ____sys_recvmsg+0x8a/0x148
    ? import_iovec+0x69/0x9c
    ? copy_msghdr_from_user+0x5c/0x86
    ___sys_recvmsg+0x72/0xaa
    ? __fget_files+0x22/0x57
    ? __fget_light+0x46/0x51
    ? fdget+0x9/0x1b
    do_recvmmsg+0x15e/0x232
    ? _raw_spin_unlock+0xa/0xb
    ? vtime_delta+0xf/0x25
    __x64_sys_recvmmsg+0x2c/0x2f
    do_syscall_64+0x4c/0x78
    entry_SYSCALL_64_after_hwframe+0x44/0xa9

    Fixes: 357f5ef64628 ("rxrpc: Call rxrpc_release_call() on error in rxrpc_new_client_call()")
    Reported-by: syzbot+b54969381df354936d96@syzkaller.appspotmail.com
    Signed-off-by: David Howells
    Reviewed-by: Marc Dionne
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

01 Aug, 2020

1 commit

  • [ Upstream commit 639f181f0ee20d3249dbc55f740f0167267180f0 ]

    rxrpc_sendmsg() returns EPIPE if there's an outstanding error, such as if
    rxrpc_recvmsg() indicating ENODATA if there's nothing for it to read.

    Change rxrpc_recvmsg() to return EAGAIN instead if there's nothing to read
    as this particular error doesn't get stored in ->sk_err by the networking
    core.

    Also change rxrpc_sendmsg() so that it doesn't fail with delayed receive
    errors (there's no way for it to report which call, if any, the error was
    caused by).

    Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

09 Jul, 2020

2 commits

  • [ Upstream commit 02c28dffb13abbaaedece1e4a6493b48ad3f913a ]

    Commit 2ad6691d988c, which moved the modification of the status annotation
    for a packet in the Tx buffer prior to the retransmission moved the state
    clearance, but managed to lose the bit that set it to UNACK.

    Consequently, if a retransmission occurs, the packet is accidentally
    changed to the ACK state (ie. 0) by masking it off, which means that the
    packet isn't counted towards the tally of newly-ACK'd packets if it gets
    hard-ACK'd. This then prevents the congestion control algorithm from
    recovering properly.

    Fix by reinstating the change of state to UNACK.

    Spotted by the generic/460 xfstest.

    Fixes: 2ad6691d988c ("rxrpc: Fix race between incoming ACK parser and retransmitter")
    Signed-off-by: David Howells
    Signed-off-by: Sasha Levin

    David Howells
     
  • [ Upstream commit 2ad6691d988c0c611362ddc2aad89e0fb50e3261 ]

    There's a race between the retransmission code and the received ACK parser.
    The problem is that the retransmission loop has to drop the lock under
    which it is iterating through the transmission buffer in order to transmit
    a packet, but whilst the lock is dropped, the ACK parser can crank the Tx
    window round and discard the packets from the buffer.

    The retransmission code then updated the annotations for the wrong packet
    and a later retransmission thought it had to retransmit a packet that
    wasn't there, leading to a NULL pointer dereference.

    Fix this by:

    (1) Moving the annotation change to before we drop the lock prior to
    transmission. This means we can't vary the annotation depending on
    the outcome of the transmission, but that's fine - we'll retransmit
    again later if it failed now.

    (2) Skipping the packet if the skb pointer is NULL.

    The following oops was seen:

    BUG: kernel NULL pointer dereference, address: 000000000000002d
    Workqueue: krxrpcd rxrpc_process_call
    RIP: 0010:rxrpc_get_skb+0x14/0x8a
    ...
    Call Trace:
    rxrpc_resend+0x331/0x41e
    ? get_vtime_delta+0x13/0x20
    rxrpc_process_call+0x3c0/0x4ac
    process_one_work+0x18f/0x27f
    worker_thread+0x1a3/0x247
    ? create_worker+0x17d/0x17d
    kthread+0xe6/0xeb
    ? kthread_delayed_work_timer_fn+0x83/0x83
    ret_from_fork+0x1f/0x30

    Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller
    Signed-off-by: Sasha Levin

    David Howells
     

01 Jul, 2020

2 commits

  • [ Upstream commit a2ad7c21ad8cf1ce4ad65e13df1c2a1c29b38ac5 ]

    The handling of the receive window size (rwind) from a received ACK packet
    is not correct. The rxrpc_input_ackinfo() function currently checks the
    current Tx window size against the rwind from the ACK to see if it has
    changed, but then limits the rwind size before storing it in the tx_winsize
    member and, if it increased, wake up the transmitting process. This means
    that if rwind > RXRPC_RXTX_BUFF_SIZE - 1, this path will always be
    followed.

    Fix this by limiting rwind before we compare it to tx_winsize.

    The effect of this can be seen by enabling the rxrpc_rx_rwind_change
    tracepoint.

    Fixes: 702f2ac87a9a ("rxrpc: Wake up the transmitter if Rx window size increases on the peer")
    Signed-off-by: David Howells
    Signed-off-by: Sasha Levin

    David Howells
     
  • [ Upstream commit 0041cd5a50442db6e456b145892a0eaf2dff061f ]

    When preallocated service calls are being discarded, they're passed to
    ->discard_new_call() to have the caller clean up any attached higher-layer
    preallocated pieces before being marked completed. However, the act of
    marking them completed now invokes the call's notification function - which
    causes a problem because that function might assume that the previously
    freed pieces of memory are still there.

    Fix this by setting a dummy notification function on the socket after
    calling ->discard_new_call().

    This results in the following kasan message when the kafs module is
    removed.

    ==================================================================
    BUG: KASAN: use-after-free in afs_wake_up_async_call+0x6aa/0x770 fs/afs/rxrpc.c:707
    Write of size 1 at addr ffff8880946c39e4 by task kworker/u4:1/21

    CPU: 0 PID: 21 Comm: kworker/u4:1 Not tainted 5.8.0-rc1-syzkaller #0
    Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
    Workqueue: netns cleanup_net
    Call Trace:
    __dump_stack lib/dump_stack.c:77 [inline]
    dump_stack+0x18f/0x20d lib/dump_stack.c:118
    print_address_description.constprop.0.cold+0xd3/0x413 mm/kasan/report.c:383
    __kasan_report mm/kasan/report.c:513 [inline]
    kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
    afs_wake_up_async_call+0x6aa/0x770 fs/afs/rxrpc.c:707
    rxrpc_notify_socket+0x1db/0x5d0 net/rxrpc/recvmsg.c:40
    __rxrpc_set_call_completion.part.0+0x172/0x410 net/rxrpc/recvmsg.c:76
    __rxrpc_call_completed net/rxrpc/recvmsg.c:112 [inline]
    rxrpc_call_completed+0xca/0xf0 net/rxrpc/recvmsg.c:111
    rxrpc_discard_prealloc+0x781/0xab0 net/rxrpc/call_accept.c:233
    rxrpc_listen+0x147/0x360 net/rxrpc/af_rxrpc.c:245
    afs_close_socket+0x95/0x320 fs/afs/rxrpc.c:110
    afs_net_exit+0x1bc/0x310 fs/afs/main.c:155
    ops_exit_list.isra.0+0xa8/0x150 net/core/net_namespace.c:186
    cleanup_net+0x511/0xa50 net/core/net_namespace.c:603
    process_one_work+0x965/0x1690 kernel/workqueue.c:2269
    worker_thread+0x96/0xe10 kernel/workqueue.c:2415
    kthread+0x3b5/0x4a0 kernel/kthread.c:291
    ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293

    Allocated by task 6820:
    save_stack+0x1b/0x40 mm/kasan/common.c:48
    set_track mm/kasan/common.c:56 [inline]
    __kasan_kmalloc mm/kasan/common.c:494 [inline]
    __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:467
    kmem_cache_alloc_trace+0x153/0x7d0 mm/slab.c:3551
    kmalloc include/linux/slab.h:555 [inline]
    kzalloc include/linux/slab.h:669 [inline]
    afs_alloc_call+0x55/0x630 fs/afs/rxrpc.c:141
    afs_charge_preallocation+0xe9/0x2d0 fs/afs/rxrpc.c:757
    afs_open_socket+0x292/0x360 fs/afs/rxrpc.c:92
    afs_net_init+0xa6c/0xe30 fs/afs/main.c:125
    ops_init+0xaf/0x420 net/core/net_namespace.c:151
    setup_net+0x2de/0x860 net/core/net_namespace.c:341
    copy_net_ns+0x293/0x590 net/core/net_namespace.c:482
    create_new_namespaces+0x3fb/0xb30 kernel/nsproxy.c:110
    unshare_nsproxy_namespaces+0xbd/0x1f0 kernel/nsproxy.c:231
    ksys_unshare+0x43d/0x8e0 kernel/fork.c:2983
    __do_sys_unshare kernel/fork.c:3051 [inline]
    __se_sys_unshare kernel/fork.c:3049 [inline]
    __x64_sys_unshare+0x2d/0x40 kernel/fork.c:3049
    do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:359
    entry_SYSCALL_64_after_hwframe+0x44/0xa9

    Freed by task 21:
    save_stack+0x1b/0x40 mm/kasan/common.c:48
    set_track mm/kasan/common.c:56 [inline]
    kasan_set_free_info mm/kasan/common.c:316 [inline]
    __kasan_slab_free+0xf7/0x140 mm/kasan/common.c:455
    __cache_free mm/slab.c:3426 [inline]
    kfree+0x109/0x2b0 mm/slab.c:3757
    afs_put_call+0x585/0xa40 fs/afs/rxrpc.c:190
    rxrpc_discard_prealloc+0x764/0xab0 net/rxrpc/call_accept.c:230
    rxrpc_listen+0x147/0x360 net/rxrpc/af_rxrpc.c:245
    afs_close_socket+0x95/0x320 fs/afs/rxrpc.c:110
    afs_net_exit+0x1bc/0x310 fs/afs/main.c:155
    ops_exit_list.isra.0+0xa8/0x150 net/core/net_namespace.c:186
    cleanup_net+0x511/0xa50 net/core/net_namespace.c:603
    process_one_work+0x965/0x1690 kernel/workqueue.c:2269
    worker_thread+0x96/0xe10 kernel/workqueue.c:2415
    kthread+0x3b5/0x4a0 kernel/kthread.c:291
    ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293

    The buggy address belongs to the object at ffff8880946c3800
    which belongs to the cache kmalloc-1k of size 1024
    The buggy address is located 484 bytes inside of
    1024-byte region [ffff8880946c3800, ffff8880946c3c00)
    The buggy address belongs to the page:
    page:ffffea000251b0c0 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0
    flags: 0xfffe0000000200(slab)
    raw: 00fffe0000000200 ffffea0002546508 ffffea00024fa248 ffff8880aa000c40
    raw: 0000000000000000 ffff8880946c3000 0000000100000002 0000000000000000
    page dumped because: kasan: bad access detected

    Memory state around the buggy address:
    ffff8880946c3880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ffff8880946c3900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    >ffff8880946c3980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ^
    ffff8880946c3a00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ffff8880946c3a80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ==================================================================

    Reported-by: syzbot+d3eccef36ddbd02713e9@syzkaller.appspotmail.com
    Fixes: 5ac0d62226a0 ("rxrpc: Fix missing notification")
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

24 Jun, 2020

1 commit


27 May, 2020

4 commits

  • [ Upstream commit 441fdee1eaf050ef0040bde0d7af075c1c6a6d8b ]

    The Rx protocol has a "previousPacket" field in it that is not handled in
    the same way by all protocol implementations. Sometimes it contains the
    serial number of the last DATA packet received, sometimes the sequence
    number of the last DATA packet received and sometimes the highest sequence
    number so far received.

    AF_RXRPC is using this to weed out ACKs that are out of date (it's possible
    for ACK packets to get reordered on the wire), but this does not work with
    OpenAFS which will just stick the sequence number of the last packet seen
    into previousPacket.

    The issue being seen is that big AFS FS.StoreData RPC (eg. of ~256MiB) are
    timing out when partly sent. A trace was captured, with an additional
    tracepoint to show ACKs being discarded in rxrpc_input_ack(). Here's an
    excerpt showing the problem.

    52873.203230: rxrpc_tx_data: c=000004ae DATA ed1a3584:00000002 0002449c q=00024499 fl=09

    A DATA packet with sequence number 00024499 has been transmitted (the "q="
    field).

    ...
    52873.243296: rxrpc_rx_ack: c=000004ae 00012a2b DLY r=00024499 f=00024497 p=00024496 n=0
    52873.243376: rxrpc_rx_ack: c=000004ae 00012a2c IDL r=0002449b f=00024499 p=00024498 n=0
    52873.243383: rxrpc_rx_ack: c=000004ae 00012a2d OOS r=0002449d f=00024499 p=0002449a n=2

    The Out-Of-Sequence ACK indicates that the server didn't see DATA sequence
    number 00024499, but did see seq 0002449a (previousPacket, shown as "p=",
    skipped the number, but firstPacket, "f=", which shows the bottom of the
    window is set at that point).

    52873.252663: rxrpc_retransmit: c=000004ae q=24499 a=02 xp=14581537
    52873.252664: rxrpc_tx_data: c=000004ae DATA ed1a3584:00000002 000244bc q=00024499 fl=0b *RETRANS*

    The packet has been retransmitted. Retransmission recurs until the peer
    says it got the packet.

    52873.271013: rxrpc_rx_ack: c=000004ae 00012a31 OOS r=000244a1 f=00024499 p=0002449e n=6

    More OOS ACKs indicate that the other packets that are already in the
    transmission pipeline are being received. The specific-ACK list is up to 6
    ACKs and NAKs.

    ...
    52873.284792: rxrpc_rx_ack: c=000004ae 00012a49 OOS r=000244b9 f=00024499 p=000244b6 n=30
    52873.284802: rxrpc_retransmit: c=000004ae q=24499 a=0a xp=63505500
    52873.284804: rxrpc_tx_data: c=000004ae DATA ed1a3584:00000002 000244c2 q=00024499 fl=0b *RETRANS*
    52873.287468: rxrpc_rx_ack: c=000004ae 00012a4a OOS r=000244ba f=00024499 p=000244b7 n=31
    52873.287478: rxrpc_rx_ack: c=000004ae 00012a4b OOS r=000244bb f=00024499 p=000244b8 n=32

    At this point, the server's receive window is full (n=32) with presumably 1
    NAK'd packet and 31 ACK'd packets. We can't transmit any more packets.

    52873.287488: rxrpc_retransmit: c=000004ae q=24499 a=0a xp=61327980
    52873.287489: rxrpc_tx_data: c=000004ae DATA ed1a3584:00000002 000244c3 q=00024499 fl=0b *RETRANS*
    52873.293850: rxrpc_rx_ack: c=000004ae 00012a4c DLY r=000244bc f=000244a0 p=00024499 n=25

    And now we've received an ACK indicating that a DATA retransmission was
    received. 7 packets have been processed (the occupied part of the window
    moved, as indicated by f= and n=).

    52873.293853: rxrpc_rx_discard_ack: c=000004ae r=00012a4c 000244a0
    Signed-off-by: David Howells
    Signed-off-by: Sasha Levin

    David Howells
     
  • [ Upstream commit d1f129470e6cb79b8b97fecd12689f6eb49e27fe ]

    Add a tracepoint to track received ACKs that are discarded due to being
    outside of the Tx window.

    Signed-off-by: David Howells
    Signed-off-by: Sasha Levin

    David Howells
     
  • commit f45d01f4f30b53c3a0a1c6c1c154acb7ff74ab9f upstream.

    A ticket was not released after a call of the function
    "rxkad_decrypt_ticket" failed. Thus replace the jump target
    "temporary_error_free_resp" by "temporary_error_free_ticket".

    Fixes: 8c2f826dc3631 ("rxrpc: Don't put crypto buffers on the stack")
    Signed-off-by: Qiushi Wu
    Signed-off-by: David Howells
    cc: Markus Elfring
    Signed-off-by: Greg Kroah-Hartman

    Qiushi Wu
     
  • commit c410bf01933e5e09d142c66c3df9ad470a7eec13 upstream.

    rxrpc currently uses a fixed 4s retransmission timeout until the RTT is
    sufficiently sampled. This can cause problems with some fileservers with
    calls to the cache manager in the afs filesystem being dropped from the
    fileserver because a packet goes missing and the retransmission timeout is
    greater than the call expiry timeout.

    Fix this by:

    (1) Copying the RTT/RTO calculation code from Linux's TCP implementation
    and altering it to fit rxrpc.

    (2) Altering the various users of the RTT to make use of the new SRTT
    value.

    (3) Replacing the use of rxrpc_resend_timeout to use the calculated RTO
    value instead (which is needed in jiffies), along with a backoff.

    Notes:

    (1) rxrpc provides RTT samples by matching the serial numbers on outgoing
    DATA packets that have the RXRPC_REQUEST_ACK set and PING ACK packets
    against the reference serial number in incoming REQUESTED ACK and
    PING-RESPONSE ACK packets.

    (2) Each packet that is transmitted on an rxrpc connection gets a new
    per-connection serial number, even for retransmissions, so an ACK can
    be cross-referenced to a specific trigger packet. This allows RTT
    information to be drawn from retransmitted DATA packets also.

    (3) rxrpc maintains the RTT/RTO state on the rxrpc_peer record rather than
    on an rxrpc_call because many RPC calls won't live long enough to
    generate more than one sample.

    (4) The calculated SRTT value is in units of 8ths of a microsecond rather
    than nanoseconds.

    The (S)RTT and RTO values are displayed in /proc/net/rxrpc/peers.

    Fixes: 17926a79320a ([AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both"")
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

02 May, 2020

1 commit

  • commit 0e631eee17dcea576ab922fa70e4fdbd596ee452 upstream.

    Fix the DATA packet transmission to disable nofrag for UDPv4 on an AF_INET6
    socket as well as UDPv6 when trying to transmit fragmentably.

    Without this, packets filled to the normal size used by the kernel AFS
    client of 1412 bytes be rejected by udp_sendmsg() with EMSGSIZE
    immediately. The ->sk_error_report() notification hook is called, but
    rxrpc doesn't generate a trace for it.

    This is a temporary fix; a more permanent solution needs to involve
    changing the size of the packets being filled in accordance with the MTU,
    which isn't currently done in AF_RXRPC. The reason for not doing so was
    that, barring the last packet in an rx jumbo packet, jumbos can only be
    assembled out of 1412-byte packets - and the plan was to construct jumbos
    on the fly at transmission time.

    Also, there's no point turning on IPV6_MTU_DISCOVER, since IPv6 has to
    engage in this anyway since fragmentation is only done by the sender. We
    can then condense the switch-statement in rxrpc_send_data_packet().

    Fixes: 75b54cb57ca3 ("rxrpc: Add IPv6 support")
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

23 Apr, 2020

1 commit

  • commit d3ec10aa95819bff18a0d936b18884c7816d0914 upstream.

    A lockdep circular locking dependency report was seen when running a
    keyutils test:

    [12537.027242] ======================================================
    [12537.059309] WARNING: possible circular locking dependency detected
    [12537.088148] 4.18.0-147.7.1.el8_1.x86_64+debug #1 Tainted: G OE --------- - -
    [12537.125253] ------------------------------------------------------
    [12537.153189] keyctl/25598 is trying to acquire lock:
    [12537.175087] 000000007c39f96c (&mm->mmap_sem){++++}, at: __might_fault+0xc4/0x1b0
    [12537.208365]
    [12537.208365] but task is already holding lock:
    [12537.234507] 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220
    [12537.270476]
    [12537.270476] which lock already depends on the new lock.
    [12537.270476]
    [12537.307209]
    [12537.307209] the existing dependency chain (in reverse order) is:
    [12537.340754]
    [12537.340754] -> #3 (&type->lock_class){++++}:
    [12537.367434] down_write+0x4d/0x110
    [12537.385202] __key_link_begin+0x87/0x280
    [12537.405232] request_key_and_link+0x483/0xf70
    [12537.427221] request_key+0x3c/0x80
    [12537.444839] dns_query+0x1db/0x5a5 [dns_resolver]
    [12537.468445] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs]
    [12537.496731] cifs_reconnect+0xe04/0x2500 [cifs]
    [12537.519418] cifs_readv_from_socket+0x461/0x690 [cifs]
    [12537.546263] cifs_read_from_socket+0xa0/0xe0 [cifs]
    [12537.573551] cifs_demultiplex_thread+0x311/0x2db0 [cifs]
    [12537.601045] kthread+0x30c/0x3d0
    [12537.617906] ret_from_fork+0x3a/0x50
    [12537.636225]
    [12537.636225] -> #2 (root_key_user.cons_lock){+.+.}:
    [12537.664525] __mutex_lock+0x105/0x11f0
    [12537.683734] request_key_and_link+0x35a/0xf70
    [12537.705640] request_key+0x3c/0x80
    [12537.723304] dns_query+0x1db/0x5a5 [dns_resolver]
    [12537.746773] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs]
    [12537.775607] cifs_reconnect+0xe04/0x2500 [cifs]
    [12537.798322] cifs_readv_from_socket+0x461/0x690 [cifs]
    [12537.823369] cifs_read_from_socket+0xa0/0xe0 [cifs]
    [12537.847262] cifs_demultiplex_thread+0x311/0x2db0 [cifs]
    [12537.873477] kthread+0x30c/0x3d0
    [12537.890281] ret_from_fork+0x3a/0x50
    [12537.908649]
    [12537.908649] -> #1 (&tcp_ses->srv_mutex){+.+.}:
    [12537.935225] __mutex_lock+0x105/0x11f0
    [12537.954450] cifs_call_async+0x102/0x7f0 [cifs]
    [12537.977250] smb2_async_readv+0x6c3/0xc90 [cifs]
    [12538.000659] cifs_readpages+0x120a/0x1e50 [cifs]
    [12538.023920] read_pages+0xf5/0x560
    [12538.041583] __do_page_cache_readahead+0x41d/0x4b0
    [12538.067047] ondemand_readahead+0x44c/0xc10
    [12538.092069] filemap_fault+0xec1/0x1830
    [12538.111637] __do_fault+0x82/0x260
    [12538.129216] do_fault+0x419/0xfb0
    [12538.146390] __handle_mm_fault+0x862/0xdf0
    [12538.167408] handle_mm_fault+0x154/0x550
    [12538.187401] __do_page_fault+0x42f/0xa60
    [12538.207395] do_page_fault+0x38/0x5e0
    [12538.225777] page_fault+0x1e/0x30
    [12538.243010]
    [12538.243010] -> #0 (&mm->mmap_sem){++++}:
    [12538.267875] lock_acquire+0x14c/0x420
    [12538.286848] __might_fault+0x119/0x1b0
    [12538.306006] keyring_read_iterator+0x7e/0x170
    [12538.327936] assoc_array_subtree_iterate+0x97/0x280
    [12538.352154] keyring_read+0xe9/0x110
    [12538.370558] keyctl_read_key+0x1b9/0x220
    [12538.391470] do_syscall_64+0xa5/0x4b0
    [12538.410511] entry_SYSCALL_64_after_hwframe+0x6a/0xdf
    [12538.435535]
    [12538.435535] other info that might help us debug this:
    [12538.435535]
    [12538.472829] Chain exists of:
    [12538.472829] &mm->mmap_sem --> root_key_user.cons_lock --> &type->lock_class
    [12538.472829]
    [12538.524820] Possible unsafe locking scenario:
    [12538.524820]
    [12538.551431] CPU0 CPU1
    [12538.572654] ---- ----
    [12538.595865] lock(&type->lock_class);
    [12538.613737] lock(root_key_user.cons_lock);
    [12538.644234] lock(&type->lock_class);
    [12538.672410] lock(&mm->mmap_sem);
    [12538.687758]
    [12538.687758] *** DEADLOCK ***
    [12538.687758]
    [12538.714455] 1 lock held by keyctl/25598:
    [12538.732097] #0: 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220
    [12538.770573]
    [12538.770573] stack backtrace:
    [12538.790136] CPU: 2 PID: 25598 Comm: keyctl Kdump: loaded Tainted: G
    [12538.844855] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS P89 12/27/2015
    [12538.881963] Call Trace:
    [12538.892897] dump_stack+0x9a/0xf0
    [12538.907908] print_circular_bug.isra.25.cold.50+0x1bc/0x279
    [12538.932891] ? save_trace+0xd6/0x250
    [12538.948979] check_prev_add.constprop.32+0xc36/0x14f0
    [12538.971643] ? keyring_compare_object+0x104/0x190
    [12538.992738] ? check_usage+0x550/0x550
    [12539.009845] ? sched_clock+0x5/0x10
    [12539.025484] ? sched_clock_cpu+0x18/0x1e0
    [12539.043555] __lock_acquire+0x1f12/0x38d0
    [12539.061551] ? trace_hardirqs_on+0x10/0x10
    [12539.080554] lock_acquire+0x14c/0x420
    [12539.100330] ? __might_fault+0xc4/0x1b0
    [12539.119079] __might_fault+0x119/0x1b0
    [12539.135869] ? __might_fault+0xc4/0x1b0
    [12539.153234] keyring_read_iterator+0x7e/0x170
    [12539.172787] ? keyring_read+0x110/0x110
    [12539.190059] assoc_array_subtree_iterate+0x97/0x280
    [12539.211526] keyring_read+0xe9/0x110
    [12539.227561] ? keyring_gc_check_iterator+0xc0/0xc0
    [12539.249076] keyctl_read_key+0x1b9/0x220
    [12539.266660] do_syscall_64+0xa5/0x4b0
    [12539.283091] entry_SYSCALL_64_after_hwframe+0x6a/0xdf

    One way to prevent this deadlock scenario from happening is to not
    allow writing to userspace while holding the key semaphore. Instead,
    an internal buffer is allocated for getting the keys out from the
    read method first before copying them out to userspace without holding
    the lock.

    That requires taking out the __user modifier from all the relevant
    read methods as well as additional changes to not use any userspace
    write helpers. That is,

    1) The put_user() call is replaced by a direct copy.
    2) The copy_to_user() call is replaced by memcpy().
    3) All the fault handling code is removed.

    Compiling on a x86-64 system, the size of the rxrpc_read() function is
    reduced from 3795 bytes to 2384 bytes with this patch.

    Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2")
    Reviewed-by: Jarkko Sakkinen
    Signed-off-by: Waiman Long
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    Waiman Long
     

17 Apr, 2020

2 commits

  • [ Upstream commit e138aa7d3271ac1b0690ae2c9b04d51468dce1d6 ]

    Fix the interruptibility of kernel-initiated client calls so that they're
    either only interruptible when they're waiting for a call slot to come
    available or they're not interruptible at all. Either way, they're not
    interruptible during transmission.

    This should help prevent StoreData calls from being interrupted when
    writeback is in progress. It doesn't, however, handle interruption during
    the receive phase.

    Userspace-initiated calls are still interruptable. After the signal has
    been handled, sendmsg() will return the amount of data copied out of the
    buffer and userspace can perform another sendmsg() call to continue
    transmission.

    Fixes: bc5e3a546d55 ("rxrpc: Use MSG_WAITALL to tell sendmsg() to temporarily ignore signals")
    Signed-off-by: David Howells
    Signed-off-by: Sasha Levin

    David Howells
     
  • [ Upstream commit 158fe6665389964a1de212818b4a5c52b7f7aff4 ]

    Abstract out the calculation of there being sufficient Tx buffer space.
    This is reproduced several times in the rxrpc sendmsg code.

    Signed-off-by: David Howells
    Signed-off-by: Sasha Levin

    David Howells
     

08 Apr, 2020

1 commit

  • commit 498b577660f08cef5d9e78e0ed6dcd4c0939e98c upstream.

    Fix the handling of sendmsg() with MSG_WAITALL for userspace to round the
    timeout for when a signal occurs up to at least two jiffies as a 1 jiffy
    timeout may end up being effectively 0 if jiffies wraps at the wrong time.

    Fixes: bc5e3a546d55 ("rxrpc: Use MSG_WAITALL to tell sendmsg() to temporarily ignore signals")
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

01 Apr, 2020

1 commit

  • commit 7d7587db0d7fd1138f2afcffdc46a8e15630b944 upstream.

    Fix the handling of signals in client rxrpc calls made by the afs
    filesystem. Ignore signals completely, leaving call abandonment or
    connection loss to be detected by timeouts inside AF_RXRPC.

    Allowing a filesystem call to be interrupted after the entire request has
    been transmitted and an abort sent means that the server may or may not
    have done the action - and we don't know. It may even be worse than that
    for older servers.

    Fixes: bc5e3a546d55 ("rxrpc: Use MSG_WAITALL to tell sendmsg() to temporarily ignore signals")
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

29 Feb, 2020

1 commit

  • commit 963485d436ccc2810177a7b08af22336ec2af67b upstream.

    rxrpc_rcu_destroy_call(), which is called as an RCU callback to clean up a
    put call, calls rxrpc_put_connection() which, deep in its bowels, takes a
    number of spinlocks in a non-BH-safe way, including rxrpc_conn_id_lock and
    local->client_conns_lock. RCU callbacks, however, are normally called from
    softirq context, which can cause lockdep to notice the locking
    inconsistency.

    To get lockdep to detect this, it's necessary to have the connection
    cleaned up on the put at the end of the last of its calls, though normally
    the clean up is deferred. This can be induced, however, by starting a call
    on an AF_RXRPC socket and then closing the socket without reading the
    reply.

    Fix this by having rxrpc_rcu_destroy_call() punt the destruction to a
    workqueue if in softirq-mode and defer the destruction to process context.

    Note that another way to fix this could be to add a bunch of bh-disable
    annotations to the spinlocks concerned - and there might be more than just
    those two - but that means spending more time with BHs disabled.

    Note also that some of these places were covered by bh-disable spinlocks
    belonging to the rxrpc_transport object, but these got removed without the
    _bh annotation being retained on the next lock in.

    Fixes: 999b69f89241 ("rxrpc: Kill the client connection bundle concept")
    Reported-by: syzbot+d82f3ac8d87e7ccbb2c9@syzkaller.appspotmail.com
    Reported-by: syzbot+3f1fd6b8cbf8702d134e@syzkaller.appspotmail.com
    Signed-off-by: David Howells
    cc: Hillf Danton
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

11 Feb, 2020

5 commits

  • [ Upstream commit b39a934ec72fa2b5a74123891f25273a38378b90 ]

    The recent patch that substituted a flag on an rxrpc_call for the
    connection pointer being NULL as an indication that a call was disconnected
    puts the set_bit in the wrong place for service calls. This is only a
    problem if a call is implicitly terminated by a new call coming in on the
    same connection channel instead of a terminating ACK packet.

    In such a case, rxrpc_input_implicit_end_call() calls
    __rxrpc_disconnect_call(), which is now (incorrectly) setting the
    disconnection bit, meaning that when rxrpc_release_call() is later called,
    it doesn't call rxrpc_disconnect_call() and so the call isn't removed from
    the peer's error distribution list and the list gets corrupted.

    KASAN finds the issue as an access after release on a call, but the
    position at which it occurs is confusing as it appears to be related to a
    different call (the call site is where the latter call is being removed
    from the error distribution list and either the next or pprev pointer
    points to a previously released call).

    Fix this by moving the setting of the flag from __rxrpc_disconnect_call()
    to rxrpc_disconnect_call() in the same place that the connection pointer
    was being cleared.

    Fixes: 5273a191dca6 ("rxrpc: Fix NULL pointer deref due to call->conn being cleared on disconnect")
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller
    Signed-off-by: Sasha Levin

    David Howells
     
  • [ Upstream commit 5273a191dca65a675dc0bcf3909e59c6933e2831 ]

    When a call is disconnected, the connection pointer from the call is
    cleared to make sure it isn't used again and to prevent further attempted
    transmission for the call. Unfortunately, there might be a daemon trying
    to use it at the same time to transmit a packet.

    Fix this by keeping call->conn set, but setting a flag on the call to
    indicate disconnection instead.

    Remove also the bits in the transmission functions where the conn pointer is
    checked and a ref taken under spinlock as this is now redundant.

    Fixes: 8d94aa381dab ("rxrpc: Calls shouldn't hold socket refs")
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     
  • [ Upstream commit 04d36d748fac349b068ef621611f454010054c58 ]

    The introduction of a split between the reference count on rxrpc_local
    objects and the usage count didn't quite go far enough. A number of kernel
    work items need to make use of the socket to perform transmission. These
    also need to get an active count on the local object to prevent the socket
    from being closed.

    Fix this by getting the active count in those places.

    Also split out the raw active count get/put functions as these places tend
    to hold refs on the rxrpc_local object already, so getting and putting an
    extra object ref is just a waste of time.

    The problem can lead to symptoms like:

    BUG: kernel NULL pointer dereference, address: 0000000000000018
    ..
    CPU: 2 PID: 818 Comm: kworker/u9:0 Not tainted 5.5.0-fscache+ #51
    ...
    RIP: 0010:selinux_socket_sendmsg+0x5/0x13
    ...
    Call Trace:
    security_socket_sendmsg+0x2c/0x3e
    sock_sendmsg+0x1a/0x46
    rxrpc_send_keepalive+0x131/0x1ae
    rxrpc_peer_keepalive_worker+0x219/0x34b
    process_one_work+0x18e/0x271
    worker_thread+0x1a3/0x247
    kthread+0xe6/0xeb
    ret_from_fork+0x1f/0x30

    Fixes: 730c5fd42c1e ("rxrpc: Fix local endpoint refcounting")
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     
  • [ Upstream commit f71dbf2fb28489a79bde0dca1c8adfb9cdb20a6b ]

    In rxrpc_input_data(), rxrpc_notify_socket() is called if the base sequence
    number of the packet is immediately following the hard-ack point at the end
    of the function. However, this isn't sufficient, since the recvmsg side
    may have been advancing the window and then overrun the position in which
    we're adding - at which point rx_hard_ack >= seq0 and no notification is
    generated.

    Fix this by always generating a notification at the end of the input
    function.

    Without this, a long call may stall, possibly indefinitely.

    Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     
  • [ Upstream commit fac20b9e738523fc884ee3ea5be360a321cd8bad ]

    Fix rxrpc_put_local() to not access local->debug_id after calling
    atomic_dec_return() as, unless that returned n==0, we no longer have the
    right to access the object.

    Fixes: 06d9532fa6b3 ("rxrpc: Fix read-after-free in rxrpc_queue_local()")
    Signed-off-by: David Howells
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

01 Feb, 2020

1 commit

  • [ Upstream commit 122d74fac84204b9a98263636f6f9a3b2e665639 ]

    The subpacket scanning loop in rxrpc_receive_data() references the
    subpacket count in the private data part of the sk_buff in the loop
    termination condition. However, when the final subpacket is pasted into
    the ring buffer, the function is no longer has a ref on the sk_buff and
    should not be looking at sp->* any more. This point is actually marked in
    the code when skb is cleared (but sp is not - which is an error).

    Fix this by caching sp->nr_subpackets in a local variable and using that
    instead.

    Also clear 'sp' to catch accesses after that point.

    This can show up as an oops in rxrpc_get_skb() if sp->nr_subpackets gets
    trashed by the sk_buff getting freed and reused in the meantime.

    Fixes: e2de6c404898 ("rxrpc: Use info in skbuff instead of reparsing a jumbo packet")
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    David Howells
     

18 Jan, 2020

3 commits

  • [ Upstream commit 063c60d39180cec7c9317f5acfc3071f8fecd705 ]

    Fix rxrpc_new_incoming_call() to check that we have a suitable service key
    available for the combination of service ID and security class of a new
    incoming call - and to reject calls for which we don't.

    This causes an assertion like the following to appear:

    rxrpc: Assertion failed - 6(0x6) == 12(0xc) is false
    kernel BUG at net/rxrpc/call_object.c:456!

    Where call->state is RXRPC_CALL_SERVER_SECURING (6) rather than
    RXRPC_CALL_COMPLETE (12).

    Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
    Reported-by: Marc Dionne
    Signed-off-by: David Howells
    Signed-off-by: Sasha Levin

    David Howells
     
  • [ Upstream commit 13b7955a0252e15265386b229b814152f109b234 ]

    Standard kernel mutexes cannot be used in any way from interrupt or softirq
    context, so the user_mutex which manages access to a call cannot be a mutex
    since on a new call the mutex must start off locked and be unlocked within
    the softirq handler to prevent userspace interfering with a call we're
    setting up.

    Commit a0855d24fc22d49cdc25664fb224caee16998683 ("locking/mutex: Complain
    upon mutex API misuse in IRQ contexts") causes big warnings to be splashed
    in dmesg for each a new call that comes in from the server. Whilst it
    *seems* like it should be okay, since the accept path uses trylock, there
    are issues with PI boosting and marking the wrong task as the owner.

    Fix this by not taking the mutex in the softirq path at all. It's not
    obvious that there should be any need for it as the state is set before the
    first notification is generated for the new call.

    There's also no particular reason why the link-assessing ping should be
    triggered inside the mutex. It's not actually transmitted there anyway,
    but rather it has to be deferred to a workqueue.

    Further, I don't think that there's any particular reason that the socket
    notification needs to be done from within rx->incoming_lock, so the amount
    of time that lock is held can be shortened too and the ping prepared before
    the new call notification is sent.

    Fixes: 540b1c48c37a ("rxrpc: Fix deadlock between call creation and sendmsg/recvmsg")
    Signed-off-by: David Howells
    cc: Peter Zijlstra (Intel)
    cc: Ingo Molnar
    cc: Will Deacon
    cc: Davidlohr Bueso
    Signed-off-by: Sasha Levin

    David Howells
     
  • [ Upstream commit f33121cbe91973a08e68e4bde8c3f7e6e4e351c1 ]

    Move the unlock and the ping transmission for a new incoming call into
    rxrpc_new_incoming_call() rather than doing it in the caller. This makes
    it clearer to see what's going on.

    Suggested-by: Peter Zijlstra
    Signed-off-by: David Howells
    Acked-by: Peter Zijlstra (Intel)
    cc: Ingo Molnar
    cc: Will Deacon
    cc: Davidlohr Bueso
    Signed-off-by: Sasha Levin

    David Howells
     

01 Nov, 2019

1 commit

  • When rxrpc_recvmsg_data() sets the return value to 1 because it's drained
    all the data for the last packet, it checks the last-packet flag on the
    whole packet - but this is wrong, since the last-packet flag is only set on
    the final subpacket of the last jumbo packet. This means that a call that
    receives its last packet in a jumbo packet won't complete properly.

    Fix this by having rxrpc_locate_data() determine the last-packet state of
    the subpacket it's looking at and passing that back to the caller rather
    than having the caller look in the packet header. The caller then needs to
    cache this in the rxrpc_call struct as rxrpc_locate_data() isn't then
    called again for this packet.

    Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
    Fixes: e2de6c404898 ("rxrpc: Use info in skbuff instead of reparsing a jumbo packet")
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller

    David Howells
     

17 Oct, 2019

1 commit

  • We need to extend the rcu_read_lock() section in rxrpc_error_report()
    and use rcu_dereference_sk_user_data() instead of plain access
    to sk->sk_user_data to make sure all rules are respected.

    The compiler wont reload sk->sk_user_data at will, and RCU rules
    prevent memory beeing freed too soon.

    Fixes: f0308fb07080 ("rxrpc: Fix possible NULL pointer access in ICMP handling")
    Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
    Signed-off-by: Eric Dumazet
    Cc: David Howells
    Signed-off-by: David S. Miller

    Eric Dumazet
     

12 Oct, 2019

1 commit

  • If an ICMP packet comes in on the UDP socket backing an AF_RXRPC socket as
    the UDP socket is being shut down, rxrpc_error_report() may get called to
    deal with it after sk_user_data on the UDP socket has been cleared, leading
    to a NULL pointer access when this local endpoint record gets accessed.

    Fix this by just returning immediately if sk_user_data was NULL.

    The oops looks like the following:

    #PF: supervisor read access in kernel mode
    #PF: error_code(0x0000) - not-present page
    ...
    RIP: 0010:rxrpc_error_report+0x1bd/0x6a9
    ...
    Call Trace:
    ? sock_queue_err_skb+0xbd/0xde
    ? __udp4_lib_err+0x313/0x34d
    __udp4_lib_err+0x313/0x34d
    icmp_unreach+0x1ee/0x207
    icmp_rcv+0x25b/0x28f
    ip_protocol_deliver_rcu+0x95/0x10e
    ip_local_deliver+0xe9/0x148
    __netif_receive_skb_one_core+0x52/0x6e
    process_backlog+0xdc/0x177
    net_rx_action+0xf9/0x270
    __do_softirq+0x1b6/0x39a
    ? smpboot_register_percpu_thread+0xce/0xce
    run_ksoftirqd+0x1d/0x42
    smpboot_thread_fn+0x19e/0x1b3
    kthread+0xf1/0xf6
    ? kthread_delayed_work_timer_fn+0x83/0x83
    ret_from_fork+0x24/0x30

    Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
    Reported-by: syzbot+611164843bd48cc2190c@syzkaller.appspotmail.com
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller

    David Howells
     

07 Oct, 2019

6 commits

  • Fix the cleanup of the crypto state on a call after the call has been
    disconnected. As the call has been disconnected, its connection ref has
    been discarded and so we can't go through that to get to the security ops
    table.

    Fix this by caching the security ops pointer in the rxrpc_call struct and
    using that when freeing the call security state. Also use this in other
    places we're dealing with call-specific security.

    The symptoms look like:

    BUG: KASAN: use-after-free in rxrpc_release_call+0xb2d/0xb60
    net/rxrpc/call_object.c:481
    Read of size 8 at addr ffff888062ffeb50 by task syz-executor.5/4764

    Fixes: 1db88c534371 ("rxrpc: Fix -Wframe-larger-than= warnings from on-stack crypto")
    Reported-by: syzbot+eed305768ece6682bb7f@syzkaller.appspotmail.com
    Signed-off-by: David Howells

    David Howells
     
  • The rxrpc_peer record needs to hold a reference on the rxrpc_local record
    it points as the peer is used as a base to access information in the
    rxrpc_local record.

    This can cause problems in __rxrpc_put_peer(), where we need the network
    namespace pointer, and in rxrpc_send_keepalive(), where we need to access
    the UDP socket, leading to symptoms like:

    BUG: KASAN: use-after-free in __rxrpc_put_peer net/rxrpc/peer_object.c:411
    [inline]
    BUG: KASAN: use-after-free in rxrpc_put_peer+0x685/0x6a0
    net/rxrpc/peer_object.c:435
    Read of size 8 at addr ffff888097ec0058 by task syz-executor823/24216

    Fix this by taking a ref on the local record for the peer record.

    Fixes: ace45bec6d77 ("rxrpc: Fix firewall route keepalive")
    Fixes: 2baec2c3f854 ("rxrpc: Support network namespacing")
    Reported-by: syzbot+b9be979c55f2bea8ed30@syzkaller.appspotmail.com
    Signed-off-by: David Howells

    David Howells
     
  • rxrpc_put_call() calls trace_rxrpc_call() after it has done the decrement
    of the refcount - which looks at the debug_id in the call record. But
    unless the refcount was reduced to zero, we no longer have the right to
    look in the record and, indeed, it may be deleted by some other thread.

    Fix this by getting the debug_id out before decrementing the refcount and
    then passing that into the tracepoint.

    Fixes: e34d4234b0b7 ("rxrpc: Trace rxrpc_call usage")
    Signed-off-by: David Howells

    David Howells
     
  • rxrpc_put_*conn() calls trace_rxrpc_conn() after they have done the
    decrement of the refcount - which looks at the debug_id in the connection
    record. But unless the refcount was reduced to zero, we no longer have the
    right to look in the record and, indeed, it may be deleted by some other
    thread.

    Fix this by getting the debug_id out before decrementing the refcount and
    then passing that into the tracepoint.

    Fixes: 363deeab6d0f ("rxrpc: Add connection tracepoint and client conn state tracepoint")
    Signed-off-by: David Howells

    David Howells
     
  • rxrpc_put_peer() calls trace_rxrpc_peer() after it has done the decrement
    of the refcount - which looks at the debug_id in the peer record. But
    unless the refcount was reduced to zero, we no longer have the right to
    look in the record and, indeed, it may be deleted by some other thread.

    Fix this by getting the debug_id out before decrementing the refcount and
    then passing that into the tracepoint.

    This can cause the following symptoms:

    BUG: KASAN: use-after-free in __rxrpc_put_peer net/rxrpc/peer_object.c:411
    [inline]
    BUG: KASAN: use-after-free in rxrpc_put_peer+0x685/0x6a0
    net/rxrpc/peer_object.c:435
    Read of size 8 at addr ffff888097ec0058 by task syz-executor823/24216

    Fixes: 1159d4b496f5 ("rxrpc: Add a tracepoint to track rxrpc_peer refcounting")
    Reported-by: syzbot+b9be979c55f2bea8ed30@syzkaller.appspotmail.com
    Signed-off-by: David Howells

    David Howells
     
  • When sendmsg() finds a call to continue on with, if the call is in an
    inappropriate state, it doesn't release the ref it just got on that call
    before returning an error.

    This causes the following symptom to show up with kasan:

    BUG: KASAN: use-after-free in rxrpc_send_keepalive+0x8a2/0x940
    net/rxrpc/output.c:635
    Read of size 8 at addr ffff888064219698 by task kworker/0:3/11077

    where line 635 is:

    whdr.epoch = htonl(peer->local->rxnet->epoch);

    The local endpoint (which cannot be pinned by the call) has been released,
    but not the peer (which is pinned by the call).

    Fix this by releasing the call in the error path.

    Fixes: 37411cad633f ("rxrpc: Fix potential NULL-pointer exception")
    Reported-by: syzbot+d850c266e3df14da1d31@syzkaller.appspotmail.com
    Signed-off-by: David Howells

    David Howells
     

15 Sep, 2019

1 commit


05 Sep, 2019

1 commit

  • There's a misplaced traceline in rxrpc_input_packet() which is looking at a
    packet that just got released rather than the replacement packet.

    Fix this by moving the traceline after the assignment that moves the new
    packet pointer to the actual packet pointer.

    Fixes: d0d5c0cd1e71 ("rxrpc: Use skb_unshare() rather than skb_cow_data()")
    Reported-by: Hillf Danton
    Signed-off-by: David Howells
    Signed-off-by: David S. Miller

    David Howells