Re: [PATCH] orangefs: bound op_timeout_secs and slot_timeout_secs

[Mike Marshall]

Your patch looks good and runs through xfstests fine.
I've added it to the stuff I intend to put on my linux
next... Thanks...

-Mike

On Tue, Nov 3, 2026 at 9:40 AM Yingjie Wang <1075151112@xxxxxx> wrote:
>
> service_operation() and wait_for_free() multiply op_timeout_secs and
> slot_timeout_secs by HZ as int and pass the result to
> schedule_timeout(). orangefs_init() only replaces negative module
> parameters with 0, and the /sys/fs/orangefs attributes store any int.
> Negative values are not valid timeouts, and values above INT_MAX / HZ
> overflow the signed multiplication. The result can be negative, zero,
> or much shorter than configured. Negative results make
> schedule_timeout() print "wrong timeout value" with a stack dump and
> return at once; all of these cases can make the operation fail with
> -ETIMEDOUT before the configured interval.
>
> Reject values outside 0..INT_MAX / HZ when the attributes are written,
> and clamp the module parameters to that range at load time, as negative
> values already were.
>
> Fixes: 274dcf55bd4a ("Orangefs: kernel client part 3")
> Signed-off-by: Yingjie Wang <1075151112@xxxxxx>
> ---
>
> Notes:
>     Tested on x86-64 (HZ=1000) in QEMU with the orangefs module and no
>     client daemon, so a pvfs2 mount waits in service_operation(). On the
>     base kernel, op_timeout_secs values of 2147484, 2147483647 and -1 were
>     accepted, and the mount failed with ETIMEDOUT after 14-29 ms with the
>     schedule_timeout() warning; loading the module with 2147484 did the
>     same. With the patch those writes fail with EINVAL and keep the old
>     value, the module parameter is clamped to 2147483, and the mount waits
>     (stopped by a 3 s userspace timeout). 1 and 2147483 behaved the same on
>     both kernels. slot_timeout_secs was checked through store and readback
>     only; its wait needs a running client. Also built for i386 with W=1.
>
>  fs/orangefs/orangefs-mod.c   |  8 +++-----
>  fs/orangefs/orangefs-sysfs.c | 19 +++++++++++++++++--
>  2 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/fs/orangefs/orangefs-mod.c b/fs/orangefs/orangefs-mod.c
> index f591f09531da..7a62d4987175 100644
> --- a/fs/orangefs/orangefs-mod.c
> +++ b/fs/orangefs/orangefs-mod.c
> @@ -83,11 +83,9 @@ static int __init orangefs_init(void)
>         int ret;
>         __u32 i = 0;
>
> -       if (op_timeout_secs < 0)
> -               op_timeout_secs = 0;
> -
> -       if (slot_timeout_secs < 0)
> -               slot_timeout_secs = 0;
> +       /* The timeouts are multiplied by HZ as int. */
> +       op_timeout_secs = clamp(op_timeout_secs, 0, INT_MAX / HZ);
> +       slot_timeout_secs = clamp(slot_timeout_secs, 0, INT_MAX / HZ);
>
>         /* initialize global book keeping data structures */
>         ret = op_cache_initialize();
> diff --git a/fs/orangefs/orangefs-sysfs.c b/fs/orangefs/orangefs-sysfs.c
> index a02bfc7b3542..1873a4f5cd16 100644
> --- a/fs/orangefs/orangefs-sysfs.c
> +++ b/fs/orangefs/orangefs-sysfs.c
> @@ -270,6 +270,21 @@ static ssize_t sysfs_int_show(struct kobject *kobj,
>         return rc;
>  }
>
> +/* The timeouts are multiplied by HZ as int. */
> +static int sysfs_timeout_secs_store(const char *buf, int *timeout_secs)
> +{
> +       int val;
> +       int rc;
> +
> +       rc = kstrtoint(buf, 0, &val);
> +       if (rc)
> +               return rc;
> +       if (val < 0 || val > INT_MAX / HZ)
> +               return -EINVAL;
> +       *timeout_secs = val;
> +       return 0;
> +}
> +
>  static ssize_t sysfs_int_store(struct kobject *kobj,
>      struct orangefs_attribute *attr, const char *buf, size_t count)
>  {
> @@ -280,10 +295,10 @@ static ssize_t sysfs_int_store(struct kobject *kobj,
>                      attr->attr.name, buf);
>
>         if (!strcmp(attr->attr.name, "op_timeout_secs")) {
> -               rc = kstrtoint(buf, 0, &op_timeout_secs);
> +               rc = sysfs_timeout_secs_store(buf, &op_timeout_secs);
>                 goto out;
>         } else if (!strcmp(attr->attr.name, "slot_timeout_secs")) {
> -               rc = kstrtoint(buf, 0, &slot_timeout_secs);
> +               rc = sysfs_timeout_secs_store(buf, &slot_timeout_secs);
>                 goto out;
>         } else if (!strcmp(attr->attr.name, "cache_timeout_msecs")) {
>                 rc = kstrtoint(buf, 0, &orangefs_cache_timeout_msecs);
>
> base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
> --
> 2.43.0
>