FAILED: patch "[PATCH] nvme-tcp: reject a read that transferred too few bytes" failed to apply to 5.10-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 7fa3f73f6c8ddc5f0425b50fb2a626a782ef7d12
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026090316-lying-divisive-d066@gregkh' --subject-prefix 'PATCH 5.10.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 7fa3f73f6c8ddc5f0425b50fb2a626a782ef7d12 Mon Sep 17 00:00:00 2001
From: Yehyeong Lee <[email protected]>
Date: Sat, 1 Aug 2026 17:18:17 +0900
Subject: [PATCH] nvme-tcp: reject a read that transferred too few bytes
nvme_tcp_recv_data() completes a request once the current C2HData PDU has been consumed. Nothing compares the total bytes received against the length the command asked for: struct nvme_tcp_request has no receive-side counter, queue->data_remaining is per queue, and blk_mq_end_request() completes for blk_rq_bytes(rq) unconditionally with no residual concept anywhere above.
A controller can therefore answer a 4096-byte read with 512 bytes and have it reported as a complete read; user space then gets 4096 bytes of which 3584 are whatever was already in the page. I reproduced that with a test target.
Count the bytes received and refuse to complete a successful read whose count does not match, at the two NVME_TCP_F_DATA_SUCCESS paths and in nvme_tcp_process_nvme_cqe(). The success test shifts req->status right by one, because the driver keeps the wire value there and shifts it on completion, so the check must see what the completion path will see. Only REQ_OP_READ is checked, because there the length comes from the sectors the request covers; a passthrough command is built by its submitter, which picks both command and buffer, so the kernel has nothing to compare against.
drivers/nvme/host/tcp.c +34 -0
@@ -80,6 +80,7 @@ struct nvme_tcp_request {struct bio *curr_bio;struct iov_iter iter; +u32 data_recvd;/* send state */ size_t offset;
@@ -617,6 +618,29 @@ static void nvme_tcp_error_recovery(struct nvme_ctrl *ctrl)queue_work (nvme_reset_wq, &to_tcp_ctrl (ctrl)->err_work); } +/* +* NVMe has no short read: a read that completes successfully must +* have transferred everything it asked for. +*/ +static bool nvme_tcp_data_in_short (struct nvme_tcp_queue *queue, +struct request *rq) +{ +struct nvme_tcp_request *req =blk_mq_rq_to_pdu (rq); + +if (le16_to_cpu (req->status) >>1 ) +return false; +if (req_op (rq) !=REQ_OP_READ || !req->data_len) +return false; +if (likely (req->data_recvd == req->data_len)) +return false; + +dev_err (queue->ctrl->ctrl.device, +"queue %d tag %#x short data-in: got %u of %u\n" , +nvme_tcp_queue_id (queue), rq->tag, + req->data_recvd, req->data_len); +return true; +} +static int nvme_tcp_process_nvme_cqe (struct nvme_tcp_queue *queue,struct nvme_completion *cqe) {
@@ -636,6 +660,9 @@ static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,if (req->status ==cpu_to_le16 (NVME_SC_SUCCESS )) req->status = cqe->status; +if (unlikely (nvme_tcp_data_in_short (queue, rq))) +return -EPROTO ; +if (!nvme_try_complete_req (rq, req->status, cqe->result))nvme_complete_rq (rq); queue->nr_cqe++;
@@ -958,6 +985,7 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb, *len -= recv_len; *offset += recv_len; queue->data_remaining -= recv_len; + req->data_recvd += recv_len; }if (!queue->data_remaining) {
@@ -966,6 +994,8 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb, queue->ddgst_remaining =NVME_TCP_DIGEST_LENGTH ; }else {if (pdu->hdr.flags &NVME_TCP_F_DATA_SUCCESS ) { +if (unlikely (nvme_tcp_data_in_short (queue, rq))) +return -EPROTO ;nvme_tcp_end_request (rq,le16_to_cpu (req->status)); queue->nr_cqe++;
@@ -1014,6 +1044,9 @@ static int nvme_tcp_recv_ddgst(struct nvme_tcp_queue *queue, pdu->command_id);struct nvme_tcp_request *req =blk_mq_rq_to_pdu (rq); +if (unlikely (nvme_tcp_data_in_short (queue, rq))) +return -EPROTO ; +nvme_tcp_end_request (rq,le16_to_cpu (req->status)); queue->nr_cqe++; }
@@ -2746,6 +2779,7 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns, req->status =cpu_to_le16 (NVME_SC_SUCCESS ); req->offset =0 ; req->data_sent =0 ; + req->data_recvd =0 ; req->pdu_len =0 ; req->pdu_sent =0 ; req->h2cdata_left =0 ;
Sasha Levin
maintainerPATCH 1/2 nvme-tcp: Do not reset transport on data digest errors
From: Daniel Wagner <[email protected]>
[ Upstream commit 1ba2e507f55c5b0cbde8c0fbfe0d9e39612a3e52 ]
The spec says
7.4.6.1 Digest Error handling
When a host detects a data digest error in a C2HData PDU, that host shall continue processing C2HData PDUs associated with the command and when the command processing has completed, if a successful status was returned by the controller, the host shall fail the command with a non-fatal transport error.
Currently the transport is reseted when a data digest error is detected. Instead, when a digest error is detected, mark the final status as NVME_SC_DATA_XFER_ERROR and let the upper layer handle the error.
In order to keep track of the final result maintain a status field in nvme_tcp_request object and use it to overwrite the completion queue status (which might be successful even though a digest error has been detected) when completing the request.
Stable-dep-of: 7fa3f73f6c8d ("nvme-tcp: reject a read that transferred too few bytes")
--- drivers/nvme/host/tcp.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
drivers/nvme/host/tcp.c +18 -4
@@ -83,6 +83,7 @@ struct nvme_tcp_request {u32 pdu_len;u32 pdu_sent;u16 ttag; +__le16 status;struct list_head entry;struct llist_node lentry;__le32 ddgst;
@@ -524,6 +525,7 @@ static void nvme_tcp_error_recovery(struct nvme_ctrl *ctrl)static int nvme_tcp_process_nvme_cqe (struct nvme_tcp_queue *queue,struct nvme_completion *cqe) { +struct nvme_tcp_request *req;struct request *rq; rq =nvme_find_rq (nvme_tcp_tagset (queue), cqe->command_id);
@@ -535,7 +537,11 @@ static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,return -EINVAL ; } -if (!nvme_try_complete_req (rq, cqe->status, cqe->result)) + req =blk_mq_rq_to_pdu (rq); +if (req->status ==cpu_to_le16 (NVME_SC_SUCCESS )) + req->status = cqe->status; + +if (!nvme_try_complete_req (rq, req->status, cqe->result))nvme_complete_rq (rq); queue->nr_cqe++;
@@ -797,7 +803,8 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb, queue->ddgst_remaining =NVME_TCP_DIGEST_LENGTH ; }else {if (pdu->hdr.flags &NVME_TCP_F_DATA_SUCCESS ) { -nvme_tcp_end_request (rq,NVME_SC_SUCCESS ); +nvme_tcp_end_request (rq, +le16_to_cpu (req->status)); queue->nr_cqe++; }nvme_tcp_init_recv_ctx (queue);
@@ -827,18 +834,24 @@ static int nvme_tcp_recv_ddgst(struct nvme_tcp_queue *queue,return 0 ;if (queue->recv_ddgst != queue->exp_ddgst) { +struct request *rq =nvme_cid_to_rq (nvme_tcp_tagset (queue), + pdu->command_id); +struct nvme_tcp_request *req =blk_mq_rq_to_pdu (rq); + + req->status =cpu_to_le16 (NVME_SC_DATA_XFER_ERROR ); +dev_err (queue->ctrl->ctrl.device,"data digest error: recv %#x expected %#x\n" ,le32_to_cpu (queue->recv_ddgst),le32_to_cpu (queue->exp_ddgst)); -return -EIO ; }if (pdu->hdr.flags &NVME_TCP_F_DATA_SUCCESS ) {struct request *rq =nvme_cid_to_rq (nvme_tcp_tagset (queue), pdu->command_id); +struct nvme_tcp_request *req =blk_mq_rq_to_pdu (rq); -nvme_tcp_end_request (rq,NVME_SC_SUCCESS ); +nvme_tcp_end_request (rq,le16_to_cpu (req->status)); queue->nr_cqe++; }
@@ -2370,6 +2383,7 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns,return ret; req->state =NVME_TCP_SEND_CMD_PDU ; + req->status =cpu_to_le16 (NVME_SC_SUCCESS ); req->offset =0 ; req->data_sent =0 ; req->pdu_len =0 ;
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 2/2 nvme-tcp: reject a read that transferred too few bytes
From: Yehyeong Lee <[email protected]>
[ Upstream commit 7fa3f73f6c8ddc5f0425b50fb2a626a782ef7d12 ]
nvme_tcp_recv_data() completes a request once the current C2HData PDU has been consumed. Nothing compares the total bytes received against the length the command asked for: struct nvme_tcp_request has no receive-side counter, queue->data_remaining is per queue, and blk_mq_end_request() completes for blk_rq_bytes(rq) unconditionally with no residual concept anywhere above.
A controller can therefore answer a 4096-byte read with 512 bytes and have it reported as a complete read; user space then gets 4096 bytes of which 3584 are whatever was already in the page. I reproduced that with a test target.
Count the bytes received and refuse to complete a successful read whose count does not match, at the two NVME_TCP_F_DATA_SUCCESS paths and in nvme_tcp_process_nvme_cqe(). The success test shifts req->status right by one, because the driver keeps the wire value there and shifts it on completion, so the check must see what the completion path will see. Only REQ_OP_READ is checked, because there the length comes from the sectors the request covers; a passthrough command is built by its submitter, which picks both command and buffer, so the kernel has nothing to compare against.
--- drivers/nvme/host/tcp.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
drivers/nvme/host/tcp.c +34 -0
@@ -90,6 +90,7 @@ struct nvme_tcp_request {struct bio *curr_bio;struct iov_iter iter; +u32 data_recvd;/* send state */ size_t offset;
@@ -522,6 +523,29 @@ static void nvme_tcp_error_recovery(struct nvme_ctrl *ctrl)queue_work (nvme_reset_wq, &to_tcp_ctrl (ctrl)->err_work); } +/* +* NVMe has no short read: a read that completes successfully must +* have transferred everything it asked for. +*/ +static bool nvme_tcp_data_in_short (struct nvme_tcp_queue *queue, +struct request *rq) +{ +struct nvme_tcp_request *req =blk_mq_rq_to_pdu (rq); + +if (le16_to_cpu (req->status) >>1 ) +return false; +if (req_op (rq) !=REQ_OP_READ || !req->data_len) +return false; +if (likely (req->data_recvd == req->data_len)) +return false; + +dev_err (queue->ctrl->ctrl.device, +"queue %d tag %#x short data-in: got %u of %u\n" , +nvme_tcp_queue_id (queue), rq->tag, + req->data_recvd, req->data_len); +return true; +} +static int nvme_tcp_process_nvme_cqe (struct nvme_tcp_queue *queue,struct nvme_completion *cqe) {
@@ -541,6 +565,9 @@ static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,if (req->status ==cpu_to_le16 (NVME_SC_SUCCESS )) req->status = cqe->status; +if (unlikely (nvme_tcp_data_in_short (queue, rq))) +return -EPROTO ; +if (!nvme_try_complete_req (rq, req->status, cqe->result))nvme_complete_rq (rq); queue->nr_cqe++;
@@ -795,6 +822,7 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb, *len -= recv_len; *offset += recv_len; queue->data_remaining -= recv_len; + req->data_recvd += recv_len; }if (!queue->data_remaining) {
@@ -803,6 +831,8 @@ static int nvme_tcp_recv_data(struct nvme_tcp_queue *queue, struct sk_buff *skb, queue->ddgst_remaining =NVME_TCP_DIGEST_LENGTH ; }else {if (pdu->hdr.flags &NVME_TCP_F_DATA_SUCCESS ) { +if (unlikely (nvme_tcp_data_in_short (queue, rq))) +return -EPROTO ;nvme_tcp_end_request (rq,le16_to_cpu (req->status)); queue->nr_cqe++;
@@ -851,6 +881,9 @@ static int nvme_tcp_recv_ddgst(struct nvme_tcp_queue *queue, pdu->command_id);struct nvme_tcp_request *req =blk_mq_rq_to_pdu (rq); +if (unlikely (nvme_tcp_data_in_short (queue, rq))) +return -EPROTO ; +nvme_tcp_end_request (rq,le16_to_cpu (req->status)); queue->nr_cqe++; }
@@ -2386,6 +2419,7 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns, req->status =cpu_to_le16 (NVME_SC_SUCCESS ); req->offset =0 ; req->data_sent =0 ; + req->data_recvd =0 ; req->pdu_len =0 ; req->pdu_sent =0 ; req->data_len = blk_rq_nr_phys_segments(rq) ?
Signature · 1 line
2.53.0