* [PATCH] nvmet: reject keep-alive timeouts that would overflow
@ 2026-10-30 8:46 Yingjie Wang
2026-11-03 5:37 ` Christoph Hellwig
2026-11-03 15:35 ` Keith Busch
0 siblings, 2 replies; 3+ messages in thread
From: Yingjie Wang @ 2026-10-30 8:46 UTC (permalink / raw)
To: linux-nvme; +Cc: hch, sagi, kch, kbusch, axboe, linux-kernel
nvmet_set_feat_kato() and nvmet_alloc_ctrl() convert the 32-bit
millisecond KATO from the host to seconds with DIV_ROUND_UP(kato, 1000).
The addition in DIV_ROUND_UP() is done in 32 bits, so any KATO above
4294966296 ms wraps around and rounds to 0 seconds. A Set Features
command with KATO 0xffffffff then succeeds but disables the keep-alive
timer: Get Features returns 0 and Keep Alive fails with Keep Alive
Timeout Invalid. A Connect command with such a KATO creates a controller
without a keep-alive timer.
The converted value has more consumers that can overflow. Get Features
multiplies it by 1000, and the timer paths multiply it by HZ in 32 bits
and arm a delay that must not exceed MAX_JIFFY_OFFSET.
Reject a KATO whose rounded value does not fit all of these. Return Keep
Alive Timeout Invalid from both Set Features and Connect. Validate Set
Features before stopping the current timer so a failed update leaves its
timeout unchanged. Round in 64 bits. A KATO of 0 keeps its current
meaning.
Fixes: a07b4970f464 ("nvmet: add a generic NVMe target")
Cc: stable@vger.kernel.org
Signed-off-by: Yingjie Wang <1075151112@qq.com>
---
drivers/nvme/target/admin-cmd.c | 7 ++++++-
drivers/nvme/target/core.c | 8 +++++++-
drivers/nvme/target/nvmet.h | 8 ++++++++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 26028be8fe4d8..a06cff8304db0 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -1227,8 +1227,13 @@ u16 nvmet_set_feat_kato(struct nvmet_req *req)
{
u32 val32 = le32_to_cpu(req->cmd->common.cdw11);
+ if (val32 > NVMET_MAX_KATO * MSEC_PER_SEC) {
+ req->error_loc = offsetof(struct nvme_common_command, cdw11);
+ return NVME_SC_KA_TIMEOUT_INVALID | NVME_STATUS_DNR;
+ }
+
nvmet_stop_keep_alive_timer(req->sq->ctrl);
- req->sq->ctrl->kato = DIV_ROUND_UP(val32, 1000);
+ req->sq->ctrl->kato = DIV_ROUND_UP_ULL(val32, MSEC_PER_SEC);
nvmet_start_keep_alive_timer(req->sq->ctrl);
nvmet_set_result(req, req->sq->ctrl->kato);
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 956f9a552e70f..9b30a9b7a0b71 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1612,6 +1612,12 @@ struct nvmet_ctrl *nvmet_alloc_ctrl(struct nvmet_alloc_ctrl_args *args)
u8 dhchap_status;
int ret;
+ if (kato > NVMET_MAX_KATO * MSEC_PER_SEC) {
+ args->status = NVME_SC_KA_TIMEOUT_INVALID | NVME_STATUS_DNR;
+ args->error_loc = offsetof(struct nvmf_connect_command, kato);
+ return NULL;
+ }
+
args->status = NVME_SC_CONNECT_INVALID_PARAM | NVME_STATUS_DNR;
subsys = nvmet_find_get_subsys(args->port, args->subsysnqn);
if (!subsys) {
@@ -1684,7 +1690,7 @@ struct nvmet_ctrl *nvmet_alloc_ctrl(struct nvmet_alloc_ctrl_args *args)
kato = NVMET_DISC_KATO_MS;
/* keep-alive timeout in seconds */
- ctrl->kato = DIV_ROUND_UP(kato, 1000);
+ ctrl->kato = DIV_ROUND_UP_ULL(kato, MSEC_PER_SEC);
ctrl->err_counter = 0;
spin_lock_init(&ctrl->error_lock);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index bd13adc72f671..9ac599d90b413 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -7,6 +7,7 @@
#define _NVMET_H
#include <linux/dma-mapping.h>
+#include <linux/jiffies.h>
#include <linux/types.h>
#include <linux/device.h>
#include <linux/kref.h>
@@ -712,6 +713,13 @@ void nvmet_add_async_event(struct nvmet_ctrl *ctrl, u8 event_type,
#define NVMET_KAS 10
#define NVMET_DISC_KATO_MS 120000
+/*
+ * Largest keep-alive timeout in seconds that can be reported back in
+ * milliseconds and converted to jiffies without overflowing.
+ */
+#define NVMET_MAX_KATO \
+ min3(U32_MAX / MSEC_PER_SEC, U32_MAX / HZ, MAX_JIFFY_OFFSET / HZ)
+
int __init nvmet_init_configfs(void);
void __exit nvmet_exit_configfs(void);
base-commit: 89480d137bf0d943a82c64f86ae2407a15276b5e
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] nvmet: reject keep-alive timeouts that would overflow
2026-10-30 8:46 [PATCH] nvmet: reject keep-alive timeouts that would overflow Yingjie Wang
@ 2026-11-03 5:37 ` Christoph Hellwig
2026-11-03 15:35 ` Keith Busch
1 sibling, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2026-11-03 5:37 UTC (permalink / raw)
To: Yingjie Wang; +Cc: linux-nvme, hch, sagi, kch, kbusch, axboe, linux-kernel
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] nvmet: reject keep-alive timeouts that would overflow
2026-10-30 8:46 [PATCH] nvmet: reject keep-alive timeouts that would overflow Yingjie Wang
2026-11-03 5:37 ` Christoph Hellwig
@ 2026-11-03 15:35 ` Keith Busch
1 sibling, 0 replies; 3+ messages in thread
From: Keith Busch @ 2026-11-03 15:35 UTC (permalink / raw)
To: Yingjie Wang; +Cc: linux-nvme, hch, sagi, kch, axboe, linux-kernel
On Fri, Oct 30, 2026 at 04:46:22PM +0800, Yingjie Wang wrote:
> nvmet_set_feat_kato() and nvmet_alloc_ctrl() convert the 32-bit
> millisecond KATO from the host to seconds with DIV_ROUND_UP(kato, 1000).
> The addition in DIV_ROUND_UP() is done in 32 bits, so any KATO above
> 4294966296 ms wraps around and rounds to 0 seconds. A Set Features
> command with KATO 0xffffffff then succeeds but disables the keep-alive
> timer: Get Features returns 0 and Keep Alive fails with Keep Alive
> Timeout Invalid. A Connect command with such a KATO creates a controller
> without a keep-alive timer.
>
> The converted value has more consumers that can overflow. Get Features
> multiplies it by 1000, and the timer paths multiply it by HZ in 32 bits
> and arm a delay that must not exceed MAX_JIFFY_OFFSET.
>
> Reject a KATO whose rounded value does not fit all of these. Return Keep
> Alive Timeout Invalid from both Set Features and Connect. Validate Set
> Features before stopping the current timer so a failed update leaves its
> timeout unchanged. Round in 64 bits. A KATO of 0 keeps its current
> meaning.
Thanks, applied to nvme-7.4.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-03 15:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-30 8:46 [PATCH] nvmet: reject keep-alive timeouts that would overflow Yingjie Wang
2026-11-03 5:37 ` Christoph Hellwig
2026-11-03 15:35 ` Keith Busch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®