Re: [PATCH v6 01/12] block: add BLK_STS_P2PDMA for unsupported peer-to-peer transfers

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

Pre-existing issues:
- [High] BLK_STS_INVAL is omitted from the non-retryable list in blk_path_error(), causing genuine invalid requests to trigger infinite path-flapping loops in dm-mpath.
--

--- Patch [1]: [PATCH v6 01/12] block: add BLK_STS_P2PDMA for unsupported peer-to-peer transfers ---
commit d0a7b922115b23f3d33b00adcd1560732559cd9f
Author: Logan Gunthorpe <[email protected]>

block: add BLK_STS_P2PDMA for unsupported peer-to-peer transfers

This commit introduces BLK_STS_P2PDMA to handle unsupported peer-to-peer DMA 
transfers that cannot be routed by the PCIe topology. It returns this new 
status from blk_dma_map_iter_start() instead of BLK_STS_INVAL to distinguish 
it from standard invalid requests. It also adds BLK_STS_P2PDMA to 
blk_path_error() so multipath does not infinitely retry these failures.

Fixes: 858299dc6160 ("block: add scatterlist-less DMA mapping helpers")
Fixes: 7ce3c1dd78fc ("nvme-pci: convert the data mapping to blk_rq_dma_map")
Cc: [email protected] # v6.17

> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 8808ee76e73c0..c8de7809320ad 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h

[ ... ]

> @@ -188,6 +197,7 @@ static inline bool blk_path_error(blk_status_t error)
>  	case BLK_STS_NOTSUPP:
>  	case BLK_STS_NOSPC:
>  	case BLK_STS_TARGET:
> +	case BLK_STS_P2PDMA:
>  	case BLK_STS_RESV_CONFLICT:
>  	case BLK_STS_MEDIUM:
>  	case BLK_STS_PROTECTION:

[Severity: High]
This is a pre-existing issue, but should BLK_STS_INVAL also be included in
this non-retryable list?

Currently, because BLK_STS_INVAL is omitted from blk_path_error(), it falls
through to the default case and returns true (treating it as a retryable
path failure).

If a user submits an inherently invalid I/O request (such as an atomic write
exceeding hardware limits via RWF_ATOMIC), the block layer correctly rejects
it and returns BLK_STS_INVAL.

However, dm-mpath will incorrectly treat this user error as a path failure,
fail the active path, and requeue the I/O. Since the request is genuinely
invalid, it fails on all paths, causing an infinite path-flapping loop as
the path checker periodically reinstates them.

Could this lead to a Denial of Service for all legitimate I/O on the
affected multipath device?

>  		return false;
>  	}
>  
>  	/* Anything else could be a path failure, so should be retried */
>  	return true;
>  }

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