Avoid integer wrapping pitfalls for (off, len) pairs.

We use check_add_overflow(a, b, d) here to validate that (off, len)
pairs do not exceed the max value type. The kernel conveniently has
several macros to sort out the problems with signed or unsigned types.

However, we're not interested in purely seeing whether (a + b)
overflows, because we're using this for (off, len) overflow checks,
where the bytes we read are from 0 to len -1. We must therefore call
this check with (b) being "len - 1".

I've made sure that we don't accidentally fail when (len == 0)
in all cases by making sure we've already checked this condition
before, and moving code around as needed to ensure that (len > 0)
in all cases where we check.

The macro check_add_overflow requires a (d) argument in which
temporarily the result of the addition is stored and then checked to see
if an overflow occurred. We put a `tmp` variable on the stack of the
correct type as needed to make the checks function.

simple-release-extents test mistakenly relied on this buggy wrap code,
so it needs fixing. The move-blocks test also got it wrong.

Signed-off-by: Auke Kok <auke.kok@versity.com>

kmod/src/client.c

@@ -20,6 +20,7 @@
 #include <net/sock.h>
 #include <net/tcp.h>
 #include <asm/barrier.h>
+#include <linux/overflow.h>
 
 #include "format.h"
 #include "counters.h"
@@ -68,6 +69,7 @@ int scoutfs_client_alloc_inodes(struct super_block *sb, u64 count,
 	struct client_info *client = SCOUTFS_SB(sb)->client_info;
 	struct scoutfs_net_inode_alloc ial;
 	__le64 lecount = cpu_to_le64(count);
+	u64 tmp;
 	int ret;
 
 	ret = scoutfs_net_sync_request(sb, client->conn,
@@ -80,7 +82,7 @@ int scoutfs_client_alloc_inodes(struct super_block *sb, u64 count,
 
 		if (*nr == 0)
 			ret = -ENOSPC;
-		else if (*ino + *nr < *ino)
+		else if (check_add_overflow(*ino, *nr - 1, &tmp))
 			ret = -EINVAL;
 	}
 

kmod/src/data.c

@@ -22,6 +22,7 @@
 #include <linux/falloc.h>
 #include <linux/fiemap.h>
 #include <linux/writeback.h>
+#include <linux/overflow.h>
 
 #include "format.h"
 #include "super.h"
@@ -1087,6 +1088,7 @@ long scoutfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 	loff_t end;
 	u64 iblock;
 	u64 last;
+	loff_t tmp;
 	s64 ret;
 
 	/* XXX support more flags */
@@ -1095,14 +1097,14 @@ long scoutfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 		goto out;
 	}
 
-	/* catch wrapping */
-	if (offset + len < offset) {
-		ret = -EINVAL;
+	if (len == 0) {
+		ret = 0;
 		goto out;
 	}
 
-	if (len == 0) {
-		ret = 0;
+	/* catch wrapping */
+	if (check_add_overflow(offset, len - 1, &tmp)) {
+		ret = -EINVAL;
 		goto out;
 	}
 
@@ -1728,6 +1730,7 @@ int scoutfs_data_wait_check(struct inode *inode, loff_t pos, loff_t len,
 	u64 last_block;
 	u64 on;
 	u64 off;
+	loff_t tmp;
 	int ret = 0;
 
 	if (len == 0)
@@ -1736,7 +1739,7 @@ int scoutfs_data_wait_check(struct inode *inode, loff_t pos, loff_t len,
 	if (WARN_ON_ONCE(sef & SEF_UNKNOWN) ||
 	    WARN_ON_ONCE(op & SCOUTFS_IOC_DWO_UNKNOWN) ||
 	    WARN_ON_ONCE(dw && !RB_EMPTY_NODE(&dw->node)) ||
-	    WARN_ON_ONCE(pos + len < pos)) {
+	    WARN_ON_ONCE(check_add_overflow(pos, len - 1, &tmp))) {
 		ret = -EINVAL;
 		goto out;
 	}

kmod/src/ioctl.c

@@ -23,6 +23,7 @@
 #include <linux/aio.h>
 #include <linux/list_sort.h>
 #include <linux/backing-dev.h>
+#include <linux/overflow.h>
 
 #include "format.h"
 #include "key.h"
@@ -47,6 +48,7 @@
 #include "wkic.h"
 #include "quota.h"
 #include "scoutfs_trace.h"
+#include "util.h"
 
 /*
  * We make inode index items coherent by locking fixed size regions of
@@ -288,6 +290,7 @@ static long scoutfs_ioc_release(struct file *file, unsigned long arg)
 	u64 online;
 	u64 offline;
 	u64 isize;
+	__u64 tmp;
 	int ret;
 
 	if (copy_from_user(&args, (void __user *)arg, sizeof(args)))
@@ -297,12 +300,11 @@ static long scoutfs_ioc_release(struct file *file, unsigned long arg)
 
 	if (args.length == 0)
 		return 0;
-	if (((args.offset + args.length) < args.offset) ||
+	if ((check_add_overflow(args.offset, args.length - 1, &tmp)) ||
 	    (args.offset & SCOUTFS_BLOCK_SM_MASK) ||
 	    (args.length & SCOUTFS_BLOCK_SM_MASK))
 		return -EINVAL;
 
-
 	ret = mnt_want_write_file(file);
 	if (ret)
 		return ret;
@@ -955,6 +957,7 @@ static long scoutfs_ioc_move_blocks(struct file *file, unsigned long arg)
 	struct scoutfs_ioctl_move_blocks mb;
 	struct file *from_file;
 	struct inode *from;
+	u64 tmp;
 	int ret;
 
 	if (copy_from_user(&mb, umb, sizeof(mb)))
@@ -963,8 +966,8 @@ static long scoutfs_ioc_move_blocks(struct file *file, unsigned long arg)
 	if (mb.len == 0)
 		return 0;
 
-	if (mb.from_off + mb.len < mb.from_off ||
-	    mb.to_off + mb.len < mb.to_off)
+	if ((check_add_overflow(mb.from_off, mb.len - 1, &tmp)) ||
+	    (check_add_overflow(mb.to_off, mb.len - 1, &tmp)))
 		return -EOVERFLOW;
 
 	from_file = fget(mb.from_fd);

tests/tests/move-blocks.sh

@@ -83,9 +83,9 @@ touch "$OTHER"
 ln "$FROM" "$HARD"
 
 echo "== wrapped offsets should fail"
-HUGE=0x8000000000000000
-scoutfs move-blocks "$FROM" -f "$HUGE" -l "$HUGE" "$TO" -t 0 2>&1 | t_filter_fs
-scoutfs move-blocks "$FROM" -f 0 -l "$HUGE" "$TO" -t "$HUGE" 2>&1 | t_filter_fs
+HUGE=0xfffffffffffff000
+scoutfs move-blocks "$FROM" -f "$HUGE" -l "8192" "$TO" -t 0 2>&1 | t_filter_fs
+scoutfs move-blocks "$FROM" -f 0 -l "$HUGE" "$TO" -t "8192" 2>&1 | t_filter_fs
 
 echo "== specifying same file fails"
 scoutfs move-blocks "$FROM" -f 0 -l "$BS" "$HARD" -t 0 2>&1 | t_filter_fs

tests/tests/simple-release-extents.sh

@@ -61,7 +61,10 @@ echo "== release past i_size is fine"
 release_vers "$FILE" stat 400K 4K
 
 echo "== wrapped blocks fails"
-release_vers "$FILE" stat $vers 0x8000000000000000 0x8000000000000000
+# just under!
+release_vers "$FILE" stat $vers 0xfffffffffffff000 4096
+# this goes over
+release_vers "$FILE" stat $vers 0xfffffffffffff000 8192
 
 echo "== releasing non-file fails"
 mknod "$CHAR" c 1 3