Re: [PATCH nvme] nvmet-rdma: reject short connection requests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] nvme-rdma: unauthenticated remote DoS via unbounded IO queue sizes
--

commit 73a9fa992c7d8c72321b7f00558b5cb1451f8574
Author: Weiming Shi <bestswngs@gmail.com>

nvmet-rdma: reject short connection requests

nvmet_rdma_parse_cm_connect_req() treats rdma cm private data as a
struct nvme_rdma_cm_req but accepts any nonzero private_data_len. The
iWARP path allocates exactly the peer-provided length, so a six-byte MPA
request reaches the parser and the hsqsize load reads two bytes past the
allocation. This requires the complete request structure before reading
any of its fields.

> diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
> index ffa1e172c259b..075ab58d8c884 100644
> --- a/drivers/nvme/target/rdma.c
> +++ b/drivers/nvme/target/rdma.c
> @@ -1412,7 +1412,7 @@ nvmet_rdma_parse_cm_connect_req(struct rdma_conn_param *conn,
>  	struct nvme_rdma_cm_req *req;
>  
>  	req = (struct nvme_rdma_cm_req *)conn->private_data;
> -	if (!req || conn->private_data_len == 0)
> +	if (!req || conn->private_data_len < sizeof(*req))
>  		return NVME_RDMA_CM_INVALID_LEN;
>  
>  	if (le16_to_cpu(req->recfmt) != NVME_RDMA_CM_FMT_1_0)

[Severity: High]
Does this code adequately validate the remote request fields for io queues?

In nvmet_rdma_parse_cm_connect_req(), the unvalidated hsqsize and hrqsize
fields from the untrusted remote request are directly assigned to the queue
sizes. While there is a check for the admin queue (host_qid == 0), the lack
of bounds checking for io queues allows an unauthenticated initiator to set
arbitrary queue bounds up to the 16-bit integer limit:

	queue->recv_queue_size = le16_to_cpu(req->hsqsize) + 1;
	queue->send_queue_size = le16_to_cpu(req->hrqsize);

	if (!queue->host_qid && queue->recv_queue_size > NVME_AQ_DEPTH)
		return NVME_RDMA_CM_INVALID_HSQSIZE;

During rdma connection establishment, these unvalidated sizes are passed to
nvmet_rdma_alloc_queue() within the unauthenticated rdma cm handler. This
introduces a regression where massive heap allocations are based on remote
input.

For instance, nvmet_rdma_alloc_rsps() uses the unvalidated size to loop up
to 131,072 times allocating cqe slabs and establishing dma mappings:

	int nr_rsps = queue->recv_queue_size * 2;
	...
	for (i = 0; i < nr_rsps; i++) {
		struct nvmet_rdma_rsp *rsp = &queue->rsps[i];
		ret = nvmet_rdma_alloc_rsp(ndev, rsp, i);

Similarly, nvmet_rdma_alloc_cmds() loops nr_cmds (up to 65,536) times,
allocating full memory pages via alloc_page(GFP_KERNEL) for inline data
and mapping them via dma:

	for (i = 0; i < nr_cmds; i++) {
		ret = nvmet_rdma_alloc_cmd(ndev, cmds + i, admin);

Could this allow an unauthenticated attacker to open a few concurrent
connections with large hsqsize values to trivially trigger severe memory
exhaustion and cause a denial of service?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016172336.286341-3-bestswngs@gmail.com?part=1