1. 05 Jul, 2020 6 commits
  2. 03 Jul, 2020 1 commit
    • Toke Høiland-Jørgensen's avatar
      sched: consistently handle layer3 header accesses in the presence of VLANs · d7bf2ebe
      Toke Høiland-Jørgensen authored
      There are a couple of places in net/sched/ that check skb->protocol and act
      on the value there. However, in the presence of VLAN tags, the value stored
      in skb->protocol can be inconsistent based on whether VLAN acceleration is
      enabled. The commit quoted in the Fixes tag below fixed the users of
      skb->protocol to use a helper that will always see the VLAN ethertype.
      
      However, most of the callers don't actually handle the VLAN ethertype, but
      expect to find the IP header type in the protocol field. This means that
      things like changing the ECN field, or parsing diffserv values, stops
      working if there's a VLAN tag, or if there are multiple nested VLAN
      tags (QinQ).
      
      To fix this, change the helper to take an argument that indicates whether
      the caller wants to skip the VLAN tags or not. When skipping VLAN tags, we
      make sure to skip all of them, so behaviour is consistent even in QinQ
      mode.
      
      To make the helper usable from the ECN code, move it to if_vlan.h instead
      of pkt_sched.h.
      
      v3:
      - Remove empty lines
      - Move vlan variable definitions inside loop in skb_protocol()
      - Also use skb_protocol() helper in IP{,6}_ECN_decapsulate() and
        bpf_skb_ecn_set_ce()
      
      v2:
      - Use eth_type_vlan() helper in skb_protocol()
      - Also fix code that reads skb->protocol directly
      - Change a couple of 'if/else if' statements to switch constructs to avoid
        calling the helper twice
      Reported-by: default avatarIlya Ponetayev <i.ponetaev@ndmsystems.com>
      Fixes: d8b9605d ("net: sched: fix skb->protocol use in case of accelerated vlan path")
      Signed-off-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      d7bf2ebe
  3. 02 Jul, 2020 9 commits
    • Nicolas Ferre's avatar
      MAINTAINERS: net: macb: add Claudiu as co-maintainer · ad4e2b64
      Nicolas Ferre authored
      I would like that Claudiu becomes co-maintainer of the Cadence macb
      driver. He's already participating to lots of reviews and enhancements
      to this driver and knows the different versions of this controller.
      Signed-off-by: default avatarNicolas Ferre <nicolas.ferre@microchip.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ad4e2b64
    • Codrin Ciubotariu's avatar
      net: dsa: microchip: set the correct number of ports · af199a1a
      Codrin Ciubotariu authored
      The number of ports is incorrectly set to the maximum available for a DSA
      switch. Even if the extra ports are not used, this causes some functions
      to be called later, like port_disable() and port_stp_state_set(). If the
      driver doesn't check the port index, it will end up modifying unknown
      registers.
      
      Fixes: b987e98e ("dsa: add DSA switch driver for Microchip KSZ9477")
      Signed-off-by: default avatarCodrin Ciubotariu <codrin.ciubotariu@microchip.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      af199a1a
    • Eric Dumazet's avatar
      tcp: md5: allow changing MD5 keys in all socket states · 1ca0fafd
      Eric Dumazet authored
      This essentially reverts commit 72123032 ("tcp: md5: reject TCP_MD5SIG
      or TCP_MD5SIG_EXT on established sockets")
      
      Mathieu reported that many vendors BGP implementations can
      actually switch TCP MD5 on established flows.
      
      Quoting Mathieu :
         Here is a list of a few network vendors along with their behavior
         with respect to TCP MD5:
      
         - Cisco: Allows for password to be changed, but within the hold-down
           timer (~180 seconds).
         - Juniper: When password is initially set on active connection it will
           reset, but after that any subsequent password changes no network
           resets.
         - Nokia: No notes on if they flap the tcp connection or not.
         - Ericsson/RedBack: Allows for 2 password (old/new) to co-exist until
           both sides are ok with new passwords.
         - Meta-Switch: Expects the password to be set before a connection is
           attempted, but no further info on whether they reset the TCP
           connection on a change.
         - Avaya: Disable the neighbor, then set password, then re-enable.
         - Zebos: Would normally allow the change when socket connected.
      
      We can revert my prior change because commit 9424e2e7 ("tcp: md5: fix potential
      overestimation of TCP option space") removed the leak of 4 kernel bytes to
      the wire that was the main reason for my patch.
      
      While doing my investigations, I found a bug when a MD5 key is changed, leading
      to these commits that stable teams want to consider before backporting this revert :
      
       Commit 6a2febec ("tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()")
       Commit e6ced831 ("tcp: md5: refine tcp_md5_do_add()/tcp_md5_hash_key() barriers")
      
      Fixes: 72123032 "tcp: md5: reject TCP_MD5SIG or TCP_MD5SIG_EXT on established sockets"
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Reported-by: default avatarMathieu Desnoyers <mathieu.desnoyers@efficios.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1ca0fafd
    • Helmut Grohne's avatar
      net: dsa: microchip: enable ksz9893 via i2c in the ksz9477 driver · e4b9a72d
      Helmut Grohne authored
      The KSZ9893 3-Port Gigabit Ethernet Switch can be controlled via SPI,
      I²C or MDIO (very limited and not supported by this driver). While there
      is already a compatible entry for the SPI bus, it was missing for I²C.
      Signed-off-by: default avatarHelmut Grohne <helmut.grohne@intenta.de>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e4b9a72d
    • Eric Dumazet's avatar
      tcp: fix SO_RCVLOWAT possible hangs under high mem pressure · ba3bb0e7
      Eric Dumazet authored
      Whenever tcp_try_rmem_schedule() returns an error, we are under
      trouble and should make sure to wakeup readers so that they
      can drain socket queues and eventually make room.
      
      Fixes: 03f45c88 ("tcp: avoid extra wakeups for SO_RCVLOWAT users")
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ba3bb0e7
    • Willem de Bruijn's avatar
      ip: Fix SO_MARK in RST, ACK and ICMP packets · 0da7536f
      Willem de Bruijn authored
      When no full socket is available, skbs are sent over a per-netns
      control socket. Its sk_mark is temporarily adjusted to match that
      of the real (request or timewait) socket or to reflect an incoming
      skb, so that the outgoing skb inherits this in __ip_make_skb.
      
      Introduction of the socket cookie mark field broke this. Now the
      skb is set through the cookie and cork:
      
      <caller>		# init sockc.mark from sk_mark or cmsg
      ip_append_data
        ip_setup_cork		# convert sockc.mark to cork mark
      ip_push_pending_frames
        ip_finish_skb
          __ip_make_skb	# set skb->mark to cork mark
      
      But I missed these special control sockets. Update all callers of
      __ip(6)_make_skb that were originally missed.
      
      For IPv6, the same two icmp(v6) paths are affected. The third
      case is not, as commit 92e55f41 ("tcp: don't annotate
      mark on control socket from tcp_v6_send_response()") replaced
      the ctl_sk->sk_mark with passing the mark field directly as a
      function argument. That commit predates the commit that
      introduced the bug.
      
      Fixes: c6af0c22 ("ip: support SO_MARK cmsg")
      Signed-off-by: default avatarWillem de Bruijn <willemb@google.com>
      Reported-by: default avatarMartin KaFai Lau <kafai@fb.com>
      Reviewed-by: default avatarMartin KaFai Lau <kafai@fb.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      0da7536f
    • Eric Dumazet's avatar
      tcp: md5: do not send silly options in SYNCOOKIES · e114e1e8
      Eric Dumazet authored
      Whenever cookie_init_timestamp() has been used to encode
      ECN,SACK,WSCALE options, we can not remove the TS option in the SYNACK.
      
      Otherwise, tcp_synack_options() will still advertize options like WSCALE
      that we can not deduce later when receiving the packet from the client
      to complete 3WHS.
      
      Note that modern linux TCP stacks wont use MD5+TS+SACK in a SYN packet,
      but we can not know for sure that all TCP stacks have the same logic.
      
      Before the fix a tcpdump would exhibit this wrong exchange :
      
      10:12:15.464591 IP C > S: Flags [S], seq 4202415601, win 65535, options [nop,nop,md5 valid,mss 1400,sackOK,TS val 456965269 ecr 0,nop,wscale 8], length 0
      10:12:15.464602 IP S > C: Flags [S.], seq 253516766, ack 4202415602, win 65535, options [nop,nop,md5 valid,mss 1400,nop,nop,sackOK,nop,wscale 8], length 0
      10:12:15.464611 IP C > S: Flags [.], ack 1, win 256, options [nop,nop,md5 valid], length 0
      10:12:15.464678 IP C > S: Flags [P.], seq 1:13, ack 1, win 256, options [nop,nop,md5 valid], length 12
      10:12:15.464685 IP S > C: Flags [.], ack 13, win 65535, options [nop,nop,md5 valid], length 0
      
      After this patch the exchange looks saner :
      
      11:59:59.882990 IP C > S: Flags [S], seq 517075944, win 65535, options [nop,nop,md5 valid,mss 1400,sackOK,TS val 1751508483 ecr 0,nop,wscale 8], length 0
      11:59:59.883002 IP S > C: Flags [S.], seq 1902939253, ack 517075945, win 65535, options [nop,nop,md5 valid,mss 1400,sackOK,TS val 1751508479 ecr 1751508483,nop,wscale 8], length 0
      11:59:59.883012 IP C > S: Flags [.], ack 1, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508483 ecr 1751508479], length 0
      11:59:59.883114 IP C > S: Flags [P.], seq 1:13, ack 1, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508483 ecr 1751508479], length 12
      11:59:59.883122 IP S > C: Flags [.], ack 13, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508483 ecr 1751508483], length 0
      11:59:59.883152 IP S > C: Flags [P.], seq 1:13, ack 13, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508484 ecr 1751508483], length 12
      11:59:59.883170 IP C > S: Flags [.], ack 13, win 256, options [nop,nop,md5 valid,nop,nop,TS val 1751508484 ecr 1751508484], length 0
      
      Of course, no SACK block will ever be added later, but nothing should break.
      Technically, we could remove the 4 nops included in MD5+TS options,
      but again some stacks could break seeing not conventional alignment.
      
      Fixes: 4957faad ("TCPCT part 1g: Responder Cookie => Initiator")
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Cc: Florian Westphal <fw@strlen.de>
      Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e114e1e8
    • Rao Shoaib's avatar
      rds: If one path needs re-connection, check all and re-connect · 9ef845f8
      Rao Shoaib authored
      In testing with mprds enabled, Oracle Cluster nodes after reboot were
      not able to communicate with others nodes and so failed to rejoin
      the cluster. Peers with lower IP address initiated connection but the
      node could not respond as it choose a different path and could not
      initiate a connection as it had a higher IP address.
      
      With this patch, when a node sends out a packet and the selected path
      is down, all other paths are also checked and any down paths are
      re-connected.
      Reviewed-by: default avatarKa-cheong Poon <ka-cheong.poon@oracle.com>
      Reviewed-by: default avatarDavid Edmondson <david.edmondson@oracle.com>
      Signed-off-by: default avatarSomasundaram Krishnasamy <somasundaram.krishnasamy@oracle.com>
      Signed-off-by: default avatarRao Shoaib <rao.shoaib@oracle.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9ef845f8
    • Eric Dumazet's avatar
      tcp: md5: refine tcp_md5_do_add()/tcp_md5_hash_key() barriers · e6ced831
      Eric Dumazet authored
      My prior fix went a bit too far, according to Herbert and Mathieu.
      
      Since we accept that concurrent TCP MD5 lookups might see inconsistent
      keys, we can use READ_ONCE()/WRITE_ONCE() instead of smp_rmb()/smp_wmb()
      
      Clearing all key->key[] is needed to avoid possible KMSAN reports,
      if key->keylen is increased. Since tcp_md5_do_add() is not fast path,
      using __GFP_ZERO to clear all struct tcp_md5sig_key is simpler.
      
      data_race() was added in linux-5.8 and will prevent KCSAN reports,
      this can safely be removed in stable backports, if data_race() is
      not yet backported.
      
      v2: use data_race() both in tcp_md5_hash_key() and tcp_md5_do_add()
      
      Fixes: 6a2febec ("tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key()")
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: Marco Elver <elver@google.com>
      Reviewed-by: default avatarMathieu Desnoyers <mathieu.desnoyers@efficios.com>
      Acked-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e6ced831
  4. 01 Jul, 2020 4 commits
    • Sean Tranchetti's avatar
      genetlink: remove genl_bind · 1e82a62f
      Sean Tranchetti authored
      A potential deadlock can occur during registering or unregistering a
      new generic netlink family between the main nl_table_lock and the
      cb_lock where each thread wants the lock held by the other, as
      demonstrated below.
      
      1) Thread 1 is performing a netlink_bind() operation on a socket. As part
         of this call, it will call netlink_lock_table(), incrementing the
         nl_table_users count to 1.
      2) Thread 2 is registering (or unregistering) a genl_family via the
         genl_(un)register_family() API. The cb_lock semaphore will be taken for
         writing.
      3) Thread 1 will call genl_bind() as part of the bind operation to handle
         subscribing to GENL multicast groups at the request of the user. It will
         attempt to take the cb_lock semaphore for reading, but it will fail and
         be scheduled away, waiting for Thread 2 to finish the write.
      4) Thread 2 will call netlink_table_grab() during the (un)registration
         call. However, as Thread 1 has incremented nl_table_users, it will not
         be able to proceed, and both threads will be stuck waiting for the
         other.
      
      genl_bind() is a noop, unless a genl_family implements the mcast_bind()
      function to handle setting up family-specific multicast operations. Since
      no one in-tree uses this functionality as Cong pointed out, simply removing
      the genl_bind() function will remove the possibility for deadlock, as there
      is no attempt by Thread 1 above to take the cb_lock semaphore.
      
      Fixes: c380d9a7 ("genetlink: pass multicast bind/unbind to families")
      Suggested-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
      Acked-by: default avatarJohannes Berg <johannes.berg@intel.com>
      Reported-by: default avatarkernel test robot <lkp@intel.com>
      Signed-off-by: default avatarSean Tranchetti <stranche@codeaurora.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1e82a62f
    • Luo bin's avatar
      hinic: fix passing non negative value to ERR_PTR · d3c54f7f
      Luo bin authored
      get_dev_cap and set_resources_state functions may return a positive
      value because of hardware failure, and the positive return value
      can not be passed to ERR_PTR directly.
      
      Fixes: 7dd29ee1 ("hinic: add sriov feature support")
      Signed-off-by: default avatarLuo bin <luobin9@huawei.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      d3c54f7f
    • Dan Carpenter's avatar
      net: qrtr: Fix an out of bounds read qrtr_endpoint_post() · 8ff41cc2
      Dan Carpenter authored
      This code assumes that the user passed in enough data for a
      qrtr_hdr_v1 or qrtr_hdr_v2 struct, but it's not necessarily true.  If
      the buffer is too small then it will read beyond the end.
      Reported-by: default avatarManivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
      Reported-by: syzbot+b8fe393f999a291a9ea6@syzkaller.appspotmail.com
      Fixes: 194ccc88 ("net: qrtr: Support decoding incoming v2 packets")
      Signed-off-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      8ff41cc2
    • Eric Dumazet's avatar
      tcp: md5: add missing memory barriers in tcp_md5_do_add()/tcp_md5_hash_key() · 6a2febec
      Eric Dumazet authored
      MD5 keys are read with RCU protection, and tcp_md5_do_add()
      might update in-place a prior key.
      
      Normally, typical RCU updates would allocate a new piece
      of memory. In this case only key->key and key->keylen might
      be updated, and we do not care if an incoming packet could
      see the old key, the new one, or some intermediate value,
      since changing the key on a live flow is known to be problematic
      anyway.
      
      We only want to make sure that in the case key->keylen
      is changed, cpus in tcp_md5_hash_key() wont try to use
      uninitialized data, or crash because key->keylen was
      read twice to feed sg_init_one() and ahash_request_set_crypt()
      
      Fixes: 9ea88a15 ("tcp: md5: check md5 signature without socket lock")
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6a2febec
  5. 30 Jun, 2020 20 commits
    • Carl Huang's avatar
      net: qrtr: free flow in __qrtr_node_release · 28541f3d
      Carl Huang authored
      The flow is allocated in qrtr_tx_wait, but not freed when qrtr node
      is released. (*slot) becomes NULL after radix_tree_iter_delete is
      called in __qrtr_node_release. The fix is to save (*slot) to a
      vairable and then free it.
      
      This memory leak is catched when kmemleak is enabled in kernel,
      the report looks like below:
      
      unreferenced object 0xffffa0de69e08420 (size 32):
        comm "kworker/u16:3", pid 176, jiffies 4294918275 (age 82858.876s)
        hex dump (first 32 bytes):
          00 00 00 00 00 00 00 00 28 84 e0 69 de a0 ff ff  ........(..i....
          28 84 e0 69 de a0 ff ff 03 00 00 00 00 00 00 00  (..i............
        backtrace:
          [<00000000e252af0a>] qrtr_node_enqueue+0x38e/0x400 [qrtr]
          [<000000009cea437f>] qrtr_sendmsg+0x1e0/0x2a0 [qrtr]
          [<000000008bddbba4>] sock_sendmsg+0x5b/0x60
          [<0000000003beb43a>] qmi_send_message.isra.3+0xbe/0x110 [qmi_helpers]
          [<000000009c9ae7de>] qmi_send_request+0x1c/0x20 [qmi_helpers]
      Signed-off-by: default avatarCarl Huang <cjhuang@codeaurora.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      28541f3d
    • Li Heng's avatar
      net: cxgb4: fix return error value in t4_prep_fw · 8a259e6b
      Li Heng authored
      t4_prep_fw goto bye tag with positive return value when something
      bad happened and which can not free resource in adap_init0.
      so fix it to return negative value.
      
      Fixes: 16e47624 ("cxgb4: Add new scheme to update T4/T5 firmware")
      Reported-by: default avatarHulk Robot <hulkci@huawei.com>
      Signed-off-by: default avatarLi Heng <liheng40@huawei.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      8a259e6b
    • David S. Miller's avatar
      Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf · e708e2bd
      David S. Miller authored
      Daniel Borkmann says:
      
      ====================
      pull-request: bpf 2020-06-30
      
      The following pull-request contains BPF updates for your *net* tree.
      
      We've added 28 non-merge commits during the last 9 day(s) which contain
      a total of 35 files changed, 486 insertions(+), 232 deletions(-).
      
      The main changes are:
      
      1) Fix an incorrect verifier branch elimination for PTR_TO_BTF_ID pointer
         types, from Yonghong Song.
      
      2) Fix UAPI for sockmap and flow_dissector progs that were ignoring various
         arguments passed to BPF_PROG_{ATTACH,DETACH}, from Lorenz Bauer & Jakub Sitnicki.
      
      3) Fix broken AF_XDP DMA hacks that are poking into dma-direct and swiotlb
         internals and integrate it properly into DMA core, from Christoph Hellwig.
      
      4) Fix RCU splat from recent changes to avoid skipping ingress policy when
         kTLS is enabled, from John Fastabend.
      
      5) Fix BPF ringbuf map to enforce size to be the power of 2 in order for its
         position masking to work, from Andrii Nakryiko.
      
      6) Fix regression from CAP_BPF work to re-allow CAP_SYS_ADMIN for loading
         of network programs, from Maciej Żenczykowski.
      
      7) Fix libbpf section name prefix for devmap progs, from Jesper Dangaard Brouer.
      
      8) Fix formatting in UAPI documentation for BPF helpers, from Quentin Monnet.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e708e2bd
    • Yonghong Song's avatar
      bpf: Add tests for PTR_TO_BTF_ID vs. null comparison · d923021c
      Yonghong Song authored
      Add two tests for PTR_TO_BTF_ID vs. null ptr comparison,
      one for PTR_TO_BTF_ID in the ctx structure and the
      other for PTR_TO_BTF_ID after one level pointer chasing.
      In both cases, the test ensures condition is not
      removed.
      
      For example, for this test
       struct bpf_fentry_test_t {
           struct bpf_fentry_test_t *a;
       };
       int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
       {
           if (arg == 0)
               test7_result = 1;
           return 0;
       }
      Before the previous verifier change, we have xlated codes:
        int test7(long long unsigned int * ctx):
        ; int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
           0: (79) r1 = *(u64 *)(r1 +0)
        ; int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
           1: (b4) w0 = 0
           2: (95) exit
      After the previous verifier change, we have:
        int test7(long long unsigned int * ctx):
        ; int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
           0: (79) r1 = *(u64 *)(r1 +0)
        ; if (arg == 0)
           1: (55) if r1 != 0x0 goto pc+4
        ; test7_result = 1;
           2: (18) r1 = map[id:6][0]+48
           4: (b7) r2 = 1
           5: (7b) *(u64 *)(r1 +0) = r2
        ; int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
           6: (b4) w0 = 0
           7: (95) exit
      Signed-off-by: default avatarYonghong Song <yhs@fb.com>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
      Acked-by: default avatarAndrii Nakryiko <andriin@fb.com>
      Link: https://lore.kernel.org/bpf/20200630171241.2523875-1-yhs@fb.com
      d923021c
    • Yonghong Song's avatar
      bpf: Fix an incorrect branch elimination by verifier · 01c66c48
      Yonghong Song authored
      Wenbo reported an issue in [1] where a checking of null
      pointer is evaluated as always false. In this particular
      case, the program type is tp_btf and the pointer to
      compare is a PTR_TO_BTF_ID.
      
      The current verifier considers PTR_TO_BTF_ID always
      reprents a non-null pointer, hence all PTR_TO_BTF_ID compares
      to 0 will be evaluated as always not-equal, which resulted
      in the branch elimination.
      
      For example,
       struct bpf_fentry_test_t {
           struct bpf_fentry_test_t *a;
       };
       int BPF_PROG(test7, struct bpf_fentry_test_t *arg)
       {
           if (arg == 0)
               test7_result = 1;
           return 0;
       }
       int BPF_PROG(test8, struct bpf_fentry_test_t *arg)
       {
           if (arg->a == 0)
               test8_result = 1;
           return 0;
       }
      
      In above bpf programs, both branch arg == 0 and arg->a == 0
      are removed. This may not be what developer expected.
      
      The bug is introduced by Commit cac616db ("bpf: Verifier
      track null pointer branch_taken with JNE and JEQ"),
      where PTR_TO_BTF_ID is considered to be non-null when evaluting
      pointer vs. scalar comparison. This may be added
      considering we have PTR_TO_BTF_ID_OR_NULL in the verifier
      as well.
      
      PTR_TO_BTF_ID_OR_NULL is added to explicitly requires
      a non-NULL testing in selective cases. The current generic
      pointer tracing framework in verifier always
      assigns PTR_TO_BTF_ID so users does not need to
      check NULL pointer at every pointer level like a->b->c->d.
      
      We may not want to assign every PTR_TO_BTF_ID as
      PTR_TO_BTF_ID_OR_NULL as this will require a null test
      before pointer dereference which may cause inconvenience
      for developers. But we could avoid branch elimination
      to preserve original code intention.
      
      This patch simply removed PTR_TO_BTD_ID from reg_type_not_null()
      in verifier, which prevented the above branches from being eliminated.
      
       [1]: https://lore.kernel.org/bpf/79dbb7c0-449d-83eb-5f4f-7af0cc269168@fb.com/T/
      
      Fixes: cac616db ("bpf: Verifier track null pointer branch_taken with JNE and JEQ")
      Reported-by: default avatarWenbo Zhang <ethercflow@gmail.com>
      Signed-off-by: default avatarYonghong Song <yhs@fb.com>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Acked-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
      Acked-by: default avatarAndrii Nakryiko <andriin@fb.com>
      Link: https://lore.kernel.org/bpf/20200630171240.2523722-1-yhs@fb.com
      01c66c48
    • David S. Miller's avatar
      Merge branch 'net-ipa-three-bug-fixes' · 0433c93d
      David S. Miller authored
      Alex Elder says:
      
      ====================
      net: ipa: three bug fixes
      
      This series contains three bug fixes for the Qualcomm IPA driver.
      In practice these bugs are unlikke.y to be harmful, but they do
      represent incorrect code.
      
      Version 2 adds "Fixes" tags to two of the patches and fixes a typo
      in one (found by checkpatch.pl).
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      0433c93d
    • Alex Elder's avatar
      net: ipa: introduce ipa_cmd_tag_process() · 6cb63ea6
      Alex Elder authored
      Create a new function ipa_cmd_tag_process() that simply allocates a
      transaction, adds a tag process command to it to clear the hardware
      pipeline, and commits the transaction.
      
      Call it in from ipa_endpoint_suspend(), after suspending the modem
      endpoints but before suspending the AP command TX and AP LAN RX
      endpoints (which are used by the tag sequence).
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6cb63ea6
    • Alex Elder's avatar
      net: ipa: no checksum offload for SDM845 LAN RX · 41af5436
      Alex Elder authored
      The AP LAN RX endpoint should not have download checksum offload
      enabled.
      
      The receive handler does properly accommodate the trailer that's
      added by the hardware, but we ignore it.
      
      Fixes: 1ed7d0c0 ("soc: qcom: ipa: configuration data")
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      41af5436
    • Alex Elder's avatar
      net: ipa: always check for stopped channel · 5468cbcd
      Alex Elder authored
      In gsi_channel_stop(), there's a check to see if the channel might
      have entered STOPPED state since a previous call, which might have
      timed out before stopping completed.
      
      That check actually belongs in gsi_channel_stop_command(), which is
      called repeatedly by gsi_channel_stop() for RX channels.
      
      Fixes: 650d1603 ("soc: qcom: ipa: the generic software interface")
      Signed-off-by: default avatarAlex Elder <elder@linaro.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      5468cbcd
    • Russell King's avatar
      net: mvneta: fix use of state->speed · f2ca673d
      Russell King authored
      When support for short preambles was added, it incorrectly keyed its
      decision off state->speed instead of state->interface.  state->speed
      is not guaranteed to be correct for in-band modes, which can lead to
      short preambles being unexpectedly disabled.
      
      Fix this by keying off the interface mode, which is the only way that
      mvneta can operate at 2.5Gbps.
      
      Fixes: da58a931 ("net: mvneta: Add support for 2500Mbps SGMII")
      Signed-off-by: default avatarRussell King <rmk+kernel@armlinux.org.uk>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f2ca673d
    • David S. Miller's avatar
      Merge branch 'support-AF_PACKET-for-layer-3-devices' · b9fcf0a0
      David S. Miller authored
      Jason A. Donenfeld says:
      
      ====================
      support AF_PACKET for layer 3 devices
      
      Hans reported that packets injected by a correct-looking and trivial
      libpcap-based program were not being accepted by wireguard. In
      investigating that, I noticed that a few devices weren't properly
      handling AF_PACKET-injected packets, and so this series introduces a bit
      of shared infrastructure to support that.
      
      The basic problem begins with socket(AF_PACKET, SOCK_RAW,
      htons(ETH_P_ALL)) sockets. When sendto is called, AF_PACKET examines the
      headers of the packet with this logic:
      
      static void packet_parse_headers(struct sk_buff *skb, struct socket *sock)
      {
          if ((!skb->protocol || skb->protocol == htons(ETH_P_ALL)) &&
              sock->type == SOCK_RAW) {
              skb_reset_mac_header(skb);
              skb->protocol = dev_parse_header_protocol(skb);
          }
      
          skb_probe_transport_header(skb);
      }
      
      The middle condition there triggers, and we jump to
      dev_parse_header_protocol. Note that this is the only caller of
      dev_parse_header_protocol in the kernel, and I assume it was designed
      for this purpose:
      
      static inline __be16 dev_parse_header_protocol(const struct sk_buff *skb)
      {
          const struct net_device *dev = skb->dev;
      
          if (!dev->header_ops || !dev->header_ops->parse_protocol)
              return 0;
          return dev->header_ops->parse_protocol(skb);
      }
      
      Since AF_PACKET already knows which netdev the packet is going to, the
      dev_parse_header_protocol function can see if that netdev has a way it
      prefers to figure out the protocol from the header. This, again, is the
      only use of parse_protocol in the kernel. At the moment, it's only used
      with ethernet devices, via eth_header_parse_protocol. This makes sense,
      as mostly people are used to AF_PACKET-injecting ethernet frames rather
      than layer 3 frames. But with nothing in place for layer 3 netdevs, this
      function winds up returning 0, and skb->protocol then is set to 0, and
      then by the time it hits the netdev's ndo_start_xmit, the driver doesn't
      know what to do with it.
      
      This is a problem because drivers very much rely on skb->protocol being
      correct, and routinely reject packets where it's incorrect. That's why
      having this parsing happen for injected packets is quite important. In
      wireguard, ipip, and ipip6, for example, packets from AF_PACKET are just
      dropped entirely. For tun devices, it's sort of uglier, with the tun
      "packet information" header being passed to userspace containing a bogus
      protocol value. Some userspace programs are ill-equipped to deal with
      that. (But of course, that doesn't happen with tap devices, which
      benefit from the similar shared infrastructure for layer 2 netdevs,
      further motiviating this patchset for layer 3 netdevs.)
      
      This patchset addresses the issue by first adding a layer 3 header parse
      function, much akin to the existing one for layer 2 packets, and then
      adds a shared header_ops structure that, also much akin to the existing
      one for layer 2 packets. Then it wires it up to a few immediate places
      that stuck out as requiring it, and does a bit of cleanup.
      
      This patchset seems like it's fixing real bugs, so it might be
      appropriate for stable. But they're also very old bugs, so if you'd
      rather not backport to stable, that'd make sense to me too.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      b9fcf0a0
    • Jason A. Donenfeld's avatar
      net: xfrmi: implement header_ops->parse_protocol for AF_PACKET · 8f9a1fa4
      Jason A. Donenfeld authored
      The xfrm interface uses skb->protocol to determine packet type, and
      bails out if it's not set. For AF_PACKET injection, we need to support
      its call chain of:
      
          packet_sendmsg -> packet_snd -> packet_parse_headers ->
            dev_parse_header_protocol -> parse_protocol
      
      Without a valid parse_protocol, this returns zero, and xfrmi rejects the
      skb. So, this wires up the ip_tunnel handler for layer 3 packets for
      that case.
      Reported-by: default avatarWillem de Bruijn <willemdebruijn.kernel@gmail.com>
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      8f9a1fa4
    • Jason A. Donenfeld's avatar
      net: sit: implement header_ops->parse_protocol for AF_PACKET · 75ea1f47
      Jason A. Donenfeld authored
      Sit uses skb->protocol to determine packet type, and bails out if it's
      not set. For AF_PACKET injection, we need to support its call chain of:
      
          packet_sendmsg -> packet_snd -> packet_parse_headers ->
            dev_parse_header_protocol -> parse_protocol
      
      Without a valid parse_protocol, this returns zero, and sit rejects the
      skb. So, this wires up the ip_tunnel handler for layer 3 packets for
      that case.
      Reported-by: default avatarWillem de Bruijn <willemdebruijn.kernel@gmail.com>
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      75ea1f47
    • Jason A. Donenfeld's avatar
      net: vti: implement header_ops->parse_protocol for AF_PACKET · ab59d2b6
      Jason A. Donenfeld authored
      Vti uses skb->protocol to determine packet type, and bails out if it's
      not set. For AF_PACKET injection, we need to support its call chain of:
      
          packet_sendmsg -> packet_snd -> packet_parse_headers ->
            dev_parse_header_protocol -> parse_protocol
      
      Without a valid parse_protocol, this returns zero, and vti rejects the
      skb. So, this wires up the ip_tunnel handler for layer 3 packets for
      that case.
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ab59d2b6
    • Jason A. Donenfeld's avatar
      tun: implement header_ops->parse_protocol for AF_PACKET · b9815eb1
      Jason A. Donenfeld authored
      The tun driver passes up skb->protocol to userspace in the form of PI headers.
      For AF_PACKET injection, we need to support its call chain of:
      
          packet_sendmsg -> packet_snd -> packet_parse_headers ->
            dev_parse_header_protocol -> parse_protocol
      
      Without a valid parse_protocol, this returns zero, and the tun driver
      then gives userspace bogus values that it can't deal with.
      
      Note that this isn't the case with tap, because tap already benefits
      from the shared infrastructure for ethernet headers. But with tun,
      there's nothing.
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      b9815eb1
    • Jason A. Donenfeld's avatar
      wireguard: queueing: make use of ip_tunnel_parse_protocol · 1a574074
      Jason A. Donenfeld authored
      Now that wg_examine_packet_protocol has been added for general
      consumption as ip_tunnel_parse_protocol, it's possible to remove
      wg_examine_packet_protocol and simply use the new
      ip_tunnel_parse_protocol function directly.
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1a574074
    • Jason A. Donenfeld's avatar
      wireguard: implement header_ops->parse_protocol for AF_PACKET · 01a4967c
      Jason A. Donenfeld authored
      WireGuard uses skb->protocol to determine packet type, and bails out if
      it's not set or set to something it's not expecting. For AF_PACKET
      injection, we need to support its call chain of:
      
          packet_sendmsg -> packet_snd -> packet_parse_headers ->
            dev_parse_header_protocol -> parse_protocol
      
      Without a valid parse_protocol, this returns zero, and wireguard then
      rejects the skb. So, this wires up the ip_tunnel handler for layer 3
      packets for that case.
      Reported-by: default avatarHans Wippel <ndev@hwipl.net>
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      01a4967c
    • Jason A. Donenfeld's avatar
      net: ipip: implement header_ops->parse_protocol for AF_PACKET · e53ac932
      Jason A. Donenfeld authored
      Ipip uses skb->protocol to determine packet type, and bails out if it's
      not set. For AF_PACKET injection, we need to support its call chain of:
      
          packet_sendmsg -> packet_snd -> packet_parse_headers ->
            dev_parse_header_protocol -> parse_protocol
      
      Without a valid parse_protocol, this returns zero, and ipip rejects the
      skb. So, this wires up the ip_tunnel handler for layer 3 packets for
      that case.
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Acked-by: default avatarWillem de Bruijn <willemb@google.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e53ac932
    • Jason A. Donenfeld's avatar
      net: ip_tunnel: add header_ops for layer 3 devices · 2606aff9
      Jason A. Donenfeld authored
      Some devices that take straight up layer 3 packets benefit from having a
      shared header_ops so that AF_PACKET sockets can inject packets that are
      recognized. This shared infrastructure will be used by other drivers
      that currently can't inject packets using AF_PACKET. It also exposes the
      parser function, as it is useful in standalone form too.
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Acked-by: default avatarWillem de Bruijn <willemb@google.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      2606aff9
    • Jakub Sitnicki's avatar
      bpf, netns: Fix use-after-free in pernet pre_exit callback · 2576f870
      Jakub Sitnicki authored
      Iterating over BPF links attached to network namespace in pre_exit hook is
      not safe, even if there is just one. Once link gets auto-detached, that is
      its back-pointer to net object is set to NULL, the link can be released and
      freed without waiting on netns_bpf_mutex, effectively causing the list
      element we are operating on to be freed.
      
      This leads to use-after-free when trying to access the next element on the
      list, as reported by KASAN. Bug can be triggered by destroying a network
      namespace, while also releasing a link attached to this network namespace.
      
      | ==================================================================
      | BUG: KASAN: use-after-free in netns_bpf_pernet_pre_exit+0xd9/0x130
      | Read of size 8 at addr ffff888119e0d778 by task kworker/u8:2/177
      |
      | CPU: 3 PID: 177 Comm: kworker/u8:2 Not tainted 5.8.0-rc1-00197-ga0c04c9d1008-dirty #776
      | Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20190727_073836-buildvm-ppc64le-16.ppc.fedoraproject.org-3.fc31 04/01/2014
      | Workqueue: netns cleanup_net
      | Call Trace:
      |  dump_stack+0x9e/0xe0
      |  print_address_description.constprop.0+0x3a/0x60
      |  ? netns_bpf_pernet_pre_exit+0xd9/0x130
      |  kasan_report.cold+0x1f/0x40
      |  ? netns_bpf_pernet_pre_exit+0xd9/0x130
      |  netns_bpf_pernet_pre_exit+0xd9/0x130
      |  cleanup_net+0x30b/0x5b0
      |  ? unregister_pernet_device+0x50/0x50
      |  ? rcu_read_lock_bh_held+0xb0/0xb0
      |  ? _raw_spin_unlock_irq+0x24/0x50
      |  process_one_work+0x4d1/0xa10
      |  ? lock_release+0x3e0/0x3e0
      |  ? pwq_dec_nr_in_flight+0x110/0x110
      |  ? rwlock_bug.part.0+0x60/0x60
      |  worker_thread+0x7a/0x5c0
      |  ? process_one_work+0xa10/0xa10
      |  kthread+0x1e3/0x240
      |  ? kthread_create_on_node+0xd0/0xd0
      |  ret_from_fork+0x1f/0x30
      |
      | Allocated by task 280:
      |  save_stack+0x1b/0x40
      |  __kasan_kmalloc.constprop.0+0xc2/0xd0
      |  netns_bpf_link_create+0xfe/0x650
      |  __do_sys_bpf+0x153a/0x2a50
      |  do_syscall_64+0x59/0x300
      |  entry_SYSCALL_64_after_hwframe+0x44/0xa9
      |
      | Freed by task 198:
      |  save_stack+0x1b/0x40
      |  __kasan_slab_free+0x12f/0x180
      |  kfree+0xed/0x350
      |  process_one_work+0x4d1/0xa10
      |  worker_thread+0x7a/0x5c0
      |  kthread+0x1e3/0x240
      |  ret_from_fork+0x1f/0x30
      |
      | The buggy address belongs to the object at ffff888119e0d700
      |  which belongs to the cache kmalloc-192 of size 192
      | The buggy address is located 120 bytes inside of
      |  192-byte region [ffff888119e0d700, ffff888119e0d7c0)
      | The buggy address belongs to the page:
      | page:ffffea0004678340 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0
      | flags: 0x2fffe0000000200(slab)
      | raw: 02fffe0000000200 ffffea00045ba8c0 0000000600000006 ffff88811a80ea80
      | raw: 0000000000000000 0000000000100010 00000001ffffffff 0000000000000000
      | page dumped because: kasan: bad access detected
      |
      | Memory state around the buggy address:
      |  ffff888119e0d600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
      |  ffff888119e0d680: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
      | >ffff888119e0d700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
      |                                                                 ^
      |  ffff888119e0d780: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
      |  ffff888119e0d800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
      | ==================================================================
      
      Remove the "fast-path" for releasing a link that got auto-detached by a
      dying network namespace to fix it. This way as long as link is on the list
      and netns_bpf mutex is held, we have a guarantee that link memory can be
      accessed.
      
      An alternative way to fix this issue would be to safely iterate over the
      list of links and ensure there is no access to link object after detaching
      it. But, at the moment, optimizing synchronization overhead on link release
      without a workload in mind seems like an overkill.
      
      Fixes: ab53cad9 ("bpf, netns: Keep a list of attached bpf_link's")
      Signed-off-by: default avatarJakub Sitnicki <jakub@cloudflare.com>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Acked-by: default avatarYonghong Song <yhs@fb.com>
      Link: https://lore.kernel.org/bpf/20200630164541.1329993-1-jakub@cloudflare.com
      2576f870