[PATCH] nvmet-fc: bound the FCP command data length before allocating buffers

[PATCH] nvmet-fc: bound the FCP command data length before allocating buffers

[Yehyeong Lee]

nvmet_fc_handle_fcp_rqst() takes the data length straight from the FCP
command IU (cmdiu->data_len, a 32-bit value from the wire) into
fod->req.transfer_len and calls nvmet_fc_alloc_tgt_pgs(), which hands it to
sgl_alloc() with no upper bound.  nvmet_req_init() only parses the command
and clears transfer_len; nvmet_check_transfer_len(), the only other place
the length is checked, runs later from req->execute(), after the allocation
has already happened.

A remote, effectively unauthenticated host can therefore declare a huge
transfer length and make the target allocate that much kernel memory for a
command that never delivers the data.  nvmet-tcp bounds the equivalent
length and the rdma transport is limited by its 24-bit SGL length field;
the fc transport had no such limit.

Reject a command whose data length exceeds NVMET_FC_MAX_XFR_LENGTH before
allocating anything.  The ceiling is 16 MiB, the largest transfer any
in-tree fc LLDD can describe (lpfc maps up to LPFC_MAX_SG_SEG_CNT 4096
segments, qla2xxx 1024), so no in-tree configuration is rejected.

A command declaring 4 GiB reaches sgl_alloc() unbounded before this change;
afterwards a length above 16 MiB is failed with SGL_INVALID_DATA before any
allocation is attempted.

Fixes: c53432030d86 ("nvme-fabrics: Add target support for FC transport")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
 drivers/nvme/target/fc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 1b557775e033a..5ec581780454e 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -62,6 +62,16 @@ struct nvmet_fc_ls_req_op {		/* for an LS RQST XMT */
 /* desired maximum for a single sequence - if sg list allows it */
 #define NVMET_FC_MAX_SEQ_LENGTH		(256 * 1024)
 
+/*
+ * Upper bound on the data length a host may declare in an FCP CMD IU.
+ * This is the largest transfer any in-tree LLDD can describe: lpfc maps
+ * at most LPFC_MAX_SG_SEG_CNT (4096) segments and qla2xxx 1024, so 4096
+ * pages covers both.  The length arrives from the wire as a full 32-bit
+ * value and sizes the command's buffer allocation, so it has to be
+ * bounded before anything is allocated for it.
+ */
+#define NVMET_FC_MAX_XFR_LENGTH		(16 * 1024 * 1024)
+
 enum nvmet_fcp_datadir {
 	NVMET_FCP_NODATA,
 	NVMET_FCP_WRITE,
@@ -2577,6 +2587,14 @@ nvmet_fc_handle_fcp_rqst(struct nvmet_fc_tgtport *tgtport,
 	atomic_inc(&fod->queue->sqtail);
 
 	if (fod->req.transfer_len) {
+		/* bound the host-declared length before allocating for it */
+		if (fod->req.transfer_len > NVMET_FC_MAX_XFR_LENGTH) {
+			nvmet_req_complete(&fod->req,
+					   NVME_SC_SGL_INVALID_DATA |
+					   NVME_STATUS_DNR);
+			return;
+		}
+
 		ret = nvmet_fc_alloc_tgt_pgs(fod);
 		if (ret) {
 			nvmet_req_complete(&fod->req, ret);

base-commit: 2c3418fffa9d037b2038a6db48be63f9e2291806
-- 
2.43.0

Re: [PATCH] nvmet-fc: bound the FCP command data length before allocating buffers

[Christoph Hellwig]

On Tue, Oct 13, 2026 at 07:40:38PM +0900, Yehyeong Lee wrote:
> nvmet_fc_handle_fcp_rqst() takes the data length straight from the FCP
> command IU (cmdiu->data_len, a 32-bit value from the wire) into
> fod->req.transfer_len and calls nvmet_fc_alloc_tgt_pgs(), which hands it to
> sgl_alloc() with no upper bound.  nvmet_req_init() only parses the command
> and clears transfer_len; nvmet_check_transfer_len(), the only other place
> the length is checked, runs later from req->execute(), after the allocation
> has already happened.
> 
> A remote, effectively unauthenticated host can therefore declare a huge
> transfer length and make the target allocate that much kernel memory for a
> command that never delivers the data.  nvmet-tcp bounds the equivalent
> length and the rdma transport is limited by its 24-bit SGL length field;
> the fc transport had no such limit.
> 
> Reject a command whose data length exceeds NVMET_FC_MAX_XFR_LENGTH before
> allocating anything.  The ceiling is 16 MiB, the largest transfer any
> in-tree fc LLDD can describe (lpfc maps up to LPFC_MAX_SG_SEG_CNT 4096
> segments, qla2xxx 1024), so no in-tree configuration is rejected.
> 
> A command declaring 4 GiB reaches sgl_alloc() unbounded before this change;
> afterwards a length above 16 MiB is failed with SGL_INVALID_DATA before any
> allocation is attempted.

The bounds check is obviously a good idea, but we really need to tell
the host about it using MDTS, don't we?

Re: [PATCH] nvmet-fc: bound the FCP command data length before allocating buffers

[Yehyeong Lee]

Agreed.  v2 adds a ->get_mdts callback that advertises the 16 MiB ceiling
so a compliant host keeps its transfers under it, and keeps the
allocation-time bound as enforcement for one that does not:
https://lore.kernel.org/linux-nvme/20261013153316.3178656-1-yhlee@isslab.korea.ac.kr/