From: Li Lingfeng lilingfeng3@huawei.com
A null-ptr-deref problem may occur since commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()") tries to get command_id by nvme_req(req)->cmd while nvme_req(req)->cmd is NULL. The problem has been sloved since the patch has been reverted by commit 929ba86476b3. However, cmd->common.command_id is set to req->tag again which should be ((genctl & 0xf)< 12 | req->tag). Generating command_id by nvme_cid() in trace event instead of nvme_req(req)->cmd->common.command_id to set it to ((genctl & 0xf)< 12 | req->tag) without trigging the null-ptr-deref problem.
Fixes: commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()") Reported-by: John Sperbeck jsperbeck@google.com Link: https://lore.kernel.org/r/20240109181722.228783-1-jsperbeck@google.com Signed-off-by: Li Lingfeng lilingfeng3@huawei.com --- drivers/nvme/host/trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/trace.h b/drivers/nvme/host/trace.h index 700fdce2ecf1..0de057a298dd 100644 --- a/drivers/nvme/host/trace.h +++ b/drivers/nvme/host/trace.h @@ -98,7 +98,7 @@ TRACE_EVENT(nvme_complete_rq, TP_fast_assign( __entry->ctrl_id = nvme_req(req)->ctrl->instance; __entry->qid = nvme_req_qid(req); - __entry->cid = req->tag; + __entry->cid = nvme_cid(req); __entry->result = le64_to_cpu(nvme_req(req)->result.u64); __entry->retries = nvme_req(req)->retries; __entry->flags = nvme_req(req)->flags;
On Wed, Mar 06, 2024 at 07:25:06PM +0800, Li Lingfeng wrote:
From: Li Lingfeng lilingfeng3@huawei.com
A null-ptr-deref problem may occur since commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()") tries to get command_id by nvme_req(req)->cmd while nvme_req(req)->cmd is NULL. The problem has been sloved since the patch has been reverted by commit 929ba86476b3. However, cmd->common.command_id is set to req->tag again which should be ((genctl & 0xf)< 12 | req->tag). Generating command_id by nvme_cid() in trace event instead of nvme_req(req)->cmd->common.command_id to set it to ((genctl & 0xf)< 12 | req->tag) without trigging the null-ptr-deref problem.
Fixes: commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()")
This committ is reverted in the 5.10.208 release, so is this change still needed?
thanks,
greg k-h
在 2024/3/29 19:59, Greg KH 写道:
On Wed, Mar 06, 2024 at 07:25:06PM +0800, Li Lingfeng wrote:
From: Li Lingfeng lilingfeng3@huawei.com
A null-ptr-deref problem may occur since commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()") tries to get command_id by nvme_req(req)->cmd while nvme_req(req)->cmd is NULL. The problem has been sloved since the patch has been reverted by commit 929ba86476b3. However, cmd->common.command_id is set to req->tag again which should be ((genctl & 0xf)< 12 | req->tag). Generating command_id by nvme_cid() in trace event instead of nvme_req(req)->cmd->common.command_id to set it to ((genctl & 0xf)< 12 | req->tag) without trigging the null-ptr-deref problem.
Fixes: commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()")
This committ is reverted in the 5.10.208 release, so is this change still needed?
thanks,
greg k-h
As described by commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()"), we should use command_id instead of req->tag in trace_nvme_complete_rq(). So I don't think it's appropriate to just revert it.
Replacing req->tag with nvme_cid(req) can solve the problem described by commit 706960d328f5 without causing issues mentioned in commit 929ba86476b3 ("Revert "nvme: use command_id instead of req->tag in trace_nvme_complete_rq()"").
Maybe fix tag should be changed to "Fixes: commit 929ba86476b3 ("Revert "nvme: use command_id instead of req->tag in trace_nvme_complete_rq()"")"?
Thanks.
On Sat, Mar 30, 2024 at 09:30:57AM +0800, Li Lingfeng wrote:
在 2024/3/29 19:59, Greg KH 写道:
On Wed, Mar 06, 2024 at 07:25:06PM +0800, Li Lingfeng wrote:
From: Li Lingfeng lilingfeng3@huawei.com
A null-ptr-deref problem may occur since commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()") tries to get command_id by nvme_req(req)->cmd while nvme_req(req)->cmd is NULL. The problem has been sloved since the patch has been reverted by commit 929ba86476b3. However, cmd->common.command_id is set to req->tag again which should be ((genctl & 0xf)< 12 | req->tag). Generating command_id by nvme_cid() in trace event instead of nvme_req(req)->cmd->common.command_id to set it to ((genctl & 0xf)< 12 | req->tag) without trigging the null-ptr-deref problem.
Fixes: commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()")
This committ is reverted in the 5.10.208 release, so is this change still needed?
thanks,
greg k-h
As described by commit 706960d328f5 ("nvme: use command_id instead of req->tag in trace_nvme_complete_rq()"), we should use command_id instead of req->tag in trace_nvme_complete_rq(). So I don't think it's appropriate to just revert it.
It's been reverted, and is in a release, so if you want something changed here, it needs to be sent as a new change.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Li Lingfeng -
Li Lingfeng