[PATCH] orangefs: bound op_timeout_secs and slot_timeout_secs

[PATCH] orangefs: bound op_timeout_secs and slot_timeout_secs

[Yingjie Wang]

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@qq.com>
---

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