Skip to content
  1. Apr 01, 2021
  2. Mar 31, 2021
    • Eric Dumazet's avatar
      net: ensure mac header is set in virtio_net_hdr_to_skb() · 61431a59
      Eric Dumazet authored
      Commit 924a9bc3 ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
      added a call to dev_parse_header_protocol() but mac_header is not yet set.
      
      This means that eth_hdr() reads complete garbage, and syzbot complained about it [1]
      
      This patch resets mac_header earlier, to get more coverage about this change.
      
      Audit of virtio_net_hdr_to_skb() callers shows that this change should be safe.
      
      [1]
      
      BUG: KASAN: use-after-free in eth_header_parse_protocol+0xdc/0xe0 net/ethernet/eth.c:282
      Read of size 2 at addr ffff888017a6200b by task syz-executor313/8409
      
      CPU: 1 PID: 8409 Comm: syz-executor313 Not tainted 5.12.0-rc2-syzkaller #0
      Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
      Call Trace:
       __dump_stack lib/dump_stack.c:79 [inline]
       dump_stack+0x141/0x1d7 lib/dump_stack.c:120
       print_address_description.constprop.0.cold+0x5b/0x2f8 mm/kasan/report.c:232
       __kasan_report mm/kasan/report.c:399 [inline]
       kasan_report.cold+0x7c/0xd8 mm/kasan/report.c:416
       eth_header_parse_protocol+0xdc/0xe0 net/ethernet/eth.c:282
       dev_parse_header_protocol include/linux/netdevice.h:3177 [inline]
       virtio_net_hdr_to_skb.constprop.0+0x99d/0xcd0 include/linux/virtio_net.h:83
       packet_snd net/packet/af_packet.c:2994 [inline]
       packet_sendmsg+0x2325/0x52b0 net/packet/af_packet.c:3031
       sock_sendmsg_nosec net/socket.c:654 [inline]
       sock_sendmsg+0xcf/0x120 net/socket.c:674
       sock_no_sendpage+0xf3/0x130 net/core/sock.c:2860
       kernel_sendpage.part.0+0x1ab/0x350 net/socket.c:3631
       kernel_sendpage net/socket.c:3628 [inline]
       sock_sendpage+0xe5/0x140 net/socket.c:947
       pipe_to_sendpage+0x2ad/0x380 fs/splice.c:364
       splice_from_pipe_feed fs/splice.c:418 [inline]
       __splice_from_pipe+0x43e/0x8a0 fs/splice.c:562
       splice_from_pipe fs/splice.c:597 [inline]
       generic_splice_sendpage+0xd4/0x140 fs/splice.c:746
       do_splice_from fs/splice.c:767 [inline]
       do_splice+0xb7e/0x1940 fs/splice.c:1079
       __do_splice+0x134/0x250 fs/splice.c:1144
       __do_sys_splice fs/splice.c:1350 [inline]
       __se_sys_splice fs/splice.c:1332 [inline]
       __x64_sys_splice+0x198/0x250 fs/splice.c:1332
       do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
      
      Fixes: 924a9bc3
      
       ("net: check if protocol extracted by virtio_net_hdr_set_proto is correct")
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Cc: Balazs Nemeth <bnemeth@redhat.com>
      Cc: Willem de Bruijn <willemb@google.com>
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      61431a59
    • Florian Fainelli's avatar
      net: phy: broadcom: Only advertise EEE for supported modes · c056d480
      Florian Fainelli authored
      We should not be advertising EEE for modes that we do not support,
      correct that oversight by looking at the PHY device supported linkmodes.
      
      Fixes: 99cec8a4
      
       ("net: phy: broadcom: Allow enabling or disabling of EEE")
      Signed-off-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      c056d480
    • Yinjun Zhang's avatar
      nfp: flower: ignore duplicate merge hints from FW · 2ea538db
      Yinjun Zhang authored
      A merge hint message needs some time to process before the merged
      flow actually reaches the firmware, during which we may get duplicate
      merge hints if there're more than one packet that hit the pre-merged
      flow. And processing duplicate merge hints will cost extra host_ctx's
      which are a limited resource.
      
      Avoid the duplicate merge by using hash table to store the sub_flows
      to be merged.
      
      Fixes: 8af56f40
      
       ("nfp: flower: offload merge flows")
      Signed-off-by: default avatarYinjun Zhang <yinjun.zhang@corigine.com>
      Signed-off-by: default avatarLouis Peens <louis.peens@corigine.com>
      Signed-off-by: default avatarSimon Horman <simon.horman@netronome.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      2ea538db
    • Paolo Abeni's avatar
      net: let skb_orphan_partial wake-up waiters. · 9adc89af
      Paolo Abeni authored
      Currently the mentioned helper can end-up freeing the socket wmem
      without waking-up any processes waiting for more write memory.
      
      If the partially orphaned skb is attached to an UDP (or raw) socket,
      the lack of wake-up can hang the user-space.
      
      Even for TCP sockets not calling the sk destructor could have bad
      effects on TSQ.
      
      Address the issue using skb_orphan to release the sk wmem before
      setting the new sock_efree destructor. Additionally bundle the
      whole ownership update in a new helper, so that later other
      potential users could avoid duplicate code.
      
      v1 -> v2:
       - use skb_orphan() instead of sort of open coding it (Eric)
       - provide an helper for the ownership change (Eric)
      
      Fixes: f6ba8d33
      
       ("netem: fix skb_orphan_partial()")
      Suggested-by: default avatarEric Dumazet <edumazet@google.com>
      Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
      Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9adc89af
    • Yunjian Wang's avatar
      sch_htb: fix null pointer dereference on a null new_q · ae81feb7
      Yunjian Wang authored
      sch_htb: fix null pointer dereference on a null new_q
      
      Currently if new_q is null, the null new_q pointer will be
      dereference when 'q->offload' is true. Fix this by adding
      a braces around htb_parent_to_leaf_offload() to avoid it.
      
      Addresses-Coverity: ("Dereference after null check")
      Fixes: d03b195b
      
       ("sch_htb: Hierarchical QoS hardware offload")
      
      Signed-off-by: default avatarYunjian Wang <wangyunjian@huawei.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ae81feb7
    • Loic Poulain's avatar
      net: qrtr: Fix memory leak on qrtr_tx_wait failure · 8a03dd92
      Loic Poulain authored
      qrtr_tx_wait does not check for radix_tree_insert failure, causing
      the 'flow' object to be unreferenced after qrtr_tx_wait return. Fix
      that by releasing flow on radix_tree_insert failure.
      
      Fixes: 5fdeb0d3
      
       ("net: qrtr: Implement outgoing flow control")
      Reported-by: default avatar <syzbot+739016799a89c530b32a@syzkaller.appspotmail.com>
      Signed-off-by: default avatarLoic Poulain <loic.poulain@linaro.org>
      Reviewed-by: default avatarBjorn Andersson <bjorn.andersson@linaro.org>
      Reviewed-by: default avatarManivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      8a03dd92
    • Kumar Kartikeya Dwivedi's avatar
      net: sched: bump refcount for new action in ACT replace mode · 6855e821
      Kumar Kartikeya Dwivedi authored
      Currently, action creation using ACT API in replace mode is buggy.
      When invoking for non-existent action index 42,
      
      	tc action replace action bpf obj foo.o sec <xyz> index 42
      
      kernel creates the action, fills up the netlink response, and then just
      deletes the action after notifying userspace.
      
      	tc action show action bpf
      
      doesn't list the action.
      
      This happens due to the following sequence when ovr = 1 (replace mode)
      is enabled:
      
      tcf_idr_check_alloc is used to atomically check and either obtain
      reference for existing action at index, or reserve the index slot using
      a dummy entry (ERR_PTR(-EBUSY)).
      
      This is necessary as pointers to these actions will be held after
      dropping the idrinfo lock, so bumping the reference count is necessary
      as we need to insert the actions, and notify userspace by dumping their
      attributes. Finally, we drop the reference we took using the
      tcf_action_put_many call in tcf_action_add. However, for the case where
      a new action is created due to free index, its refcount remains one.
      This when paired with the put_many call leads to the kernel setting up
      the action, notifying userspace of its creation, and then tearing it
      down. For existing actions, the refcount is still held so they remain
      unaffected.
      
      Fortunately due to rtnl_lock serialization requirement, such an action
      with refcount == 1 will not be concurrently deleted by anything else, at
      best CLS API can move its refcount up and down by binding to it after it
      has been published from tcf_idr_insert_many. Since refcount is atleast
      one until put_many call, CLS API cannot delete it. Also __tcf_action_put
      release path already ensures deterministic outcome (either new action
      will be created or existing action will be reused in case CLS API tries
      to bind to action concurrently) due to idr lock serialization.
      
      We fix this by making refcount of newly created actions as 2 in ACT API
      replace mode. A relaxed store will suffice as visibility is ensured only
      after the tcf_idr_insert_many call.
      
      Note that in case of creation or overwriting using CLS API only (i.e.
      bind = 1), overwriting existing action object is not allowed, and any
      such request is silently ignored (without error).
      
      The refcount bump that occurs in tcf_idr_check_alloc call there for
      existing action will pair with tcf_exts_destroy call made from the
      owner module for the same action. In case of action creation, there
      is no existing action, so no tcf_exts_destroy callback happens.
      
      This means no code changes for CLS API.
      
      Fixes: cae422f3
      
       ("net: sched: use reference counting action init")
      Signed-off-by: default avatarKumar Kartikeya Dwivedi <memxor@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6855e821
    • Milton Miller's avatar
      net/ncsi: Avoid channel_monitor hrtimer deadlock · 03cb4d05
      Milton Miller authored
      Calling ncsi_stop_channel_monitor from channel_monitor is a guaranteed
      deadlock on SMP because stop calls del_timer_sync on the timer that
      invoked channel_monitor as its timer function.
      
      Recognise the inherent race of marking the monitor disabled before
      deleting the timer by just returning if enable was cleared.  After
      a timeout (the default case -- reset to START when response received)
      just mark the monitor.enabled false.
      
      If the channel has an entry on the channel_queue list, or if the
      state is not ACTIVE or INACTIVE, then warn and mark the timer stopped
      and don't restart, as the locking is broken somehow.
      
      Fixes: 0795fb20
      
       ("net/ncsi: Stop monitor if channel times out or is inactive")
      Signed-off-by: default avatarMilton Miller <miltonm@us.ibm.com>
      Signed-off-by: default avatarEddie James <eajames@linux.ibm.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      03cb4d05
  3. Mar 30, 2021
  4. Mar 29, 2021
    • Marc Kleine-Budde's avatar
      can: uapi: can.h: mark union inside struct can_frame packed · f5076c6b
      Marc Kleine-Budde authored
      In commit ea780056 ("can: add optional DLC element to Classical
      CAN frame structure") the struct can_frame::can_dlc was put into an
      anonymous union with another u8 variable.
      
      For various reasons some members in struct can_frame and canfd_frame
      including the first 8 byes of data are expected to have the same
      memory layout. This is enforced by a BUILD_BUG_ON check in af_can.c.
      
      Since the above mentioned commit this check fails on ARM kernels
      compiled with the ARM OABI (which means CONFIG_AEABI not set). In this
      case -mabi=apcs-gnu is passed to the compiler, which leads to a
      structure size boundary of 32, instead of 8 compared to CONFIG_AEABI
      enabled. This means the the union in struct can_frame takes 4 bytes
      instead of the expected 1.
      
      Rong Chen illustrates the problem with pahole in the ARM OABI case:
      
      | struct can_frame {
      |          canid_t                    can_id;               /* 0     4 */
      |          union {
      |                  __u8               len;                  /* 4     1 */
      |                  __u8               can_dlc;              /* 4     1 */
      |          };                                               /* 4     4 */
      |          __u8                       __pad;                /* 8     1 */
      |          __u8                       __res0;               /* 9     1 */
      |          __u8                       len8_dlc;             /* 10    1 */
      |
      |          /* XXX 5 bytes hole, try to pack */
      |
      |          __u8                       data[8]
      | __attribute__((__aligned__(8))); /*    16     8 */
      |
      |          /* size: 24, cachelines: 1, members: 6 */
      |          /* sum members: 19, holes: 1, sum holes: 5 */
      |          /* forced alignments: 1, forced holes: 1, sum forced holes: 5 */
      |          /* last cacheline: 24 bytes */
      | } __attribute__((__aligned__(8)));
      
      Marking the anonymous union as __attribute__((packed)) fixes the
      BUILD_BUG_ON problem on these compilers.
      
      Fixes: ea780056
      
       ("can: add optional DLC element to Classical CAN frame structure")
      Reported-by: default avatarkernel test robot <lkp@intel.com>
      Suggested-by: default avatarRong Chen <rong.a.chen@intel.com>
      Link: https://lore.kernel.org/linux-can/2c82ec23-3551-61b5-1bd8-178c3407ee83@hartkopp.net/
      Link: https://lore.kernel.org/r/20210325125850.1620-3-socketcan@hartkopp.net
      Signed-off-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
      Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
      f5076c6b
    • Oliver Hartkopp's avatar
      can: isotp: fix msg_namelen values depending on CAN_REQUIRED_SIZE · f522d955
      Oliver Hartkopp authored
      Since commit f5223e9e ("can: extend sockaddr_can to include j1939
      members") the sockaddr_can has been extended in size and a new
      CAN_REQUIRED_SIZE macro has been introduced to calculate the protocol
      specific needed size.
      
      The ABI for the msg_name and msg_namelen has not been adapted to the
      new CAN_REQUIRED_SIZE macro for the other CAN protocols which leads to
      a problem when an existing binary reads the (increased) struct
      sockaddr_can in msg_name.
      
      Fixes: e057dd3f
      
       ("can: add ISO 15765-2:2016 transport protocol")
      Reported-by: default avatarRichard Weinberger <richard@nod.at>
      Acked-by: default avatarKurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
      Link: https://lore.kernel.org/linux-can/1135648123.112255.1616613706554.JavaMail.zimbra@nod.at/T/#t
      Link: https://lore.kernel.org/r/20210325125850.1620-2-socketcan@hartkopp.net
      Signed-off-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
      Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
      f522d955
    • Oliver Hartkopp's avatar
      can: bcm/raw: fix msg_namelen values depending on CAN_REQUIRED_SIZE · 9e971474
      Oliver Hartkopp authored
      Since commit f5223e9e ("can: extend sockaddr_can to include j1939
      members") the sockaddr_can has been extended in size and a new
      CAN_REQUIRED_SIZE macro has been introduced to calculate the protocol
      specific needed size.
      
      The ABI for the msg_name and msg_namelen has not been adapted to the
      new CAN_REQUIRED_SIZE macro for the other CAN protocols which leads to
      a problem when an existing binary reads the (increased) struct
      sockaddr_can in msg_name.
      
      Fixes: f5223e9e
      
       ("can: extend sockaddr_can to include j1939 members")
      Reported-by: default avatarRichard Weinberger <richard@nod.at>
      Tested-by: default avatarRichard Weinberger <richard@nod.at>
      Acked-by: default avatarKurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
      Link: https://lore.kernel.org/linux-can/1135648123.112255.1616613706554.JavaMail.zimbra@nod.at/T/#t
      Link: https://lore.kernel.org/r/20210325125850.1620-1-socketcan@hartkopp.net
      Signed-off-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
      Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
      9e971474
    • Lv Yunlong's avatar
      drivers/net/wan/hdlc_fr: Fix a double free in pvc_xmit · 1b479fb8
      Lv Yunlong authored
      In pvc_xmit, if __skb_pad(skb, pad, false) failed, it will free
      the skb in the first time and goto drop. But the same skb is freed
      by kfree_skb(skb) in the second time in drop.
      
      Maintaining the original function unchanged, my patch adds a new
      label out to avoid the double free if __skb_pad() failed.
      
      Fixes: f5083d0c
      
       ("drivers/net/wan/hdlc_fr: Improvements to the code of pvc_xmit")
      Signed-off-by: default avatarLv Yunlong <lyl2019@mail.ustc.edu.cn>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1b479fb8