Re: [PATCH net v1] tipc: fix use-after-free of broadcast reception link at node release

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message contradicts itself about runtime evidence.
- [Low] The commit message says the issue was found with a Coccinelle
  rule and 'LLM-assisted (GLM-5.3) code inspection', but the trailer…

--

tipc: fix use-after-free of broadcast reception link at node release

This patch moves the kfree() of n->bc_entry.link out of
tipc_node_kref_release() and into the tipc_node_free() RCU callback. As a
result, the broadcast reception link is freed together with the enclosing
tipc_node, after a grace period. Netlink walkers holding rcu_read_lock()
can then no longer reach freed link memory.

> Reproduced in a QEMU/KVM guest running v7.3.0 with KASAN and
> PREEMPT(full): 10 TIPC peer namespaces on veth bearers, bearer removal
> staggered so the 300 s node cleanup timers expire while other tasks
> loop link dumps with the broadcast filter and reset-link-stats lookups.
> KASAN hit within two cleanup cycles:
>
>   BUG: KASAN: slab-use-after-free in strlen+0x85/0xa0

[ ... ]

> with the freed object being the kmalloc-1k tipc_link allocated by
> tipc_link_bc_create() during peer discovery.  The same defect fires in
> strcmp() called from tipc_nl_node_reset_link_stats().

[ ... ]

> Found during a static-analysis-assisted audit: candidate sites were
> surfaced by a Coccinelle rule, and the reader/writer paths were
> confirmed by LLM-assisted (GLM-5.3) code inspection.  The issue was
> not observed at runtime; it is identified by code inspection only.

[Severity: Medium]
These two paragraphs seem to contradict each other. Which one is accurate?

The earlier paragraph describes a QEMU/KVM reproduction. It includes a
KASAN slab-use-after-free splat from:

  tipc_nl_node_dump_link()
    __tipc_nl_add_node_links()
      tipc_nl_add_bc_link()
        strlen()

with the object freed from tipc_node_timeout() in timer softirq context. It
also says the same defect fires in strcmp() called from
tipc_nl_node_reset_link_stats().

The closing paragraph says the issue was not observed at runtime and was
found by code inspection only.

If the reproduction and the KASAN log were really captured, could the "not
observed at runtime" sentence be dropped? If the issue was found by
inspection only, could the reproduction paragraph and the splat be removed?

> Fixes: d25a01257e42 ("tipc: fix crash during node removal")
> Signed-off-by: Binbin Deng <18983559317@163.com>

[Severity: Low]
This isn't a bug, but the message says a Coccinelle rule and GLM-5.3 were
used, and there is no Assisted-by: trailer.

Documentation/process/coding-assistants.rst asks for one in this format:

  Assisted-by: LLM [TOOL1] [TOOL2]

The "Using Assisted-by:" section of
Documentation/process/submitting-patches.rst says:

  If you used any sort of advanced coding tool in the creation of your
  patch, you need to acknowledge that use by adding an Assisted-by tag.
  Failure to do so may impede the acceptance of your work.

Could a trailer such as "Assisted-by: GLM-5.3 coccinelle" be added?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015041435.33561-1-18983559317%40163.com