1. 21 Apr, 2023 9 commits
    • Vladimir Oltean's avatar
      net: enetc: only commit preemptible TCs to hardware when MM TX is active · 82714539
      Vladimir Oltean authored
      This was left as TODO in commit 01e23b2b ("net: enetc: add support
      for preemptible traffic classes") since it's relatively complicated.
      
      Where this makes a difference is with a configuration as follows:
      
      ethtool --set-mm eno0 pmac-enabled on tx-enabled on verify-enabled on
      
      Preemptible packets should only be sent when the MAC Merge TX direction
      becomes active (i.o.w. when the verification process succeeds, aka when
      the link partner confirms it can process preemptible traffic). But the
      tc qdisc with the preemptible traffic classes is offloaded completely
      asynchronously w.r.t. the MM becoming active.
      
      The ENETC manual does suggest that this should be handled in the driver:
      "On startup, software should wait for the verification process to
      complete (MMCSR[VSTS]=011) before initiating traffic".
      
      Adding the necessary logic allows future selftests to uphold the claim
      that an inactive or disabled MAC Merge layer should never send data
      packets through the pMAC.
      
      This change moves enetc_set_ptcfpr() from enetc.c to enetc_ethtool.c,
      where its only caller is now - enetc_mm_commit_preemptible_tcs().
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      82714539
    • Vladimir Oltean's avatar
      net: enetc: report mm tx-active based on tx-enabled and verify-status · 153b5b1d
      Vladimir Oltean authored
      The MMCSR register contains 2 fields with overlapping meaning:
      
      - LPA (Local preemption active):
      This read-only status bit indicates whether preemption is active for
      this port. This bit will be set if preemption is both enabled and has
      completed the verification process.
      - TXSTS (Merge status):
      This read-only status field provides the state of the MAC Merge sublayer
      transmit status as defined in IEEE Std 802.3-2018 Clause 99.
      00 Transmit preemption is inactive
      01 Transmit preemption is active
      10 Reserved
      11 Reserved
      
      However none of these 2 fields offer reliable reporting to software.
      
      When connecting ENETC to a link partner which is not capable of Frame
      Preemption, the expectation is that ENETC's verification should fail
      (VSTS=4) and its MM TX direction should be inactive (LPA=0, TXSTS=00)
      even though the MM TX is enabled (ME=1). But surprise, the LPA bit of
      MMCSR stays set even if VSTS=4 and ME=1.
      
      OTOH, the TXSTS field has the opposite problem. I cannot get its value
      to change from 0, even when connecting to a link partner capable of
      frame preemption, which does respond to its verification frames (ME=1
      and VSTS=3, "SUCCEEDED").
      
      The only option with such buggy hardware seems to be to reimplement the
      formula for calculating tx-active in software, which is for tx-enabled
      to be true, and for the verify-status to be either SUCCEEDED, or
      DISABLED.
      
      Without reliable tx-active reporting, we have no good indication when
      to commit the preemptible traffic classes to hardware, which makes it
      possible (but not desirable) to send preemptible traffic to a link
      partner incapable of receiving it. However, currently we do not have the
      logic to wait for TX to be active yet, so the impact is limited.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      153b5b1d
    • Vladimir Oltean's avatar
      net: enetc: fix MAC Merge layer remaining enabled until a link down event · 59be75db
      Vladimir Oltean authored
      Current enetc_set_mm() is designed to set the priv->active_offloads bit
      ENETC_F_QBU for enetc_mm_link_state_update() to act on, but if the link
      is already up, it modifies the ENETC_MMCSR_ME ("Merge Enable") bit
      directly.
      
      The problem is that it only *sets* ENETC_MMCSR_ME if the link is up, it
      doesn't *clear* it if needed. So subsequent enetc_get_mm() calls still
      see tx-enabled as true, up until a link down event, which is when
      enetc_mm_link_state_update() will get called.
      
      This is not a functional issue as far as I can assess. It has only come
      up because I'd like to uphold a simple API rule in core ethtool code:
      the pMAC cannot be disabled if TX is going to be enabled. Currently,
      the fact that TX remains enabled for longer than expected (after the
      enetc_set_mm() call that disables it) is going to violate that rule,
      which is how it was caught.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      59be75db
    • Slark Xiao's avatar
      wwan: core: add print for wwan port attach/disconnect · 787e6144
      Slark Xiao authored
      Refer to USB serial device or net device, there is a notice to
      let end user know the status of device, like attached or
      disconnected. Add attach/disconnect print for wwan device as
      well.
      Signed-off-by: default avatarSlark Xiao <slark_xiao@163.com>
      Reviewed-by: default avatarLoic Poulain <loic.poulain@linaro.org>
      Link: https://lore.kernel.org/r/20230420023617.3919569-1-slark_xiao@163.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      787e6144
    • Jakub Kicinski's avatar
      net: skbuff: update and rename __kfree_skb_defer() · 8fa66e4a
      Jakub Kicinski authored
      __kfree_skb_defer() uses the old naming where "defer" meant
      slab bulk free/alloc APIs. In the meantime we also made
      __kfree_skb_defer() feed the per-NAPI skb cache, which
      implies bulk APIs. So take away the 'defer' and add 'napi'.
      
      While at it add a drop reason. This only matters on the
      tx_action path, if the skb has a frag_list. But getting
      rid of a SKB_DROP_REASON_NOT_SPECIFIED seems like a net
      benefit so why not.
      Reviewed-by: default avatarAlexander Lobakin <aleksander.lobakin@intel.com>
      Link: https://lore.kernel.org/r/20230420020005.815854-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8fa66e4a
    • Jakub Kicinski's avatar
      eth: mlx5: avoid iterator use outside of a loop · 61718206
      Jakub Kicinski authored
      Fix the following warning about risky iterator use:
      
      drivers/net/ethernet/mellanox/mlx5/core/eq.c:1010 mlx5_comp_irq_get_affinity_mask() warn: iterator used outside loop: 'eq'
      Acked-by: default avatarSaeed Mahameed <saeed@kernel.org>
      Link: https://lore.kernel.org/r/20230420015802.815362-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      61718206
    • Simon Horman's avatar
      flow_dissector: Address kdoc warnings · 8c966a10
      Simon Horman authored
      Address a number of warnings flagged by
      ./scripts/kernel-doc -none include/net/flow_dissector.h
      
       include/net/flow_dissector.h:23: warning: Function parameter or member 'addr_type' not described in 'flow_dissector_key_control'
       include/net/flow_dissector.h:23: warning: Function parameter or member 'flags' not described in 'flow_dissector_key_control'
       include/net/flow_dissector.h:46: warning: Function parameter or member 'padding' not described in 'flow_dissector_key_basic'
       include/net/flow_dissector.h:145: warning: Function parameter or member 'tipckey' not described in 'flow_dissector_key_addrs'
       include/net/flow_dissector.h:157: warning: cannot understand function prototype: 'struct flow_dissector_key_arp '
       include/net/flow_dissector.h:171: warning: cannot understand function prototype: 'struct flow_dissector_key_ports '
       include/net/flow_dissector.h:203: warning: cannot understand function prototype: 'struct flow_dissector_key_icmp '
      
      Also improve indentation on adjacent lines to those changed
      to address the above.
      
      No functional changes intended.
      Signed-off-by: default avatarSimon Horman <horms@kernel.org>
      Link: https://lore.kernel.org/r/20230419-flow-dissector-kdoc-v1-1-1aa0cca1118b@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8c966a10
    • Jakub Kicinski's avatar
      page_pool: unlink from napi during destroy · dd64b232
      Jakub Kicinski authored
      Jesper points out that we must prevent recycling into cache
      after page_pool_destroy() is called, because page_pool_destroy()
      is not synchronized with recycling (some pages may still be
      outstanding when destroy() gets called).
      
      I assumed this will not happen because NAPI can't be scheduled
      if its page pool is being destroyed. But I missed the fact that
      NAPI may get reused. For instance when user changes ring configuration
      driver may allocate a new page pool, stop NAPI, swap, start NAPI,
      and then destroy the old pool. The NAPI is running so old page
      pool will think it can recycle to the cache, but the consumer
      at that point is the destroy() path, not NAPI.
      
      To avoid extra synchronization let the drivers do "unlinking"
      during the "swap" stage while NAPI is indeed disabled.
      
      Fixes: 8c48eea3 ("page_pool: allow caching from safely localized NAPI")
      Reported-by: default avatarJesper Dangaard Brouer <jbrouer@redhat.com>
      Link: https://lore.kernel.org/all/e8df2654-6a5b-3c92-489d-2fe5e444135f@redhat.com/Acked-by: default avatarJesper Dangaard Brouer <brouer@redhat.com>
      Link: https://lore.kernel.org/r/20230419182006.719923-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      dd64b232
    • Arnd Bergmann's avatar
      net: phy: fix circular LEDS_CLASS dependencies · 4bb7aac7
      Arnd Bergmann authored
      The CONFIG_PHYLIB symbol is selected by a number of device drivers that
      need PHY support, but it now has a dependency on CONFIG_LEDS_CLASS,
      which may not be enabled, causing build failures.
      
      Avoid the risk of missing and circular dependencies by guarding the
      phylib LED support itself in another Kconfig symbol that can only be
      enabled if the dependency is met.
      
      This could be made a hidden symbol and always enabled when both CONFIG_OF
      and CONFIG_LEDS_CLASS are reachable from the phylib, but there may be an
      advantage in having users see this option when they have a misconfigured
      kernel without built-in LED support.
      
      Fixes: 01e5b728 ("net: phy: Add a binding for PHY LEDs")
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Link: https://lore.kernel.org/r/20230420084624.3005701-1-arnd@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      4bb7aac7
  2. 20 Apr, 2023 24 commits
  3. 19 Apr, 2023 7 commits
    • Linus Torvalds's avatar
      Merge tag 'spi-fix-v6.3-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi · 23990b1a
      Linus Torvalds authored
      Pull spi fix from Mark Brown:
       "A small fix in the error handling for the rockchip driver, ensuring we
        don't leak clock enables if we fail to request the interrupt for the
        device"
      
      * tag 'spi-fix-v6.3-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi:
        spi: spi-rockchip: Fix missing unwind goto in rockchip_sfc_probe()
      23990b1a
    • Linus Torvalds's avatar
      Merge tag 'regulator-fix-v6.3-rc7' of... · 72b4fb4c
      Linus Torvalds authored
      Merge tag 'regulator-fix-v6.3-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
      
      Pull regulator fixes from Mark Brown:
       "A few driver specific fixes, one build coverage issue and a couple of
        'someone typed in the wrong number' style errors in describing devices
        to the subsystem"
      
      * tag 'regulator-fix-v6.3-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator:
        regulator: sm5703: Fix missing n_voltages for fixed regulators
        regulator: fan53555: Fix wrong TCS_SLEW_MASK
        regulator: fan53555: Explicitly include bits header
      72b4fb4c
    • Christophe JAILLET's avatar
      net: dsa: microchip: ksz8795: Correctly handle huge frame configuration · 3d2f8f1f
      Christophe JAILLET authored
      Because of the logic in place, SW_HUGE_PACKET can never be set.
      (If the first condition is true, then the 2nd one is also true, but is not
      executed)
      
      Change the logic and update each bit individually.
      
      Fixes: 29d1e85f ("net: dsa: microchip: ksz8: add MTU configuration support")
      Signed-off-by: default avatarChristophe JAILLET <christophe.jaillet@wanadoo.fr>
      Reviewed-by: default avatarOleksij Rempel <o.rempel@pengutronix.de>
      Reviewed-by: default avatarSimon Horman <simon.horman@corigine.com>
      Reviewed-by: default avatarVladimir Oltean <olteanv@gmail.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Link: https://lore.kernel.org/r/43107d9e8b5b8b05f0cbd4e1f47a2bb88c8747b2.1681755535.git.christophe.jaillet@wanadoo.frSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      3d2f8f1f
    • Jakub Kicinski's avatar
      page_pool: add DMA_ATTR_WEAK_ORDERING on all mappings · 8e4c62c7
      Jakub Kicinski authored
      Commit c519fe9a ("bnxt: add dma mapping attributes") added
      DMA_ATTR_WEAK_ORDERING to DMA attrs on bnxt. It has since spread
      to a few more drivers (possibly as a copy'n'paste).
      
      DMA_ATTR_WEAK_ORDERING only seems to matter on Sparc and PowerPC/cell,
      the rarity of these platforms is likely why we never bothered adding
      the attribute in the page pool, even though it should be safe to add.
      
      To make the page pool migration in drivers which set this flag less
      of a risk (of regressing the precious sparc database workloads or
      whatever needed this) let's add DMA_ATTR_WEAK_ORDERING on all
      page pool DMA mappings.
      
      We could make this a driver opt-in but frankly I don't think it's
      worth complicating the API. I can't think of a reason why device
      accesses to packet memory would have to be ordered.
      Acked-by: default avatarIlias Apalodimas <ilias.apalodimas@linaro.org>
      Acked-by: default avatarSomnath Kotur <somnath.kotur@broadcom.com>
      Reviewed-by: default avatarAlexander Lobakin <aleksander.lobakin@intel.com>
      Acked-by: default avatarJesper Dangaard Brouer <brouer@redhat.com>
      Link: https://lore.kernel.org/r/20230417152805.331865-1-kuba@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8e4c62c7
    • Andrea Righi's avatar
      rust: allow to use INIT_STACK_ALL_ZERO · d966c3ca
      Andrea Righi authored
      With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
      -ftrivial-auto-var-init=zero to clang, that triggers the following
      error:
      
       error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'
      
      However, this additional option that is currently required by clang is
      deprecated since clang-16 and going to be removed in the future,
      likely with clang-18.
      
      So, make sure bindgen is using this extra option if the major version of
      the libclang used by bindgen is < 16.
      
      In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
      without triggering any build error.
      
      Link: https://github.com/llvm/llvm-project/issues/44842
      Link: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flagsSigned-off-by: default avatarAndrea Righi <andrea.righi@canonical.com>
      Reviewed-by: default avatarKees Cook <keescook@chromium.org>
      [Changed to < 16, added link and reworded]
      Signed-off-by: default avatarMiguel Ojeda <ojeda@kernel.org>
      d966c3ca
    • Andrea Righi's avatar
      rust: fix regexp in scripts/is_rust_module.sh · ccc45054
      Andrea Righi authored
      nm can use "R" or "r" to show read-only data sections, but
      scripts/is_rust_module.sh can only recognize "r", so with some versions
      of binutils it can fail to detect if a module is a Rust module or not.
      
      Right now we're using this script only to determine if we need to skip
      BTF generation (that is disabled globally if CONFIG_RUST is enabled),
      but it's still nice to fix this script to do the proper job.
      
      Moreover, with this patch applied I can also relax the constraint of
      "RUST depends on !DEBUG_INFO_BTF" and build a kernel with Rust and BTF
      enabled at the same time (of course BTF generation is still skipped for
      Rust modules).
      
      [ Miguel: The actual reason is likely to be a change on the Rust
        compiler between 1.61.0 and 1.62.0:
      
          echo '#[used] static S: () = ();' |
              rustup run 1.61.0 rustc --emit=obj --crate-type=lib - &&
              nm rust_out.o
      
          echo '#[used] static S: () = ();' |
              rustup run 1.62.0 rustc --emit=obj --crate-type=lib - &&
              nm rust_out.o
      
        Gives:
      
          0000000000000000 r _ZN8rust_out1S17h48027ce0da975467E
          0000000000000000 R _ZN8rust_out1S17h58e1f3d9c0e97cefE
      
        See https://godbolt.org/z/KE6jneoo4. ]
      Signed-off-by: default avatarAndrea Righi <andrea.righi@canonical.com>
      Reviewed-by: default avatarVincenzo Palazzo <vincenzopalazzodev@gmail.com>
      Reviewed-by: default avatarEric Curtin <ecurtin@redhat.com>
      Reviewed-by: default avatarMartin Rodriguez Reboredo <yakoyoku@gmail.com>
      Signed-off-by: default avatarMiguel Ojeda <ojeda@kernel.org>
      ccc45054
    • Daniel Borkmann's avatar
      bpf: Fix incorrect verifier pruning due to missing register precision taints · 71b547f5
      Daniel Borkmann authored
      Juan Jose et al reported an issue found via fuzzing where the verifier's
      pruning logic prematurely marks a program path as safe.
      
      Consider the following program:
      
         0: (b7) r6 = 1024
         1: (b7) r7 = 0
         2: (b7) r8 = 0
         3: (b7) r9 = -2147483648
         4: (97) r6 %= 1025
         5: (05) goto pc+0
         6: (bd) if r6 <= r9 goto pc+2
         7: (97) r6 %= 1
         8: (b7) r9 = 0
         9: (bd) if r6 <= r9 goto pc+1
        10: (b7) r6 = 0
        11: (b7) r0 = 0
        12: (63) *(u32 *)(r10 -4) = r0
        13: (18) r4 = 0xffff888103693400 // map_ptr(ks=4,vs=48)
        15: (bf) r1 = r4
        16: (bf) r2 = r10
        17: (07) r2 += -4
        18: (85) call bpf_map_lookup_elem#1
        19: (55) if r0 != 0x0 goto pc+1
        20: (95) exit
        21: (77) r6 >>= 10
        22: (27) r6 *= 8192
        23: (bf) r1 = r0
        24: (0f) r0 += r6
        25: (79) r3 = *(u64 *)(r0 +0)
        26: (7b) *(u64 *)(r1 +0) = r3
        27: (95) exit
      
      The verifier treats this as safe, leading to oob read/write access due
      to an incorrect verifier conclusion:
      
        func#0 @0
        0: R1=ctx(off=0,imm=0) R10=fp0
        0: (b7) r6 = 1024                     ; R6_w=1024
        1: (b7) r7 = 0                        ; R7_w=0
        2: (b7) r8 = 0                        ; R8_w=0
        3: (b7) r9 = -2147483648              ; R9_w=-2147483648
        4: (97) r6 %= 1025                    ; R6_w=scalar()
        5: (05) goto pc+0
        6: (bd) if r6 <= r9 goto pc+2         ; R6_w=scalar(umin=18446744071562067969,var_off=(0xffffffff00000000; 0xffffffff)) R9_w=-2147483648
        7: (97) r6 %= 1                       ; R6_w=scalar()
        8: (b7) r9 = 0                        ; R9=0
        9: (bd) if r6 <= r9 goto pc+1         ; R6=scalar(umin=1) R9=0
        10: (b7) r6 = 0                       ; R6_w=0
        11: (b7) r0 = 0                       ; R0_w=0
        12: (63) *(u32 *)(r10 -4) = r0
        last_idx 12 first_idx 9
        regs=1 stack=0 before 11: (b7) r0 = 0
        13: R0_w=0 R10=fp0 fp-8=0000????
        13: (18) r4 = 0xffff8ad3886c2a00      ; R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        15: (bf) r1 = r4                      ; R1_w=map_ptr(off=0,ks=4,vs=48,imm=0) R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        16: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
        17: (07) r2 += -4                     ; R2_w=fp-4
        18: (85) call bpf_map_lookup_elem#1   ; R0=map_value_or_null(id=1,off=0,ks=4,vs=48,imm=0)
        19: (55) if r0 != 0x0 goto pc+1       ; R0=0
        20: (95) exit
      
        from 19 to 21: R0=map_value(off=0,ks=4,vs=48,imm=0) R6=0 R7=0 R8=0 R9=0 R10=fp0 fp-8=mmmm????
        21: (77) r6 >>= 10                    ; R6_w=0
        22: (27) r6 *= 8192                   ; R6_w=0
        23: (bf) r1 = r0                      ; R0=map_value(off=0,ks=4,vs=48,imm=0) R1_w=map_value(off=0,ks=4,vs=48,imm=0)
        24: (0f) r0 += r6
        last_idx 24 first_idx 19
        regs=40 stack=0 before 23: (bf) r1 = r0
        regs=40 stack=0 before 22: (27) r6 *= 8192
        regs=40 stack=0 before 21: (77) r6 >>= 10
        regs=40 stack=0 before 19: (55) if r0 != 0x0 goto pc+1
        parent didn't have regs=40 stack=0 marks: R0_rw=map_value_or_null(id=1,off=0,ks=4,vs=48,imm=0) R6_rw=P0 R7=0 R8=0 R9=0 R10=fp0 fp-8=mmmm????
        last_idx 18 first_idx 9
        regs=40 stack=0 before 18: (85) call bpf_map_lookup_elem#1
        regs=40 stack=0 before 17: (07) r2 += -4
        regs=40 stack=0 before 16: (bf) r2 = r10
        regs=40 stack=0 before 15: (bf) r1 = r4
        regs=40 stack=0 before 13: (18) r4 = 0xffff8ad3886c2a00
        regs=40 stack=0 before 12: (63) *(u32 *)(r10 -4) = r0
        regs=40 stack=0 before 11: (b7) r0 = 0
        regs=40 stack=0 before 10: (b7) r6 = 0
        25: (79) r3 = *(u64 *)(r0 +0)         ; R0_w=map_value(off=0,ks=4,vs=48,imm=0) R3_w=scalar()
        26: (7b) *(u64 *)(r1 +0) = r3         ; R1_w=map_value(off=0,ks=4,vs=48,imm=0) R3_w=scalar()
        27: (95) exit
      
        from 9 to 11: R1=ctx(off=0,imm=0) R6=0 R7=0 R8=0 R9=0 R10=fp0
        11: (b7) r0 = 0                       ; R0_w=0
        12: (63) *(u32 *)(r10 -4) = r0
        last_idx 12 first_idx 11
        regs=1 stack=0 before 11: (b7) r0 = 0
        13: R0_w=0 R10=fp0 fp-8=0000????
        13: (18) r4 = 0xffff8ad3886c2a00      ; R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        15: (bf) r1 = r4                      ; R1_w=map_ptr(off=0,ks=4,vs=48,imm=0) R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        16: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
        17: (07) r2 += -4                     ; R2_w=fp-4
        18: (85) call bpf_map_lookup_elem#1
        frame 0: propagating r6
        last_idx 19 first_idx 11
        regs=40 stack=0 before 18: (85) call bpf_map_lookup_elem#1
        regs=40 stack=0 before 17: (07) r2 += -4
        regs=40 stack=0 before 16: (bf) r2 = r10
        regs=40 stack=0 before 15: (bf) r1 = r4
        regs=40 stack=0 before 13: (18) r4 = 0xffff8ad3886c2a00
        regs=40 stack=0 before 12: (63) *(u32 *)(r10 -4) = r0
        regs=40 stack=0 before 11: (b7) r0 = 0
        parent didn't have regs=40 stack=0 marks: R1=ctx(off=0,imm=0) R6_r=P0 R7=0 R8=0 R9=0 R10=fp0
        last_idx 9 first_idx 9
        regs=40 stack=0 before 9: (bd) if r6 <= r9 goto pc+1
        parent didn't have regs=40 stack=0 marks: R1=ctx(off=0,imm=0) R6_rw=Pscalar() R7_w=0 R8_w=0 R9_rw=0 R10=fp0
        last_idx 8 first_idx 0
        regs=40 stack=0 before 8: (b7) r9 = 0
        regs=40 stack=0 before 7: (97) r6 %= 1
        regs=40 stack=0 before 6: (bd) if r6 <= r9 goto pc+2
        regs=40 stack=0 before 5: (05) goto pc+0
        regs=40 stack=0 before 4: (97) r6 %= 1025
        regs=40 stack=0 before 3: (b7) r9 = -2147483648
        regs=40 stack=0 before 2: (b7) r8 = 0
        regs=40 stack=0 before 1: (b7) r7 = 0
        regs=40 stack=0 before 0: (b7) r6 = 1024
        19: safe
        frame 0: propagating r6
        last_idx 9 first_idx 0
        regs=40 stack=0 before 6: (bd) if r6 <= r9 goto pc+2
        regs=40 stack=0 before 5: (05) goto pc+0
        regs=40 stack=0 before 4: (97) r6 %= 1025
        regs=40 stack=0 before 3: (b7) r9 = -2147483648
        regs=40 stack=0 before 2: (b7) r8 = 0
        regs=40 stack=0 before 1: (b7) r7 = 0
        regs=40 stack=0 before 0: (b7) r6 = 1024
      
        from 6 to 9: safe
        verification time 110 usec
        stack depth 4
        processed 36 insns (limit 1000000) max_states_per_insn 0 total_states 3 peak_states 3 mark_read 2
      
      The verifier considers this program as safe by mistakenly pruning unsafe
      code paths. In the above func#0, code lines 0-10 are of interest. In line
      0-3 registers r6 to r9 are initialized with known scalar values. In line 4
      the register r6 is reset to an unknown scalar given the verifier does not
      track modulo operations. Due to this, the verifier can also not determine
      precisely which branches in line 6 and 9 are taken, therefore it needs to
      explore them both.
      
      As can be seen, the verifier starts with exploring the false/fall-through
      paths first. The 'from 19 to 21' path has both r6=0 and r9=0 and the pointer
      arithmetic on r0 += r6 is therefore considered safe. Given the arithmetic,
      r6 is correctly marked for precision tracking where backtracking kicks in
      where it walks back the current path all the way where r6 was set to 0 in
      the fall-through branch.
      
      Next, the pruning logics pops the path 'from 9 to 11' from the stack. Also
      here, the state of the registers is the same, that is, r6=0 and r9=0, so
      that at line 19 the path can be pruned as it is considered safe. It is
      interesting to note that the conditional in line 9 turned r6 into a more
      precise state, that is, in the fall-through path at the beginning of line
      10, it is R6=scalar(umin=1), and in the branch-taken path (which is analyzed
      here) at the beginning of line 11, r6 turned into a known const r6=0 as
      r9=0 prior to that and therefore (unsigned) r6 <= 0 concludes that r6 must
      be 0 (**):
      
        [...]                                 ; R6_w=scalar()
        9: (bd) if r6 <= r9 goto pc+1         ; R6=scalar(umin=1) R9=0
        [...]
      
        from 9 to 11: R1=ctx(off=0,imm=0) R6=0 R7=0 R8=0 R9=0 R10=fp0
        [...]
      
      The next path is 'from 6 to 9'. The verifier considers the old and current
      state equivalent, and therefore prunes the search incorrectly. Looking into
      the two states which are being compared by the pruning logic at line 9, the
      old state consists of R6_rwD=Pscalar() R9_rwD=0 R10=fp0 and the new state
      consists of R1=ctx(off=0,imm=0) R6_w=scalar(umax=18446744071562067968)
      R7_w=0 R8_w=0 R9_w=-2147483648 R10=fp0. While r6 had the reg->precise flag
      correctly set in the old state, r9 did not. Both r6'es are considered as
      equivalent given the old one is a superset of the current, more precise one,
      however, r9's actual values (0 vs 0x80000000) mismatch. Given the old r9
      did not have reg->precise flag set, the verifier does not consider the
      register as contributing to the precision state of r6, and therefore it
      considered both r9 states as equivalent. However, for this specific pruned
      path (which is also the actual path taken at runtime), register r6 will be
      0x400 and r9 0x80000000 when reaching line 21, thus oob-accessing the map.
      
      The purpose of precision tracking is to initially mark registers (including
      spilled ones) as imprecise to help verifier's pruning logic finding equivalent
      states it can then prune if they don't contribute to the program's safety
      aspects. For example, if registers are used for pointer arithmetic or to pass
      constant length to a helper, then the verifier sets reg->precise flag and
      backtracks the BPF program instruction sequence and chain of verifier states
      to ensure that the given register or stack slot including their dependencies
      are marked as precisely tracked scalar. This also includes any other registers
      and slots that contribute to a tracked state of given registers/stack slot.
      This backtracking relies on recorded jmp_history and is able to traverse
      entire chain of parent states. This process ends only when all the necessary
      registers/slots and their transitive dependencies are marked as precise.
      
      The backtrack_insn() is called from the current instruction up to the first
      instruction, and its purpose is to compute a bitmask of registers and stack
      slots that need precision tracking in the parent's verifier state. For example,
      if a current instruction is r6 = r7, then r6 needs precision after this
      instruction and r7 needs precision before this instruction, that is, in the
      parent state. Hence for the latter r7 is marked and r6 unmarked.
      
      For the class of jmp/jmp32 instructions, backtrack_insn() today only looks
      at call and exit instructions and for all other conditionals the masks
      remain as-is. However, in the given situation register r6 has a dependency
      on r9 (as described above in **), so also that one needs to be marked for
      precision tracking. In other words, if an imprecise register influences a
      precise one, then the imprecise register should also be marked precise.
      Meaning, in the parent state both dest and src register need to be tracked
      for precision and therefore the marking must be more conservative by setting
      reg->precise flag for both. The precision propagation needs to cover both
      for the conditional: if the src reg was marked but not the dst reg and vice
      versa.
      
      After the fix the program is correctly rejected:
      
        func#0 @0
        0: R1=ctx(off=0,imm=0) R10=fp0
        0: (b7) r6 = 1024                     ; R6_w=1024
        1: (b7) r7 = 0                        ; R7_w=0
        2: (b7) r8 = 0                        ; R8_w=0
        3: (b7) r9 = -2147483648              ; R9_w=-2147483648
        4: (97) r6 %= 1025                    ; R6_w=scalar()
        5: (05) goto pc+0
        6: (bd) if r6 <= r9 goto pc+2         ; R6_w=scalar(umin=18446744071562067969,var_off=(0xffffffff80000000; 0x7fffffff),u32_min=-2147483648) R9_w=-2147483648
        7: (97) r6 %= 1                       ; R6_w=scalar()
        8: (b7) r9 = 0                        ; R9=0
        9: (bd) if r6 <= r9 goto pc+1         ; R6=scalar(umin=1) R9=0
        10: (b7) r6 = 0                       ; R6_w=0
        11: (b7) r0 = 0                       ; R0_w=0
        12: (63) *(u32 *)(r10 -4) = r0
        last_idx 12 first_idx 9
        regs=1 stack=0 before 11: (b7) r0 = 0
        13: R0_w=0 R10=fp0 fp-8=0000????
        13: (18) r4 = 0xffff9290dc5bfe00      ; R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        15: (bf) r1 = r4                      ; R1_w=map_ptr(off=0,ks=4,vs=48,imm=0) R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        16: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
        17: (07) r2 += -4                     ; R2_w=fp-4
        18: (85) call bpf_map_lookup_elem#1   ; R0=map_value_or_null(id=1,off=0,ks=4,vs=48,imm=0)
        19: (55) if r0 != 0x0 goto pc+1       ; R0=0
        20: (95) exit
      
        from 19 to 21: R0=map_value(off=0,ks=4,vs=48,imm=0) R6=0 R7=0 R8=0 R9=0 R10=fp0 fp-8=mmmm????
        21: (77) r6 >>= 10                    ; R6_w=0
        22: (27) r6 *= 8192                   ; R6_w=0
        23: (bf) r1 = r0                      ; R0=map_value(off=0,ks=4,vs=48,imm=0) R1_w=map_value(off=0,ks=4,vs=48,imm=0)
        24: (0f) r0 += r6
        last_idx 24 first_idx 19
        regs=40 stack=0 before 23: (bf) r1 = r0
        regs=40 stack=0 before 22: (27) r6 *= 8192
        regs=40 stack=0 before 21: (77) r6 >>= 10
        regs=40 stack=0 before 19: (55) if r0 != 0x0 goto pc+1
        parent didn't have regs=40 stack=0 marks: R0_rw=map_value_or_null(id=1,off=0,ks=4,vs=48,imm=0) R6_rw=P0 R7=0 R8=0 R9=0 R10=fp0 fp-8=mmmm????
        last_idx 18 first_idx 9
        regs=40 stack=0 before 18: (85) call bpf_map_lookup_elem#1
        regs=40 stack=0 before 17: (07) r2 += -4
        regs=40 stack=0 before 16: (bf) r2 = r10
        regs=40 stack=0 before 15: (bf) r1 = r4
        regs=40 stack=0 before 13: (18) r4 = 0xffff9290dc5bfe00
        regs=40 stack=0 before 12: (63) *(u32 *)(r10 -4) = r0
        regs=40 stack=0 before 11: (b7) r0 = 0
        regs=40 stack=0 before 10: (b7) r6 = 0
        25: (79) r3 = *(u64 *)(r0 +0)         ; R0_w=map_value(off=0,ks=4,vs=48,imm=0) R3_w=scalar()
        26: (7b) *(u64 *)(r1 +0) = r3         ; R1_w=map_value(off=0,ks=4,vs=48,imm=0) R3_w=scalar()
        27: (95) exit
      
        from 9 to 11: R1=ctx(off=0,imm=0) R6=0 R7=0 R8=0 R9=0 R10=fp0
        11: (b7) r0 = 0                       ; R0_w=0
        12: (63) *(u32 *)(r10 -4) = r0
        last_idx 12 first_idx 11
        regs=1 stack=0 before 11: (b7) r0 = 0
        13: R0_w=0 R10=fp0 fp-8=0000????
        13: (18) r4 = 0xffff9290dc5bfe00      ; R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        15: (bf) r1 = r4                      ; R1_w=map_ptr(off=0,ks=4,vs=48,imm=0) R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        16: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
        17: (07) r2 += -4                     ; R2_w=fp-4
        18: (85) call bpf_map_lookup_elem#1
        frame 0: propagating r6
        last_idx 19 first_idx 11
        regs=40 stack=0 before 18: (85) call bpf_map_lookup_elem#1
        regs=40 stack=0 before 17: (07) r2 += -4
        regs=40 stack=0 before 16: (bf) r2 = r10
        regs=40 stack=0 before 15: (bf) r1 = r4
        regs=40 stack=0 before 13: (18) r4 = 0xffff9290dc5bfe00
        regs=40 stack=0 before 12: (63) *(u32 *)(r10 -4) = r0
        regs=40 stack=0 before 11: (b7) r0 = 0
        parent didn't have regs=40 stack=0 marks: R1=ctx(off=0,imm=0) R6_r=P0 R7=0 R8=0 R9=0 R10=fp0
        last_idx 9 first_idx 9
        regs=40 stack=0 before 9: (bd) if r6 <= r9 goto pc+1
        parent didn't have regs=240 stack=0 marks: R1=ctx(off=0,imm=0) R6_rw=Pscalar() R7_w=0 R8_w=0 R9_rw=P0 R10=fp0
        last_idx 8 first_idx 0
        regs=240 stack=0 before 8: (b7) r9 = 0
        regs=40 stack=0 before 7: (97) r6 %= 1
        regs=40 stack=0 before 6: (bd) if r6 <= r9 goto pc+2
        regs=240 stack=0 before 5: (05) goto pc+0
        regs=240 stack=0 before 4: (97) r6 %= 1025
        regs=240 stack=0 before 3: (b7) r9 = -2147483648
        regs=40 stack=0 before 2: (b7) r8 = 0
        regs=40 stack=0 before 1: (b7) r7 = 0
        regs=40 stack=0 before 0: (b7) r6 = 1024
        19: safe
      
        from 6 to 9: R1=ctx(off=0,imm=0) R6_w=scalar(umax=18446744071562067968) R7_w=0 R8_w=0 R9_w=-2147483648 R10=fp0
        9: (bd) if r6 <= r9 goto pc+1
        last_idx 9 first_idx 0
        regs=40 stack=0 before 6: (bd) if r6 <= r9 goto pc+2
        regs=240 stack=0 before 5: (05) goto pc+0
        regs=240 stack=0 before 4: (97) r6 %= 1025
        regs=240 stack=0 before 3: (b7) r9 = -2147483648
        regs=40 stack=0 before 2: (b7) r8 = 0
        regs=40 stack=0 before 1: (b7) r7 = 0
        regs=40 stack=0 before 0: (b7) r6 = 1024
        last_idx 9 first_idx 0
        regs=200 stack=0 before 6: (bd) if r6 <= r9 goto pc+2
        regs=240 stack=0 before 5: (05) goto pc+0
        regs=240 stack=0 before 4: (97) r6 %= 1025
        regs=240 stack=0 before 3: (b7) r9 = -2147483648
        regs=40 stack=0 before 2: (b7) r8 = 0
        regs=40 stack=0 before 1: (b7) r7 = 0
        regs=40 stack=0 before 0: (b7) r6 = 1024
        11: R6=scalar(umax=18446744071562067968) R9=-2147483648
        11: (b7) r0 = 0                       ; R0_w=0
        12: (63) *(u32 *)(r10 -4) = r0
        last_idx 12 first_idx 11
        regs=1 stack=0 before 11: (b7) r0 = 0
        13: R0_w=0 R10=fp0 fp-8=0000????
        13: (18) r4 = 0xffff9290dc5bfe00      ; R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        15: (bf) r1 = r4                      ; R1_w=map_ptr(off=0,ks=4,vs=48,imm=0) R4_w=map_ptr(off=0,ks=4,vs=48,imm=0)
        16: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
        17: (07) r2 += -4                     ; R2_w=fp-4
        18: (85) call bpf_map_lookup_elem#1   ; R0_w=map_value_or_null(id=3,off=0,ks=4,vs=48,imm=0)
        19: (55) if r0 != 0x0 goto pc+1       ; R0_w=0
        20: (95) exit
      
        from 19 to 21: R0=map_value(off=0,ks=4,vs=48,imm=0) R6=scalar(umax=18446744071562067968) R7=0 R8=0 R9=-2147483648 R10=fp0 fp-8=mmmm????
        21: (77) r6 >>= 10                    ; R6_w=scalar(umax=18014398507384832,var_off=(0x0; 0x3fffffffffffff))
        22: (27) r6 *= 8192                   ; R6_w=scalar(smax=9223372036854767616,umax=18446744073709543424,var_off=(0x0; 0xffffffffffffe000),s32_max=2147475456,u32_max=-8192)
        23: (bf) r1 = r0                      ; R0=map_value(off=0,ks=4,vs=48,imm=0) R1_w=map_value(off=0,ks=4,vs=48,imm=0)
        24: (0f) r0 += r6
        last_idx 24 first_idx 21
        regs=40 stack=0 before 23: (bf) r1 = r0
        regs=40 stack=0 before 22: (27) r6 *= 8192
        regs=40 stack=0 before 21: (77) r6 >>= 10
        parent didn't have regs=40 stack=0 marks: R0_rw=map_value(off=0,ks=4,vs=48,imm=0) R6_r=Pscalar(umax=18446744071562067968) R7=0 R8=0 R9=-2147483648 R10=fp0 fp-8=mmmm????
        last_idx 19 first_idx 11
        regs=40 stack=0 before 19: (55) if r0 != 0x0 goto pc+1
        regs=40 stack=0 before 18: (85) call bpf_map_lookup_elem#1
        regs=40 stack=0 before 17: (07) r2 += -4
        regs=40 stack=0 before 16: (bf) r2 = r10
        regs=40 stack=0 before 15: (bf) r1 = r4
        regs=40 stack=0 before 13: (18) r4 = 0xffff9290dc5bfe00
        regs=40 stack=0 before 12: (63) *(u32 *)(r10 -4) = r0
        regs=40 stack=0 before 11: (b7) r0 = 0
        parent didn't have regs=40 stack=0 marks: R1=ctx(off=0,imm=0) R6_rw=Pscalar(umax=18446744071562067968) R7_w=0 R8_w=0 R9_w=-2147483648 R10=fp0
        last_idx 9 first_idx 0
        regs=40 stack=0 before 9: (bd) if r6 <= r9 goto pc+1
        regs=240 stack=0 before 6: (bd) if r6 <= r9 goto pc+2
        regs=240 stack=0 before 5: (05) goto pc+0
        regs=240 stack=0 before 4: (97) r6 %= 1025
        regs=240 stack=0 before 3: (b7) r9 = -2147483648
        regs=40 stack=0 before 2: (b7) r8 = 0
        regs=40 stack=0 before 1: (b7) r7 = 0
        regs=40 stack=0 before 0: (b7) r6 = 1024
        math between map_value pointer and register with unbounded min value is not allowed
        verification time 886 usec
        stack depth 4
        processed 49 insns (limit 1000000) max_states_per_insn 1 total_states 5 peak_states 5 mark_read 2
      
      Fixes: b5dc0163 ("bpf: precise scalar_value tracking")
      Reported-by: default avatarJuan Jose Lopez Jaimez <jjlopezjaimez@google.com>
      Reported-by: default avatarMeador Inge <meadori@google.com>
      Reported-by: default avatarSimon Scannell <simonscannell@google.com>
      Reported-by: default avatarNenad Stojanovski <thenenadx@google.com>
      Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
      Co-developed-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Reviewed-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
      Reviewed-by: default avatarJuan Jose Lopez Jaimez <jjlopezjaimez@google.com>
      Reviewed-by: default avatarMeador Inge <meadori@google.com>
      Reviewed-by: default avatarSimon Scannell <simonscannell@google.com>
      71b547f5