Re: [PATCH net v2 4/4] selftests: net: add devmem RX and TX netdev unregister tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests: net: test falsely assumes global dma_buf attachments drop to zero
- [Medium] selftests: net: netdevsim device leak on exception paths
--

commit 3ebc183bfb941c2f36dca38f1bd05b883cc61a30
Author: Mina Almasry <almasrymina@google.com>

selftests: net: add devmem RX and TX netdev unregister tests

Add devmem_bind_rx_unregister_check and devmem_bind_tx_unregister_check
to nl_netdev.py to verify that unregistering a netdevsim device while a
netlink socket holds an active RX or TX devmem binding synchronously
detaches the dma_buf attachment and cleanly closes the netlink socket
without use-after-free or page faults.

> diff --git a/tools/testing/selftests/net/nl_netdev.py b/tools/testing/selftests/net/nl_netdev.py
> --- a/tools/testing/selftests/net/nl_netdev.py
> +++ b/tools/testing/selftests/net/nl_netdev.py
[ ... ]
> +def _dmabuf_attached_devs():
> +    """Return attached device names from debugfs dma_buf/bufinfo if available."""
> +    path = "/sys/kernel/debug/dma_buf/bufinfo"
[ ... ]
> +    for line in text.splitlines():
> +        if line.strip() == "Attached Devices:":
> +            in_attached = True
> +            continue
> +        if in_attached:
> +            if line.startswith("\t") and line.strip():
> +                attached.append(line.strip())

[Severity: Medium]
Will this parsing approach unconditionally aggregate all attachments across all
dma_bufs in the system?

Because this does not filter the debugfs output to the specific dma_buf
created by the test, if the system executing the test has any other active
dma_bufs with attachments (e.g., from DRM graphics drivers), the global
attached count will remain non-zero.

> +            else:
> +                in_attached = False
> +    return attached
> +
> +
> +def devmem_bind_rx_unregister_check(_nf) -> None:
> +    """Verify RX devmem bindings detach dma_buf synchronously on netdev unregister."""
> +    dmabuf_fd = _create_udmabuf()
> +    nf_priv = NetdevFamily()
> +    ef = EthtoolFamily()
> +    nsimdev = NetdevSimDev(queue_count=2)
> +    nsim = nsimdev.nsims[0]

[Severity: Medium]
If an exception occurs during the test, will this leak the netdevsim device?

The netdevsim device is instantiated without registering a teardown deferral
like defer(nsimdev.remove) or using a try/finally block. If any step before
the manual nsimdev.remove() call raises an exception, the cleanup is
bypassed. This permanently leaks the kernel netdevsim device and its sysfs
entries in the kernel namespace, potentially causing subsequent tests to fail.

[ ... ]
> +    attached = _dmabuf_attached_devs()
> +    if attached is not None:
> +        ksft_ge(len(attached), 1)
> +
> +    nsimdev.remove()
> +
> +    attached = _dmabuf_attached_devs()
> +    if attached is not None:
> +        ksft_eq(len(attached), 0)

[Severity: Medium]
Can this assertion spuriously fail if other dma_bufs exist system-wide?

Since _dmabuf_attached_devs() aggregates all attachments without filtering
for the specific dma_buf created by the test, the attached count will not
drop to zero if the system has other active dma_bufs, causing the test to
fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012174300.2338784-1-almasrymina@google.com?part=4