Re: [PATCH 08/20] net: xilinx: tsn: deliver endpoint RX frames to DSA user ports

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use-After-Free due to unconditionally freeing a shared metadata dst that can be refcounted by the network stack.
- [Medium] API Violation: Queuing an SKB with a `noref` dst asynchronously via `__netif_rx()` allows it to escape the RCU read-side critical section.
--

--- Patch [8]: [PATCH 08/20] net: xilinx: tsn: deliver endpoint RX frames to DSA user ports ---
commit 0c90a3d99f1bcc8cd94ff937c6565b0da39afbe4
Author: Nagadheeraj Rottela <[email protected]>
Date:   Fri Aug 7 10:44:19 2026 +0000

    net: xilinx: tsn: deliver endpoint RX frames to DSA user ports
    
    The DSA core routes an RX frame to a user port from the port metadata
    attached to the skb. Without that metadata, frames from the two MACs
    cannot reach their swpN netdevs.
    
    Allocate a METADATA_HW_PORT_MUX entry per MAC port at probe and attach
    the matching entry to each RX frame by its TUSER port_id. Frames from
    MAC1 and MAC2 now reach the correct user netdev, which completes the
    conduit data path.

> diff --git a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
> index 7615d27f3b360..051285b47ddbd 100644
> --- a/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c
> +++ b/drivers/net/ethernet/xilinx/tsn/xilinx_tsn_ep.c

[ ... ]

> @@ -254,6 +262,7 @@ static void ep_dma_rx_cb(void *data, const struct dmaengine_result *result)
>  	}
>  
>  	skb_put(skb, rx_len);
> +	skb_dst_set_noref(skb, &ep->port_md[port_id]->dst);
>  	skb->dev = ndev;
>  	skb->protocol = eth_type_trans(skb, ndev);
>  	skb->ip_summed = CHECKSUM_NONE;

[Severity: High]
Can this cause a use-after-free if DSA is not configured?

The metadata dst is attached unconditionally to every packet here. Because there
is no check for netdev_uses_dsa(), non-DSA packets carry this dst into the IP
stack, exposing it to skb_dst_force().

If the packet is bridged or queued to userspace (e.g., NF_QUEUE), the stack
will call skb_dst_force() to upgrade the noref dst to a refcounted one. If the
driver is unloaded and frees the metadata dst unconditionally, any packet still
in flight will trigger a use-after-free when dst_release() is called.

Should this check netdev_uses_dsa() before attaching the dst?

[Severity: Medium]
Does this violate the RCU and skb_dst_set_noref() API contract?

This code combines skb_dst_set_noref() with asynchronous queuing via
__netif_rx() (called just below this hunk).

__netif_rx() places the packet on the per-CPU backlog and defers processing to
a future softirq. This means the packet inherently escapes the driver's current
RCU read-side critical section without being upgraded via skb_dst_force().

Other DSA-aware drivers avoid this by using napi_gro_receive(), which processes
the packet synchronously within the NAPI poll loop's RCU critical section.

[ ... ]

> @@ -771,6 +780,37 @@ static int ep_count_dma_queues(struct device *dev, u32 *out_tx, u32 *out_rx)
>  	return 0;
>  }
>  
> +static void ep_free_port_md(struct xlnx_tsn_ep *ep)
> +{
> +	int i;
> +
> +	for (i = 0; i < XLNX_TSN_EP_PORT_MD_SLOTS; i++) {
> +		if (ep->port_md[i]) {
> +			metadata_dst_free(ep->port_md[i]);

[Severity: High]
Is it safe to unconditionally free the memory here?

Since the metadata dst was attached to packets that could have been upgraded
to refcounted references in the network stack, calling metadata_dst_free()
(which calls kfree directly) bypasses the reference counting.

Should this drop the initial reference via dst_release() instead?

> +			ep->port_md[i] = NULL;
> +		}
> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8