Re: [PATCH RFC 1/3] IB/iser: Post header-only PDUs inline

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] iser: DMA mapping leak on iser_send_data_out() error path
--

commit 9c1d70cc4e6b5a103b601d5536559ca76fc30e86
Author: Andrei-Alexandru Bleortu <me@andrei-z.com>

IB/iser: Post header-only PDUs inline

iser_post_send() posts every PDU by reference, and the QP asks for no
inline data. Before the HCA can send a SCSI command it therefore has
to DMA-read the 76 bytes of iSER and iSCSI headers from host memory,
and that read is on the latency path of every command.

> diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
> index ab2a27d23478a..6319d7fd17cf1 100644
> --- a/drivers/infiniband/ulp/iser/iser_verbs.c
> +++ b/drivers/infiniband/ulp/iser/iser_verbs.c
[ ... ]
> @@ -870,6 +882,19 @@ int iser_post_send(struct ib_conn *ib_conn, struct iser_tx_desc *tx_desc)
>  	wr->opcode = IB_WR_SEND;
>  	wr->send_flags = IB_SEND_SIGNALED;
>  
> +	/*
> +	 * Post a header-only PDU inline if it fits in the granted inline
> +	 * size, so the HCA does not have to DMA-read it from host memory.
> +	 * The CPU copies inline data from a kernel virtual address, so point
> +	 * the SGE at the headers.
> +	 */
> +	tx_desc->tx_sg[0].addr = tx_desc->dma_addr;
> +	if (tx_desc->num_sge == 1 &&
> +	    tx_desc->tx_sg[0].length <= ib_conn->max_inline_data) {
> +		tx_desc->tx_sg[0].addr = (uintptr_t)&tx_desc->iser_header;
> +		wr->send_flags |= IB_SEND_INLINE;
> +	}
> +
>  	if (tx_desc->inv_wr.next)
>  		first_wr = &tx_desc->inv_wr;
>  	else if (tx_desc->reg_wr.wr.next)

[Severity: Medium]
If iser_post_send() returns an error here (such as when ib_post_send() fails),
iser_send_data_out() handles the error by jumping to send_data_out_error:

drivers/infiniband/ulp/iser/iser_initiator.c:iser_send_data_out() {
...
	err = iser_post_send(&iser_conn->ib_conn, tx_desc);
	if (!err)
		return 0;

send_data_out_error:
	kmem_cache_free(ig.desc_cache, tx_desc);
	iser_err("conn %p failed err %d\n", conn, err);
	return err;
}

When tx_desc was initialized earlier in iser_initialize_task_headers(), a DMA
mapping was created.

Does freeing tx_desc without calling ib_dma_unmap_single() cause a permanent
leak of the IOMMU DMA mapping?

This could potentially exhaust IOMMU space or cause memory corruption if the
address is reused.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015125118.2966247-1-me@andrei-z.com?part=1