24 Aug, 2020

1 commit

  • Replace the existing /* fall through */ comments and its variants with
    the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary
    fall-through markings when it is the case.

    [1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through

    Signed-off-by: Gustavo A. R. Silva

    Gustavo A. R. Silva
     

15 Aug, 2020

4 commits

  • According to SAE J1939/21 (Chapter 5.12.3 and APPENDIX C), for transmit side
    the required time interval between packets of a multipacket broadcast message
    is 50 to 200 ms, the responder shall use a timeout of 250ms (provides margin
    allowing for the maximumm spacing of 200ms). For receive side a timeout will
    occur when a time of greater than 750 ms elapsed between two message packets
    when more packets were expected.

    So this patch fix and add rxtimer for multipacket broadcast session.

    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Signed-off-by: Zhang Changzhong
    Link: https://lore.kernel.org/r/1596599425-5534-5-git-send-email-zhangchangzhong@huawei.com
    Acked-by: Oleksij Rempel
    Signed-off-by: Marc Kleine-Budde

    Zhang Changzhong
     
  • If timeout occurs, j1939_tp_rxtimer() first calls hrtimer_start() to restart
    rxtimer, and then calls __j1939_session_cancel() to set session->state =
    J1939_SESSION_WAITING_ABORT. At next timeout expiration, because of the
    J1939_SESSION_WAITING_ABORT session state j1939_tp_rxtimer() will call
    j1939_session_deactivate_activate_next() to deactivate current session, and
    rxtimer won't be set.

    But for multipacket broadcast session, __j1939_session_cancel() don't set
    session->state = J1939_SESSION_WAITING_ABORT, thus current session won't be
    deactivate and hrtimer_start() is called to start new rxtimer again and again.

    So fix it by moving session->state = J1939_SESSION_WAITING_ABORT out of if
    (!j1939_cb_is_broadcast(&session->skcb)) statement.

    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Signed-off-by: Zhang Changzhong
    Link: https://lore.kernel.org/r/1596599425-5534-4-git-send-email-zhangchangzhong@huawei.com
    Acked-by: Oleksij Rempel
    Signed-off-by: Marc Kleine-Budde

    Zhang Changzhong
     
  • If j1939_xtp_rx_dat_one() receive last frame of multipacket broadcast message,
    j1939_session_timers_cancel() should be called to cancel rxtimer.

    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Signed-off-by: Zhang Changzhong
    Link: https://lore.kernel.org/r/1596599425-5534-3-git-send-email-zhangchangzhong@huawei.com
    Acked-by: Oleksij Rempel
    Signed-off-by: Marc Kleine-Budde

    Zhang Changzhong
     
  • Currently j1939_tp_im_involved_anydir() in j1939_tp_recv() check the previously
    set flags J1939_ECU_LOCAL_DST and J1939_ECU_LOCAL_SRC of incoming skb, thus
    multipacket broadcast message was aborted by receive side because it may come
    from remote ECUs and have no exact dst address. Similarly, j1939_tp_cmd_recv()
    and j1939_xtp_rx_dat() didn't process broadcast message.

    So fix it by checking and process broadcast message in j1939_tp_recv(),
    j1939_tp_cmd_recv() and j1939_xtp_rx_dat().

    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Signed-off-by: Zhang Changzhong
    Link: https://lore.kernel.org/r/1596599425-5534-2-git-send-email-zhangchangzhong@huawei.com
    Acked-by: Oleksij Rempel
    Signed-off-by: Marc Kleine-Budde

    Zhang Changzhong
     

14 Aug, 2020

6 commits

  • Since the stack relays on receiving own packets, it was overwriting own
    transmit buffer from received packets.

    At least theoretically, the received echo buffer can be corrupt or
    changed and the session partner can request to resend previous data. In
    this case we will re-send bad data.

    With this patch we will stop to overwrite own TX buffer and use it for
    sanity checking.

    Signed-off-by: Oleksij Rempel
    Link: https://lore.kernel.org/r/20200807105200.26441-6-o.rempel@pengutronix.de
    Signed-off-by: Marc Kleine-Budde

    Oleksij Rempel
     
  • Sometimes it makes no sense to search the skb by pkt.dpo, since we need
    next the skb within the transaction block. This may happen if we have an
    ETP session with CTS set to less than 255 packets.

    After this patch, we will be able to work with ETP sessions where the
    block size (ETP.CM_CTS byte 2) is less than 255 packets.

    Reported-by: Henrique Figueira
    Reported-by: https://github.com/linux-can/can-utils/issues/228
    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Signed-off-by: Oleksij Rempel
    Link: https://lore.kernel.org/r/20200807105200.26441-5-o.rempel@pengutronix.de
    Signed-off-by: Marc Kleine-Budde

    Oleksij Rempel
     
  • This patch adds check to ensure that the struct net_device::ml_priv is
    allocated, as it is used later by the j1939 stack.

    The allocation is done by all mainline CAN network drivers, but when using
    bond or team devices this is not the case.

    Bail out if no ml_priv is allocated.

    Reported-by: syzbot+f03d384f3455d28833eb@syzkaller.appspotmail.com
    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Cc: linux-stable # >= v5.4
    Signed-off-by: Oleksij Rempel
    Link: https://lore.kernel.org/r/20200807105200.26441-4-o.rempel@pengutronix.de
    Signed-off-by: Marc Kleine-Budde

    Oleksij Rempel
     
  • The current stack implementation do not support ECTS requests of not
    aligned TP sized blocks.

    If ECTS will request a block with size and offset spanning two TP
    blocks, this will cause memcpy() to read beyond the queued skb (which
    does only contain one TP sized block).

    Sometimes KASAN will detect this read if the memory region beyond the
    skb was previously allocated and freed. In other situations it will stay
    undetected. The ETP transfer in any case will be corrupted.

    This patch adds a sanity check to avoid this kind of read and abort the
    session with error J1939_XTP_ABORT_ECTS_TOO_BIG.

    Reported-by: syzbot+5322482fe520b02aea30@syzkaller.appspotmail.com
    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Cc: linux-stable # >= v5.4
    Signed-off-by: Oleksij Rempel
    Link: https://lore.kernel.org/r/20200807105200.26441-3-o.rempel@pengutronix.de
    Signed-off-by: Marc Kleine-Budde

    Oleksij Rempel
     
  • In current J1939 stack implementation, we process all locally send
    messages as own messages. Even if it was send by CAN_RAW socket.

    To reproduce it use following commands:
    testj1939 -P -r can0:0x80 &
    cansend can0 18238040#0123

    This step will trigger false positive not critical warning:
    j1939_simple_recv: Received already invalidated message

    With this patch we add additional check to make sure, related skb is own
    echo message.

    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Signed-off-by: Oleksij Rempel
    Link: https://lore.kernel.org/r/20200807105200.26441-2-o.rempel@pengutronix.de
    Signed-off-by: Marc Kleine-Budde

    Oleksij Rempel
     
  • syzbot found that at least 2 bytes of kernel information
    were leaked during getsockname() on AF_CAN CAN_J1939 socket.

    Since struct sockaddr_can has in fact two holes, simply
    clear the whole area before filling it with useful data.

    BUG: KMSAN: kernel-infoleak in kmsan_copy_to_user+0x81/0x90 mm/kmsan/kmsan_hooks.c:253
    CPU: 0 PID: 8466 Comm: syz-executor511 Not tainted 5.8.0-rc5-syzkaller #0
    Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
    Call Trace:
    __dump_stack lib/dump_stack.c:77 [inline]
    dump_stack+0x21c/0x280 lib/dump_stack.c:118
    kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:121
    kmsan_internal_check_memory+0x238/0x3d0 mm/kmsan/kmsan.c:423
    kmsan_copy_to_user+0x81/0x90 mm/kmsan/kmsan_hooks.c:253
    instrument_copy_to_user include/linux/instrumented.h:91 [inline]
    _copy_to_user+0x18e/0x260 lib/usercopy.c:39
    copy_to_user include/linux/uaccess.h:186 [inline]
    move_addr_to_user+0x3de/0x670 net/socket.c:237
    __sys_getsockname+0x407/0x5e0 net/socket.c:1909
    __do_sys_getsockname net/socket.c:1920 [inline]
    __se_sys_getsockname+0x91/0xb0 net/socket.c:1917
    __x64_sys_getsockname+0x4a/0x70 net/socket.c:1917
    do_syscall_64+0xad/0x160 arch/x86/entry/common.c:386
    entry_SYSCALL_64_after_hwframe+0x44/0xa9
    RIP: 0033:0x440219
    Code: Bad RIP value.
    RSP: 002b:00007ffe5ee150c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000033
    RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
    RDX: 0000000020000240 RSI: 0000000020000100 RDI: 0000000000000003
    RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
    R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401a20
    R13: 0000000000401ab0 R14: 0000000000000000 R15: 0000000000000000

    Local variable ----address@__sys_getsockname created at:
    __sys_getsockname+0x91/0x5e0 net/socket.c:1894
    __sys_getsockname+0x91/0x5e0 net/socket.c:1894

    Bytes 2-3 of 24 are uninitialized
    Memory access of size 24 starts at ffff8880ba2c7de8
    Data copied to user address 0000000020000100

    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Signed-off-by: Eric Dumazet
    Reported-by: syzbot
    Cc: Robin van der Gracht
    Cc: Oleksij Rempel
    Cc: Pengutronix Kernel Team
    Cc: linux-can@vger.kernel.org
    Acked-by: Oleksij Rempel
    Link: https://lore.kernel.org/r/20200813161834.4021638-1-edumazet@google.com
    Signed-off-by: Marc Kleine-Budde

    Eric Dumazet
     

25 Jul, 2020

1 commit

  • Rework the remaining setsockopt code to pass a sockptr_t instead of a
    plain user pointer. This removes the last remaining set_fs(KERNEL_DS)
    outside of architecture specific code.

    Signed-off-by: Christoph Hellwig
    Acked-by: Stefan Schmidt [ieee802154]
    Acked-by: Matthieu Baerts
    Signed-off-by: David S. Miller

    Christoph Hellwig
     

20 Jul, 2020

1 commit


14 Jul, 2020

1 commit


14 Jun, 2020

1 commit

  • Since commit 84af7a6194e4 ("checkpatch: kconfig: prefer 'help' over
    '---help---'"), the number of '---help---' has been gradually
    decreasing, but there are still more than 2400 instances.

    This commit finishes the conversion. While I touched the lines,
    I also fixed the indentation.

    There are a variety of indentation styles found.

    a) 4 spaces + '---help---'
    b) 7 spaces + '---help---'
    c) 8 spaces + '---help---'
    d) 1 space + 1 tab + '---help---'
    e) 1 tab + '---help---' (correct indentation)
    f) 1 tab + 1 space + '---help---'
    g) 1 tab + 2 spaces + '---help---'

    In order to convert all of them to 1 tab + 'help', I ran the
    following commend:

    $ find . -name 'Kconfig*' | xargs sed -i 's/^[[:space:]]*---help---/\thelp/'

    Signed-off-by: Masahiro Yamada

    Masahiro Yamada
     

