1. 19 8月, 2020 2 次提交
  2. 18 8月, 2020 2 次提交
  3. 17 8月, 2020 6 次提交
  4. 15 8月, 2020 5 次提交
  5. 14 8月, 2020 11 次提交
    • O
      can: j1939: transport: j1939_xtp_rx_dat_one(): compare own packets to detect corruptions · e052d054
      Oleksij Rempel 提交于
      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: NOleksij Rempel <o.rempel@pengutronix.de>
      Link: https://lore.kernel.org/r/20200807105200.26441-6-o.rempel@pengutronix.deSigned-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>
      e052d054
    • O
      can: j1939: transport: add j1939_session_skb_find_by_offset() function · 840835c9
      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: NHenrique Figueira <henrislip@gmail.com>
      Reported-by: https://github.com/linux-can/can-utils/issues/228
      Fixes: 9d71dd0c ("can: add support of SAE J1939 protocol")
      Signed-off-by: NOleksij Rempel <o.rempel@pengutronix.de>
      Link: https://lore.kernel.org/r/20200807105200.26441-5-o.rempel@pengutronix.deSigned-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>
      840835c9
    • O
      can: j1939: socket: j1939_sk_bind(): make sure ml_priv is allocated · af804b78
      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: 9d71dd0c ("can: add support of SAE J1939 protocol")
      Cc: linux-stable <stable@vger.kernel.org> # >= v5.4
      Signed-off-by: NOleksij Rempel <o.rempel@pengutronix.de>
      Link: https://lore.kernel.org/r/20200807105200.26441-4-o.rempel@pengutronix.deSigned-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>
      af804b78
    • O
      can: j1939: transport: j1939_session_tx_dat(): fix use-after-free read in j1939_tp_txtimer() · cd3b3636
      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: 9d71dd0c ("can: add support of SAE J1939 protocol")
      Cc: linux-stable <stable@vger.kernel.org> # >= v5.4
      Signed-off-by: NOleksij Rempel <o.rempel@pengutronix.de>
      Link: https://lore.kernel.org/r/20200807105200.26441-3-o.rempel@pengutronix.deSigned-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>
      cd3b3636
    • O
      can: j1939: transport: j1939_simple_recv(): ignore local J1939 messages send not by J1939 stack · b43e3a82
      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: 9d71dd0c ("can: add support of SAE J1939 protocol")
      Signed-off-by: NOleksij Rempel <o.rempel@pengutronix.de>
      Link: https://lore.kernel.org/r/20200807105200.26441-2-o.rempel@pengutronix.deSigned-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>
      b43e3a82
    • E
      can: j1939: fix kernel-infoleak in j1939_sk_sock2sockaddr_can() · 38ba8b92
      Eric Dumazet 提交于
      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: 9d71dd0c ("can: add support of SAE J1939 protocol")
      Signed-off-by: NEric Dumazet <edumazet@google.com>
      Reported-by: Nsyzbot <syzkaller@googlegroups.com>
      Cc: Robin van der Gracht <robin@protonic.nl>
      Cc: Oleksij Rempel <o.rempel@pengutronix.de>
      Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
      Cc: linux-can@vger.kernel.org
      Acked-by: NOleksij Rempel <o.rempel@pengutronix.de>
      Link: https://lore.kernel.org/r/20200813161834.4021638-1-edumazet@google.comSigned-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>
      38ba8b92
    • F
      netfilter: ebtables: reject bogus getopt len value · 5c04da55
      Florian Westphal 提交于
      syzkaller reports splat:
      ------------[ cut here ]------------
      Buffer overflow detected (80 < 137)!
      Call Trace:
       do_ebt_get_ctl+0x2b4/0x790 net/bridge/netfilter/ebtables.c:2317
       nf_getsockopt+0x72/0xd0 net/netfilter/nf_sockopt.c:116
       ip_getsockopt net/ipv4/ip_sockglue.c:1778 [inline]
      
      caused by a copy-to-user with a too-large "*len" value.
      This adds a argument check on *len just like in the non-compat version
      of the handler.
      
      Before the "Fixes" commit, the reproducer fails with -EINVAL as
      expected:
      1. core calls the "compat" getsockopt version
      2. compat getsockopt version detects the *len value is possibly
         in 64-bit layout (*len != compat_len)
      3. compat getsockopt version delegates everything to native getsockopt
         version
      4. native getsockopt rejects invalid *len
      
      -> compat handler only sees len == sizeof(compat_struct) for GET_ENTRIES.
      
      After the refactor, event sequence is:
      1. getsockopt calls "compat" version (len != native_len)
      2. compat version attempts to copy *len bytes, where *len is random
         value from userspace
      
      Fixes: fc66de8e ("netfilter/ebtables: clean up compat {get, set}sockopt handling")
      Reported-by: syzbot+5accb5c62faa1d346480@syzkaller.appspotmail.com
      Signed-off-by: NFlorian Westphal <fw@strlen.de>
      Reviewed-by: NChristoph Hellwig <hch@lst.de>
      Signed-off-by: NPablo Neira Ayuso <pablo@netfilter.org>
      5c04da55
    • T
      net: openvswitch: introduce common code for flushing flows · 1f3a090b
      Tonghao Zhang 提交于
      To avoid some issues, for example RCU usage warning and double free,
      we should flush the flows under ovs_lock. This patch refactors
      table_instance_destroy and introduces table_instance_flow_flush
      which can be invoked by __dp_destroy or ovs_flow_tbl_flush.
      
      Fixes: 50b0e61b ("net: openvswitch: fix possible memleak on destroy flow-table")
      Reported-by: NJohan Knöös <jknoos@google.com>
      Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-August/050489.htmlSigned-off-by: NTonghao Zhang <xiangxia.m.yue@gmail.com>
      Reviewed-by: NCong Wang <xiyou.wangcong@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      1f3a090b
    • J
      af_packet: TPACKET_V3: fix fill status rwlock imbalance · 88fd1cb8
      John Ogness 提交于
      After @blk_fill_in_prog_lock is acquired there is an early out vnet
      situation that can occur. In that case, the rwlock needs to be
      released.
      
      Also, since @blk_fill_in_prog_lock is only acquired when @tp_version
      is exactly TPACKET_V3, only release it on that exact condition as
      well.
      
      And finally, add sparse annotation so that it is clearer that
      prb_fill_curr_block() and prb_clear_blk_fill_status() are acquiring
      and releasing @blk_fill_in_prog_lock, respectively. sparse is still
      unable to understand the balance, but the warnings are now on a
      higher level that make more sense.
      
      Fixes: 632ca50f ("af_packet: TPACKET_V3: replace busy-wait loop")
      Signed-off-by: NJohn Ogness <john.ogness@linutronix.de>
      Reported-by: Nkernel test robot <lkp@intel.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      88fd1cb8
    • J
      bpf: sock_ops sk access may stomp registers when dst_reg = src_reg · 84f44df6
      John Fastabend 提交于
      Similar to patch ("bpf: sock_ops ctx access may stomp registers") if the
      src_reg = dst_reg when reading the sk field of a sock_ops struct we
      generate xlated code,
      
        53: (61) r9 = *(u32 *)(r9 +28)
        54: (15) if r9 == 0x0 goto pc+3
        56: (79) r9 = *(u64 *)(r9 +0)
      
      This stomps on the r9 reg to do the sk_fullsock check and then when
      reading the skops->sk field instead of the sk pointer we get the
      sk_fullsock. To fix use similar pattern noted in the previous fix
      and use the temp field to save/restore a register used to do
      sk_fullsock check.
      
      After the fix the generated xlated code reads,
      
        52: (7b) *(u64 *)(r9 +32) = r8
        53: (61) r8 = *(u32 *)(r9 +28)
        54: (15) if r9 == 0x0 goto pc+3
        55: (79) r8 = *(u64 *)(r9 +32)
        56: (79) r9 = *(u64 *)(r9 +0)
        57: (05) goto pc+1
        58: (79) r8 = *(u64 *)(r9 +32)
      
      Here r9 register was in-use so r8 is chosen as the temporary register.
      In line 52 r8 is saved in temp variable and at line 54 restored in case
      fullsock != 0. Finally we handle fullsock == 0 case by restoring at
      line 58.
      
      This adds a new macro SOCK_OPS_GET_SK it is almost possible to merge
      this with SOCK_OPS_GET_FIELD, but I found the extra branch logic a
      bit more confusing than just adding a new macro despite a bit of
      duplicating code.
      
      Fixes: 1314ef56 ("bpf: export bpf_sock for BPF_PROG_TYPE_SOCK_OPS prog type")
      Signed-off-by: NJohn Fastabend <john.fastabend@gmail.com>
      Signed-off-by: NDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: NSong Liu <songliubraving@fb.com>
      Acked-by: NMartin KaFai Lau <kafai@fb.com>
      Link: https://lore.kernel.org/bpf/159718349653.4728.6559437186853473612.stgit@john-Precision-5820-Tower
      84f44df6
    • J
      bpf: sock_ops ctx access may stomp registers in corner case · fd09af01
      John Fastabend 提交于
      I had a sockmap program that after doing some refactoring started spewing
      this splat at me:
      
      [18610.807284] BUG: unable to handle kernel NULL pointer dereference at 0000000000000001
      [...]
      [18610.807359] Call Trace:
      [18610.807370]  ? 0xffffffffc114d0d5
      [18610.807382]  __cgroup_bpf_run_filter_sock_ops+0x7d/0xb0
      [18610.807391]  tcp_connect+0x895/0xd50
      [18610.807400]  tcp_v4_connect+0x465/0x4e0
      [18610.807407]  __inet_stream_connect+0xd6/0x3a0
      [18610.807412]  ? __inet_stream_connect+0x5/0x3a0
      [18610.807417]  inet_stream_connect+0x3b/0x60
      [18610.807425]  __sys_connect+0xed/0x120
      
      After some debugging I was able to build this simple reproducer,
      
       __section("sockops/reproducer_bad")
       int bpf_reproducer_bad(struct bpf_sock_ops *skops)
       {
              volatile __maybe_unused __u32 i = skops->snd_ssthresh;
              return 0;
       }
      
      And along the way noticed that below program ran without splat,
      
      __section("sockops/reproducer_good")
      int bpf_reproducer_good(struct bpf_sock_ops *skops)
      {
              volatile __maybe_unused __u32 i = skops->snd_ssthresh;
              volatile __maybe_unused __u32 family;
      
              compiler_barrier();
      
              family = skops->family;
              return 0;
      }
      
      So I decided to check out the code we generate for the above two
      programs and noticed each generates the BPF code you would expect,
      
      0000000000000000 <bpf_reproducer_bad>:
      ;       volatile __maybe_unused __u32 i = skops->snd_ssthresh;
             0:       r1 = *(u32 *)(r1 + 96)
             1:       *(u32 *)(r10 - 4) = r1
      ;       return 0;
             2:       r0 = 0
             3:       exit
      
      0000000000000000 <bpf_reproducer_good>:
      ;       volatile __maybe_unused __u32 i = skops->snd_ssthresh;
             0:       r2 = *(u32 *)(r1 + 96)
             1:       *(u32 *)(r10 - 4) = r2
      ;       family = skops->family;
             2:       r1 = *(u32 *)(r1 + 20)
             3:       *(u32 *)(r10 - 8) = r1
      ;       return 0;
             4:       r0 = 0
             5:       exit
      
      So we get reasonable assembly, but still something was causing the null
      pointer dereference. So, we load the programs and dump the xlated version
      observing that line 0 above 'r* = *(u32 *)(r1 +96)' is going to be
      translated by the skops access helpers.
      
      int bpf_reproducer_bad(struct bpf_sock_ops * skops):
      ; volatile __maybe_unused __u32 i = skops->snd_ssthresh;
         0: (61) r1 = *(u32 *)(r1 +28)
         1: (15) if r1 == 0x0 goto pc+2
         2: (79) r1 = *(u64 *)(r1 +0)
         3: (61) r1 = *(u32 *)(r1 +2340)
      ; volatile __maybe_unused __u32 i = skops->snd_ssthresh;
         4: (63) *(u32 *)(r10 -4) = r1
      ; return 0;
         5: (b7) r0 = 0
         6: (95) exit
      
      int bpf_reproducer_good(struct bpf_sock_ops * skops):
      ; volatile __maybe_unused __u32 i = skops->snd_ssthresh;
         0: (61) r2 = *(u32 *)(r1 +28)
         1: (15) if r2 == 0x0 goto pc+2
         2: (79) r2 = *(u64 *)(r1 +0)
         3: (61) r2 = *(u32 *)(r2 +2340)
      ; volatile __maybe_unused __u32 i = skops->snd_ssthresh;
         4: (63) *(u32 *)(r10 -4) = r2
      ; family = skops->family;
         5: (79) r1 = *(u64 *)(r1 +0)
         6: (69) r1 = *(u16 *)(r1 +16)
      ; family = skops->family;
         7: (63) *(u32 *)(r10 -8) = r1
      ; return 0;
         8: (b7) r0 = 0
         9: (95) exit
      
      Then we look at lines 0 and 2 above. In the good case we do the zero
      check in r2 and then load 'r1 + 0' at line 2. Do a quick cross-check
      into the bpf_sock_ops check and we can confirm that is the 'struct
      sock *sk' pointer field. But, in the bad case,
      
         0: (61) r1 = *(u32 *)(r1 +28)
         1: (15) if r1 == 0x0 goto pc+2
         2: (79) r1 = *(u64 *)(r1 +0)
      
      Oh no, we read 'r1 +28' into r1, this is skops->fullsock and then in
      line 2 we read the 'r1 +0' as a pointer. Now jumping back to our spat,
      
      [18610.807284] BUG: unable to handle kernel NULL pointer dereference at 0000000000000001
      
      The 0x01 makes sense because that is exactly the fullsock value. And
      its not a valid dereference so we splat.
      
      To fix we need to guard the case when a program is doing a sock_ops field
      access with src_reg == dst_reg. This is already handled in the load case
      where the ctx_access handler uses a tmp register being careful to
      store the old value and restore it. To fix the get case test if
      src_reg == dst_reg and in this case do the is_fullsock test in the
      temporary register. Remembering to restore the temporary register before
      writing to either dst_reg or src_reg to avoid smashing the pointer into
      the struct holding the tmp variable.
      
      Adding this inline code to test_tcpbpf_kern will now be generated
      correctly from,
      
        9: r2 = *(u32 *)(r2 + 96)
      
      to xlated code,
      
        12: (7b) *(u64 *)(r2 +32) = r9
        13: (61) r9 = *(u32 *)(r2 +28)
        14: (15) if r9 == 0x0 goto pc+4
        15: (79) r9 = *(u64 *)(r2 +32)
        16: (79) r2 = *(u64 *)(r2 +0)
        17: (61) r2 = *(u32 *)(r2 +2348)
        18: (05) goto pc+1
        19: (79) r9 = *(u64 *)(r2 +32)
      
      And in the normal case we keep the original code, because really this
      is an edge case. From this,
      
        9: r2 = *(u32 *)(r6 + 96)
      
      to xlated code,
      
        22: (61) r2 = *(u32 *)(r6 +28)
        23: (15) if r2 == 0x0 goto pc+2
        24: (79) r2 = *(u64 *)(r6 +0)
        25: (61) r2 = *(u32 *)(r2 +2348)
      
      So three additional instructions if dst == src register, but I scanned
      my current code base and did not see this pattern anywhere so should
      not be a big deal. Further, it seems no one else has hit this or at
      least reported it so it must a fairly rare pattern.
      
      Fixes: 9b1f3d6e ("bpf: Refactor sock_ops_convert_ctx_access")
      Signed-off-by: NJohn Fastabend <john.fastabend@gmail.com>
      Signed-off-by: NDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: NSong Liu <songliubraving@fb.com>
      Acked-by: NMartin KaFai Lau <kafai@fb.com>
      Link: https://lore.kernel.org/bpf/159718347772.4728.2781381670567919577.stgit@john-Precision-5820-Tower
      fd09af01
  6. 13 8月, 2020 7 次提交
  7. 12 8月, 2020 5 次提交
  8. 11 8月, 2020 2 次提交