Re: [PATCH v2 12/14] NTB: ntb_transport: Clear QP pointers when freeing an MW

[Dave Jiang]

On 9/9/26 9:08 PM, Koichiro Den wrote:
> ntb_transport_link_cleanup() frees MW buffers but leaves rx_buff and
> remote_rx_info pointing into them. With a QP still allocated, another
> link-down notification or transport unbind before MW setup runs again
> can make ntb_qp_link_down_reset() write to freed memory through
> remote_rx_info.
> 
> Clear both pointers in ntb_free_mw() for all QPs using that MW,
> including those without a client. This also covers link-setup failures.
> 
> How to reproduce:
> 
>   1. Load ntb_transport and ntb_netdev on both sides and establish the
>      transport/QP links once. Stop traffic, but leave ntb_netdev loaded
>      on VHOST so its QPs remain allocated throughout the test.
> 
>   2. On HOST, unload ntb_netdev and ntb_transport, leaving ntb_hw_epf
>      bound:
> 
>      modprobe -r ntb_netdev ntb_transport
> 
>      Transport removal sends COMMAND_LINK_DOWN to VHOST. Wait for
>      ntb_transport_link_cleanup_work() to return on VHOST, using a
>      function-graph trace. The "Link Cleanup" message is printed before
>      MW release and is not sufficient to establish completion. Do not
>      bring the link back up before the next step.
> 
>   3-(A). UAF via repeated link-down notification
> 
>      Use ntb_tool on HOST to send another link-down request:
> 
>      HOST# modprobe ntb_tool
>      HOST# echo N > "/sys/kernel/debug/ntb_tool/$ntb_host_dev/link"
> 
>      ==================================================================
>      BUG: KASAN: vmalloc-out-of-bounds in ntb_qp_link_down_reset+0x2c0..
>      ...
>      Call trace:
>       ...
>       __asan_report_store4_noabort+0x1c/0x28
>       ntb_qp_link_down_reset+0x2c0/0x2e0 [ntb_transport]
>       ntb_qp_link_cleanup+0xc4/0x148 [ntb_transport]
>       ntb_transport_link_cleanup+0x314/0x350 [ntb_transport]
>       ntb_transport_link_cleanup_work+0x2c/0x50 [ntb_transport]
>       process_one_work+0x5b8/0x12f0
>       ...
> 
>   3-(B). UAF via transport removal after link-down
> 
>      VHOST# echo "$ntb_vhost_dev" > \
>          /sys/bus/ntb/drivers/ntb_transport/unbind
> 
>      ==================================================================
>      BUG: KASAN: vmalloc-out-of-bounds in ntb_qp_link_down_reset+0x2c0..
>      ...
>      Call trace:
>       ...
>       __asan_report_store4_noabort+0x1c/0x28
>       ntb_qp_link_down_reset+0x2c0/0x2e0 [ntb_transport]
>       ntb_qp_link_cleanup+0xc4/0x148 [ntb_transport]
>       ntb_transport_link_cleanup+0x314/0x350 [ntb_transport]
>       ntb_transport_free+0x68/0x588 [ntb_transport]
>       ntb_remove+0x5c/0xa0 [ntb]
> 
> Verified that neither test triggers a KASAN report with this patch.
> 
> Fixes: cc79bd2738c2 ("ntb: Clean up tx tail index on link down")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>

Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

I would move the full reproduce details under ---

> ---
> Changes in v2:
>   - No changes.
> 
>  drivers/ntb/ntb_transport.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index b949f36a4f2d..096be87e5ede 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -781,10 +781,17 @@ static void ntb_free_mw(struct ntb_transport_ctx *nt, int num_mw)
>  {
>  	struct ntb_transport_mw *mw = &nt->mw_vec[num_mw];
>  	struct device *dma_dev = ntb_get_dma_dev(nt->ndev);
> +	unsigned int i;
>  
>  	if (!mw->virt_addr)
>  		return;
>  
> +	/* Drop references from every QP using this MW. */
> +	for (i = num_mw; i < nt->qp_count; i += nt->mw_count) {
> +		nt->qp_vec[i].rx_buff = NULL;
> +		WRITE_ONCE(nt->qp_vec[i].remote_rx_info, NULL);
> +	}
> +
>  	ntb_mw_clear_trans(nt->ndev, PIDX, num_mw);
>  	dma_free_attrs(dma_dev, mw->alloc_size, mw->alloc_addr,
>  		       mw->original_dma_addr, DMA_ATTR_FORCE_CONTIGUOUS);