08 Dec, 2019

1 commit

  • syzbot reproduced following crash:

    ===============================================================================
    kasan: CONFIG_KASAN_INLINE enabled
    kasan: GPF could be caused by NULL-ptr deref or user memory access
    general protection fault: 0000 [#1] PREEMPT SMP KASAN
    CPU: 0 PID: 9844 Comm: syz-executor.0 Not tainted 5.4.0-syzkaller #0
    Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
    Google 01/01/2011
    RIP: 0010:__lock_acquire+0x1254/0x4a00 kernel/locking/lockdep.c:3828
    Code: 00 0f 85 96 24 00 00 48 81 c4 f0 00 00 00 5b 41 5c 41 5d 41 5e 41
    5f 5d c3 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 3c 02
    00 0f 85 0b 28 00 00 49 81 3e 20 19 78 8a 0f 84 5f ee ff
    RSP: 0018:ffff888099c3fb48 EFLAGS: 00010006
    RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
    RDX: 0000000000000218 RSI: 0000000000000000 RDI: 0000000000000001
    RBP: ffff888099c3fc60 R08: 0000000000000001 R09: 0000000000000001
    R10: fffffbfff146e1d0 R11: ffff888098720400 R12: 00000000000010c0
    R13: 0000000000000000 R14: 00000000000010c0 R15: 0000000000000000
    FS: 00007f0559e98700(0000) GS:ffff8880ae800000(0000)
    knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 00007fe4d89e0000 CR3: 0000000099606000 CR4: 00000000001406f0
    DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
    Call Trace:
    lock_acquire+0x190/0x410 kernel/locking/lockdep.c:4485
    __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
    _raw_spin_lock_bh+0x33/0x50 kernel/locking/spinlock.c:175
    spin_lock_bh include/linux/spinlock.h:343 [inline]
    j1939_jsk_del+0x32/0x210 net/can/j1939/socket.c:89
    j1939_sk_bind+0x2ea/0x8f0 net/can/j1939/socket.c:448
    __sys_bind+0x239/0x290 net/socket.c:1648
    __do_sys_bind net/socket.c:1659 [inline]
    __se_sys_bind net/socket.c:1657 [inline]
    __x64_sys_bind+0x73/0xb0 net/socket.c:1657
    do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
    entry_SYSCALL_64_after_hwframe+0x49/0xbe
    RIP: 0033:0x45a679
    Code: ad b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89
    f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 3d 01
    f0 ff ff 0f 83 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
    RSP: 002b:00007f0559e97c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000031
    RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 000000000045a679
    RDX: 0000000000000018 RSI: 0000000020000240 RDI: 0000000000000003
    RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000246 R12: 00007f0559e986d4
    R13: 00000000004c09e9 R14: 00000000004d37d0 R15: 00000000ffffffff
    Modules linked in:
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 9844 at kernel/locking/mutex.c:1419
    mutex_trylock+0x279/0x2f0 kernel/locking/mutex.c:1427
    ===============================================================================

    This issues was caused by null pointer deference. Where j1939_sk_bind()
    was using currently not existing priv.

    Possible scenario may look as following:
    cpu0 cpu1
    bind()
    bind()
    j1939_sk_bind()
    j1939_sk_bind()
    priv = jsk->priv;
    priv = jsk->priv;
    lock_sock(sock->sk);
    priv = j1939_netdev_start(ndev);
    j1939_jsk_add(priv, jsk);
    jsk->priv = priv;
    relase_sock(sock->sk);
    lock_sock(sock->sk);
    j1939_jsk_del(priv, jsk);
    ..... ooops ......

    With this patch we move "priv = jsk->priv;" after the lock, to avoid
    assigning of wrong priv pointer.

    Reported-by: syzbot+99e9e1b200a1e363237d@syzkaller.appspotmail.com
    Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
    Signed-off-by: Oleksij Rempel
    Cc: linux-stable # >= v5.4
    Signed-off-by: Marc Kleine-Budde

    Oleksij Rempel
     

13 Nov, 2019

9 commits


05 Nov, 2019

4 commits


04 Sep, 2019

11 commits

  • SAE J1939 is the vehicle bus recommended practice used for communication
    and diagnostics among vehicle components. Originating in the car and
    heavy-duty truck industry in the United States, it is now widely used in
    other parts of the world.

    J1939, ISO 11783 and NMEA 2000 all share the same high level protocol.
    SAE J1939 can be considered the replacement for the older SAE J1708 and
    SAE J1587 specifications.

    Acked-by: Oliver Hartkopp
    Signed-off-by: Bastian Stender
    Signed-off-by: Elenita Hinds
    Signed-off-by: kbuild test robot
    Signed-off-by: Kurt Van Dijck
    Signed-off-by: Maxime Jayat
    Signed-off-by: Robin van der Gracht
    Signed-off-by: Oleksij Rempel
    Signed-off-by: Marc Kleine-Budde

    The j1939 authors
     
  • The size of this structure will be increased with J1939 support. To stay
    binary compatible, the CAN_REQUIRED_SIZE macro is introduced for
    existing CAN protocols.

    Signed-off-by: Kurt Van Dijck
    Signed-off-by: Oleksij Rempel
    Acked-by: Oliver Hartkopp
    Signed-off-by: Marc Kleine-Budde

    Kurt Van Dijck
     
  • The can_rx_unregister() can be called from NAPI (soft IRQ) context, at least
    by j1939 stack. This leads to potential dead lock with &net->can.rcvlists_lock
    called from can_rx_register:
    ===============================================================================
    WARNING: inconsistent lock state
    4.19.0-20181029-1-g3e67f95ba0d3 #3 Not tainted
    --------------------------------
    inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
    testj1939/224 [HC0[0]:SC1[1]:HE1:SE0] takes:
    1ad0fda3 (&(&net->can.rcvlists_lock)->rlock){+.?.}, at: can_rx_unregister+0x4c/0x1ac
    {SOFTIRQ-ON-W} state was registered at:
    lock_acquire+0xd0/0x1f4
    _raw_spin_lock+0x30/0x40
    can_rx_register+0x5c/0x14c
    j1939_netdev_start+0xdc/0x1f8
    j1939_sk_bind+0x18c/0x1c8
    __sys_bind+0x70/0xb0
    sys_bind+0x10/0x14
    ret_fast_syscall+0x0/0x28
    0xbedc9b64
    irq event stamp: 2440
    hardirqs last enabled at (2440): [] __local_bh_enable_ip+0xac/0x184
    hardirqs last disabled at (2439): [] __local_bh_enable_ip+0x60/0x184
    softirqs last enabled at (2412): [] release_sock+0x84/0xa4
    softirqs last disabled at (2415): [] irq_exit+0x100/0x1b0

    other info that might help us debug this:
    Possible unsafe locking scenario:

    CPU0
    ----
    lock(&(&net->can.rcvlists_lock)->rlock);

    lock(&(&net->can.rcvlists_lock)->rlock);

    *** DEADLOCK ***

    2 locks held by testj1939/224:
    #0: 168eb13b (rcu_read_lock){....}, at: netif_receive_skb_internal+0x3c/0x350
    #1: 168eb13b (rcu_read_lock){....}, at: can_receive+0x88/0x1c0
    ===============================================================================

    To avoid this situation, we should use spin_lock_bh() instead of spin_lock().

    Signed-off-by: Oleksij Rempel
    Acked-by: Oliver Hartkopp
    Signed-off-by: Marc Kleine-Budde

    Oleksij Rempel
     
  • Since using the "struct can_ml_priv" for the per device "struct
    dev_rcv_lists" the call can_dev_rcv_lists_find() cannot fail anymore.
    This patch simplifies af_can by removing the NULL pointer checks from
    the dev_rcv_lists returned by can_dev_rcv_lists_find().

    Signed-off-by: Oleksij Rempel
    Acked-by: Oliver Hartkopp
    Signed-off-by: Marc Kleine-Budde

    Marc Kleine-Budde
     
  • This patch removes the old method of allocating the per device protocol
    specific memory via a netdevice_notifier. This had the drawback, that
    the allocation can fail, leading to a lot of null pointer checks in the
    code. This also makes the live cycle management of this memory quite
    complicated.

    This patch switches from the allocating the struct can_dev_rcv_lists in
    a NETDEV_REGISTER call to using the dev->ml_priv, which is allocated by
    the driver since the previous patch.

    Signed-off-by: Oleksij Rempel
    Acked-by: Oliver Hartkopp
    Signed-off-by: Marc Kleine-Budde

    Marc Kleine-Budde
     
  • This patch introduces the CAN midlayer private structure ("struct
    can_ml_priv") which should be used to hold protocol specific per device
    data structures. For now it's only member is "struct can_dev_rcv_lists".

    The CAN midlayer private is allocated via alloc_netdev()'s private and
    assigned to "struct net_device::ml_priv" during device creation. This is
    done transparently for CAN drivers using alloc_candev(). The slcan, vcan
    and vxcan drivers which are not using alloc_candev() have been adopted
    manually. The memory layout of the netdev_priv allocated via
    alloc_candev() will looke like this:

    +-------------------------+
    | driver's priv |
    +-------------------------+
    | struct can_ml_priv |
    +-------------------------+
    | array of struct sk_buff |
    +-------------------------+

    Signed-off-by: Oleksij Rempel
    Signed-off-by: Oliver Hartkopp
    Signed-off-by: Marc Kleine-Budde

    Marc Kleine-Budde
     
  • The networking core takes care and unregisters every network device in
    a namespace before calling the can_pernet_exit() hook. This patch
    removes the unneeded cleanup.

    Acked-by: Oliver Hartkopp
    Suggested-by: Kirill Tkhai
    Signed-off-by: Oleksij Rempel
    Signed-off-by: Marc Kleine-Budde

    Marc Kleine-Budde
     
  • This patch replaces an open coded max by the proper kernel define max().

    Acked-by: Oliver Hartkopp
    Signed-off-by: Oleksij Rempel
    Signed-off-by: Marc Kleine-Budde

    Marc Kleine-Budde
     
  • This patch gives the variables holding the CAN receiver and the receiver
    list a better name by renaming them from "r to "rcv" and "rl" to
    "recv_list".

    Signed-off-by: Oleksij Rempel
    Acked-by: Oliver Hartkopp
    Signed-off-by: Marc Kleine-Budde

    Marc Kleine-Budde
     
  • This patch add the commonly used prefix "can_" to the find_dev_rcv_lists()
    function and moves the "find" to the end, as the function returns a struct
    can_dev_rcv_list. This improves the overall readability of the code.

    Signed-off-by: Oleksij Rempel
    Acked-by: Oliver Hartkopp
    Signed-off-by: Marc Kleine-Budde

    Marc Kleine-Budde
     
  • This patch add the commonly used prefix "can_" to the find_rcv_list()
    function and add the "find" to the end, as the function returns a struct
    rcv_list. This improves the overall readability of the code.

    Signed-off-by: Oleksij Rempel
    Acked-by: Oliver Hartkopp
    Signed-off-by: Marc Kleine-Budde

    Marc Kleine-Budde