[PATCH 93/98] nvmet-fc: only send read data for successful commands

[PATCH 93/98] nvmet-fc: only send read data for successful commands

[Jens Axboe]

From: Andrea Parri <parri.andrea@gmail.com>

__nvmet_fc_fcp_nvme_cmd_done() pushes the whole data buffer of a read
to the host even when the command failed. The buffer comes from
sgl_alloc() without __GFP_ZERO, and a command rejected by
nvmet_check_transfer_len() never touches it, so the host receives stale
target memory, as much as it asked for.

The tcp and rdma targets only send data when cqe->status is zero. Do
the same for fc. The ERSP then reports fod->offset, which is 0, as the
transferred length, matching what the host checks.

Fixes: c53432030d86 ("nvme-fabrics: Add target support for FC transport")
Cc: stable@vger.kernel.org
Cc: Justin Tee <justin.tee@broadcom.com>
Cc: Naresh Gottumukkala <nareshgottumukkala83@gmail.com>
Cc: Paul Ely <paul.ely@broadcom.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 drivers/nvme/target/fc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index eecdffefa24f..b95893adae36 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -2487,11 +2487,11 @@ __nvmet_fc_fcp_nvme_cmd_done(struct nvmet_fc_tgtport *tgtport,
 	} else {
 
 		/*
-		 * try to push the data even if the SQE status is non-zero.
-		 * There may be a status where data still was intended to
-		 * be moved
+		 * A failed command may not have filled its read buffer, so
+		 * only push data for success, as the tcp and rdma targets do.
 		 */
-		if ((fod->io_dir == NVMET_FCP_READ) && (fod->data_sg_cnt)) {
+		if (fod->io_dir == NVMET_FCP_READ && fod->data_sg_cnt &&
+		    !cqe->status) {
 			/* push the data over before sending rsp */
 			nvmet_fc_transfer_fcp_data(tgtport, fod,
 						NVMET_FCOP_READDATA);
-- 
2.56.0

Re: [PATCH 93/98] nvmet-fc: only send read data for successful commands

[Hannes Reinecke]

On 10/28/26 5:42 PM, Jens Axboe wrote:
> From: Andrea Parri <parri.andrea@gmail.com>
> 
> __nvmet_fc_fcp_nvme_cmd_done() pushes the whole data buffer of a read
> to the host even when the command failed. The buffer comes from
> sgl_alloc() without __GFP_ZERO, and a command rejected by
> nvmet_check_transfer_len() never touches it, so the host receives stale
> target memory, as much as it asked for.
> 
> The tcp and rdma targets only send data when cqe->status is zero. Do
> the same for fc. The ERSP then reports fod->offset, which is 0, as the
> transferred length, matching what the host checks.
> 
> Fixes: c53432030d86 ("nvme-fabrics: Add target support for FC transport")
> Cc: stable@vger.kernel.org
> Cc: Justin Tee <justin.tee@broadcom.com>
> Cc: Naresh Gottumukkala <nareshgottumukkala83@gmail.com>
> Cc: Paul Ely <paul.ely@broadcom.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Sagi Grimberg <sagi@grimberg.me>
> Cc: Chaitanya Kulkarni <kch@nvidia.com>
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> Signed-off-by: Chris Mason <mason@kernel.org>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>   drivers/nvme/target/fc.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index eecdffefa24f..b95893adae36 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -2487,11 +2487,11 @@ __nvmet_fc_fcp_nvme_cmd_done(struct nvmet_fc_tgtport *tgtport,
>   	} else {
>   
>   		/*
> -		 * try to push the data even if the SQE status is non-zero.
> -		 * There may be a status where data still was intended to
> -		 * be moved
> +		 * A failed command may not have filled its read buffer, so
> +		 * only push data for success, as the tcp and rdma targets do.
>   		 */
> -		if ((fod->io_dir == NVMET_FCP_READ) && (fod->data_sg_cnt)) {
> +		if (fod->io_dir == NVMET_FCP_READ && fod->data_sg_cnt &&
> +		    !cqe->status) {
>   			/* push the data over before sending rsp */
>   			nvmet_fc_transfer_fcp_data(tgtport, fod,
>   						NVMET_FCOP_READDATA);

Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich