1. 02 Jul, 2020 7 commits
    • 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
  2. 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
  3. 30 Jun, 2020 29 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
    • Alexei Starovoitov's avatar
      Merge branch 'fix-sockmap-flow_dissector-uapi' · 084af57c
      Alexei Starovoitov authored
      Lorenz Bauer says:
      
      ====================
      Both sockmap and flow_dissector ingnore various arguments passed to
      BPF_PROG_ATTACH and BPF_PROG_DETACH. We can fix the attach case by
      checking that the unused arguments are zero. I considered requiring
      target_fd to be -1 instead of 0, but this leads to a lot of churn
      in selftests. There is also precedent in that bpf_iter already
      expects 0 for a similar field. I think that we can come up with a
      work around for fd 0 should we need to in the future.
      
      The detach case is more problematic: both cgroups and lirc2 verify
      that attach_bpf_fd matches the currently attached program. This
      way you need access to the program fd to be able to remove it.
      Neither sockmap nor flow_dissector do this. flow_dissector even
      has a check for CAP_NET_ADMIN because of this. The patch set
      addresses this by implementing the desired behaviour.
      
      There is a possibility for user space breakage: any callers that
      don't provide the correct fd will fail with ENOENT. For sockmap
      the risk is low: even the selftests assume that sockmap works
      the way I described. For flow_dissector the story is less
      straightforward, and the selftests use a variety of arguments.
      
      I've includes fixes tags for the oldest commits that allow an easy
      backport, however the behaviour dates back to when sockmap and
      flow_dissector were introduced. What is the best way to handle these?
      
      This set is based on top of Jakub's work "bpf, netns: Prepare
      for multi-prog attachment" available at
      https://lore.kernel.org/bpf/87k0zwmhtb.fsf@cloudflare.com/T/
      
      Since v1:
      - Adjust selftests
      - Implement detach behaviour
      ====================
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      084af57c
    • Lorenz Bauer's avatar
      selftests: bpf: Pass program to bpf_prog_detach in flow_dissector · 1a1ad3c2
      Lorenz Bauer authored
      Calling bpf_prog_detach is incorrect, since it takes target_fd as
      its argument. The intention here is to pass it as attach_bpf_fd,
      so use bpf_prog_detach2 and pass zero for target_fd.
      
      Fixes: 06716e04 ("selftests/bpf: Extend test_flow_dissector to cover link creation")
      Signed-off-by: default avatarLorenz Bauer <lmb@cloudflare.com>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200629095630.7933-7-lmb@cloudflare.com
      1a1ad3c2
    • Lorenz Bauer's avatar
      selftests: bpf: Pass program and target_fd in flow_dissector_reattach · 0434296c
      Lorenz Bauer authored
      Pass 0 as target_fd when attaching and detaching flow dissector.
      Additionally, pass the expected program when detaching.
      
      Fixes: 1f043f87 ("selftests/bpf: Add tests for attaching bpf_link to netns")
      Signed-off-by: default avatarLorenz Bauer <lmb@cloudflare.com>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200629095630.7933-6-lmb@cloudflare.com
      0434296c
    • Lorenz Bauer's avatar
      bpf: sockmap: Require attach_bpf_fd when detaching a program · bb0de313
      Lorenz Bauer authored
      The sockmap code currently ignores the value of attach_bpf_fd when
      detaching a program. This is contrary to the usual behaviour of
      checking that attach_bpf_fd represents the currently attached
      program.
      
      Ensure that attach_bpf_fd is indeed the currently attached
      program. It turns out that all sockmap selftests already do this,
      which indicates that this is unlikely to cause breakage.
      
      Fixes: 604326b4 ("bpf, sockmap: convert to generic sk_msg interface")
      Signed-off-by: default avatarLorenz Bauer <lmb@cloudflare.com>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200629095630.7933-5-lmb@cloudflare.com
      bb0de313
    • Lorenz Bauer's avatar
      bpf: sockmap: Check value of unused args to BPF_PROG_ATTACH · 9b2b0971
      Lorenz Bauer authored
      Using BPF_PROG_ATTACH on a sockmap program currently understands no
      flags or replace_bpf_fd, but accepts any value. Return EINVAL instead.
      
      Fixes: 604326b4 ("bpf, sockmap: convert to generic sk_msg interface")
      Signed-off-by: default avatarLorenz Bauer <lmb@cloudflare.com>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200629095630.7933-4-lmb@cloudflare.com
      9b2b0971
    • Lorenz Bauer's avatar
      bpf: flow_dissector: Check value of unused flags to BPF_PROG_DETACH · 4ac2add6
      Lorenz Bauer authored
      Using BPF_PROG_DETACH on a flow dissector program supports neither
      attach_flags nor attach_bpf_fd. Yet no value is enforced for them.
      
      Enforce that attach_flags are zero, and require the current program
      to be passed via attach_bpf_fd. This allows us to remove the check
      for CAP_SYS_ADMIN, since userspace can now no longer remove
      arbitrary flow dissector programs.
      
      Fixes: b27f7bb5 ("flow_dissector: Move out netns_bpf prog callbacks")
      Signed-off-by: default avatarLorenz Bauer <lmb@cloudflare.com>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200629095630.7933-3-lmb@cloudflare.com
      4ac2add6
    • Lorenz Bauer's avatar
      bpf: flow_dissector: Check value of unused flags to BPF_PROG_ATTACH · 1b514239
      Lorenz Bauer authored
      Using BPF_PROG_ATTACH on a flow dissector program supports neither
      target_fd, attach_flags or replace_bpf_fd but accepts any value.
      
      Enforce that all of them are zero. This is fine for replace_bpf_fd
      since its presence is indicated by BPF_F_REPLACE. It's more
      problematic for target_fd, since zero is a valid fd. Should we
      want to use the flag later on we'd have to add an exception for
      fd 0. The alternative is to force a value like -1. This requires
      more changes to tests. There is also precedent for using 0,
      since bpf_iter uses this for target_fd as well.
      
      Fixes: b27f7bb5 ("flow_dissector: Move out netns_bpf prog callbacks")
      Signed-off-by: default avatarLorenz Bauer <lmb@cloudflare.com>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Link: https://lore.kernel.org/bpf/20200629095630.7933-2-lmb@cloudflare.com
      1b514239
    • Alexei Starovoitov's avatar
      Merge branch 'bpf-multi-prog-prep' · 951f38cf
      Alexei Starovoitov authored
      Jakub Sitnicki says:
      
      ====================
      This patch set prepares ground for link-based multi-prog attachment for
      future netns attach types, with BPF_SK_LOOKUP attach type in mind [0].
      
      Two changes are needed in order to attach and run a series of BPF programs:
      
        1) an bpf_prog_array of programs to run (patch #2), and
        2) a list of attached links to keep track of attachments (patch #3).
      
      Nothing changes for BPF flow_dissector. Just as before only one program can
      be attached to netns.
      
      In v3 I've simplified patch #2 that introduces bpf_prog_array to take
      advantage of the fact that it will hold at most one program for now.
      
      In particular, I'm no longer using bpf_prog_array_copy. It turned out to be
      less suitable for link operations than I thought as it fails to append the
      same BPF program.
      
      bpf_prog_array_replace_item is also gone, because we know we always want to
      replace the first element in prog_array.
      
      Naturally the code that handles bpf_prog_array will need change once
      more when there is a program type that allows multi-prog attachment. But I
      feel it will be better to do it gradually and present it together with
      tests that actually exercise multi-prog code paths.
      
      [0] https://lore.kernel.org/bpf/20200511185218.1422406-1-jakub@cloudflare.com/
      
      v2 -> v3:
      - Don't check if run_array is null in link update callback. (Martin)
      - Allow updating the link with the same BPF program. (Andrii)
      - Add patch #4 with a test for the above case.
      - Kill bpf_prog_array_replace_item. Access the run_array directly.
      - Switch from bpf_prog_array_copy() to bpf_prog_array_alloc(1, ...).
      - Replace rcu_deref_protected & RCU_INIT_POINTER with rcu_replace_pointer.
      - Drop Andrii's Ack from patch #2. Code changed.
      
      v1 -> v2:
      
      - Show with a (void) cast that bpf_prog_array_replace_item() return value
        is ignored on purpose. (Andrii)
      - Explain why bpf-cgroup cannot replace programs in bpf_prog_array based
        on bpf_prog pointer comparison in patch #2 description. (Andrii)
      ====================
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      951f38cf
    • Jakub Sitnicki's avatar
      selftests/bpf: Test updating flow_dissector link with same program · 6ebb85c8
      Jakub Sitnicki authored
      This case, while not particularly useful, is worth covering because we
      expect the operation to succeed as opposed when re-attaching the same
      program directly with PROG_ATTACH.
      
      While at it, update the tests summary that fell out of sync when tests
      extended to cover links.
      Signed-off-by: default avatarJakub Sitnicki <jakub@cloudflare.com>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Acked-by: default avatarMartin KaFai Lau <kafai@fb.com>
      Link: https://lore.kernel.org/bpf/20200625141357.910330-5-jakub@cloudflare.com
      6ebb85c8