[PATCH 0/2] nvme-fabrics: bound reconnect_delay and keep_alive_tmo

[PATCH 0/2] nvme-fabrics: bound reconnect_delay and keep_alive_tmo

[Martin George]

reconnect_delay and keep_alive_tmo (kato) are accepted in seconds but 
later multiplied by HZ (and, for kato, by 1000) in 32-bit arithmetic, so 
large values overflow and yield bogus timers. Bound both at parse/store 
time so the conversions can't wrap.

The target already does this for kato via NVMET_MAX_KATO, and the host 
recently fixed the same class for admin_timeout/io_timeout (4a5aeebf1a00) 
and partly for reconnect_delay (f92fe8dea19c).

Martin George (2):
  nvme-fabrics: bound reconnect_delay to avoid timer overflow
  nvme-fabrics: bound keep_alive_tmo to avoid overflow

 drivers/nvme/host/fabrics.c | 4 ++--
 drivers/nvme/host/sysfs.c   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.53.0

[PATCH 1/2] nvme-fabrics: bound reconnect_delay to avoid timer overflow

[Martin George]

reconnect_delay is accepted up to INT_MAX, but every consumer computes
"reconnect_delay * HZ" in 32-bit (nvme-tcp, nvme-rdma, nvme-fc), so a
large value wraps into a short, bogus reconnect interval.

Reject values above INT_MAX / HZ in both the option parser and the
sysfs store.

Signed-off-by: Martin George <marting@netapp.com>
---
 drivers/nvme/host/fabrics.c | 2 +-
 drivers/nvme/host/sysfs.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index c262538523fb..25029978b978 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -893,7 +893,7 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 				ret = -EINVAL;
 				goto out;
 			}
-			if (token <= 0) {
+			if (token <= 0 || token > INT_MAX / HZ) {
 				pr_err("Invalid reconnect_delay %d\n", token);
 				ret = -EINVAL;
 				goto out;
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 9672ca3f74d2..8876d8db5401 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -632,7 +632,7 @@ static ssize_t nvme_ctrl_reconnect_delay_store(struct device *dev,
 	if (err)
 		return err;
 
-	if (!v || v > INT_MAX)
+	if (!v || v > INT_MAX / HZ)
 		return -EINVAL;
 
 	ctrl->opts->reconnect_delay = v;
-- 
2.53.0

[PATCH 2/2] nvme-fabrics: bound keep_alive_tmo to avoid overflow

[Martin George]

keep_alive_tmo (kato) is only checked for being negative, yet it is sent
to the controller as "kato * 1000" ms and used as "kato * HZ" jiffies for
the keep-alive timer; a large value overflows both.

Reject values above INT_MAX / 1000, which (as 1000 >= HZ) covers both
conversions, as the target already does via NVMET_MAX_KATO.

Signed-off-by: Martin George <marting@netapp.com>
---
 drivers/nvme/host/fabrics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 25029978b978..b4ca0f9fd703 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -837,7 +837,7 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 				goto out;
 			}
 
-			if (token < 0) {
+			if (token < 0 || token > INT_MAX / 1000) {
 				pr_err("Invalid keep_alive_tmo %d\n", token);
 				ret = -EINVAL;
 				goto out;
-- 
2.53.0

Re: [PATCH 0/2] nvme-fabrics: bound reconnect_delay and keep_alive_tmo

[Laurence Oberman]

On Fri, 2026-11-13 at 15:39 +0530, Martin George wrote:
> reconnect_delay and keep_alive_tmo (kato) are accepted in seconds but
> later multiplied by HZ (and, for kato, by 1000) in 32-bit arithmetic,
> so 
> large values overflow and yield bogus timers. Bound both at
> parse/store 
> time so the conversions can't wrap.
> 
> The target already does this for kato via NVMET_MAX_KATO, and the
> host 
> recently fixed the same class for admin_timeout/io_timeout
> (4a5aeebf1a00) 
> and partly for reconnect_delay (f92fe8dea19c).
> 
> Martin George (2):
>   nvme-fabrics: bound reconnect_delay to avoid timer overflow
>   nvme-fabrics: bound keep_alive_tmo to avoid overflow
> 
>  drivers/nvme/host/fabrics.c | 4 ++--
>  drivers/nvme/host/sysfs.c   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

For the series:

Looks good

Reviewed-by: Laurence Oberman <loberman@redhat.com>

One thought: fast_io_fail_tmo looks like the same class -- it's
scheduled as fast_io_fail_tmo * HZ in nvme_start_failfast_work() and
isn't upper-bounded in the parser or sysfs store. Might be worth
folding into this series.

Minor nit on 2/2: "1000 >= HZ" doesn't hold on every arch (alpha
allows HZ=1024/1200), though kato being unsigned keeps it safe here.

Re: [PATCH 0/2] nvme-fabrics: bound reconnect_delay and keep_alive_tmo

[Martin George]

On Fri, 2026-11-13 at 08:01 -0500, Laurence Oberman wrote:
> On Fri, 2026-11-13 at 15:39 +0530, Martin George wrote:
> > reconnect_delay and keep_alive_tmo (kato) are accepted in seconds
> > but
> > later multiplied by HZ (and, for kato, by 1000) in 32-bit
> > arithmetic,
> > so 
> > large values overflow and yield bogus timers. Bound both at
> > parse/store 
> > time so the conversions can't wrap.
> > 
> > The target already does this for kato via NVMET_MAX_KATO, and the
> > host 
> > recently fixed the same class for admin_timeout/io_timeout
> > (4a5aeebf1a00) 
> > and partly for reconnect_delay (f92fe8dea19c).
> > 
> > Martin George (2):
> >   nvme-fabrics: bound reconnect_delay to avoid timer overflow
> >   nvme-fabrics: bound keep_alive_tmo to avoid overflow
> > 
> >  drivers/nvme/host/fabrics.c | 4 ++--
> >  drivers/nvme/host/sysfs.c   | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> For the series:
> 
> Looks good
> 
> Reviewed-by: Laurence Oberman <loberman@redhat.com>
> 
> One thought: fast_io_fail_tmo looks like the same class -- it's
> scheduled as fast_io_fail_tmo * HZ in nvme_start_failfast_work() and
> isn't upper-bounded in the parser or sysfs store. Might be worth
> folding into this series.
> 

Good catch. fast_io_fail_tmo seems to be arguably worse since it is a
signed int (so "* HZ" is signed overflow). I'll add a 3/3 patch
bounding it at INT_MAX / HZ in the parser and sysfs store.

> Minor nit on 2/2: "1000 >= HZ" doesn't hold on every arch (alpha
> allows HZ=1024/1200), though kato being unsigned keeps it safe here.

Right, I'll drop that wording. The bound should be fine (INT_MAX / 1000
keeps "kato * 1000" in range, and kato being unsigned keeps "kato * HZ"
from wrapping) - just a changelog reword, no code change.

Thanks for the review comments.

-Martin