Re: [Samba] vfs fruit disk_free fails on tmsize overflow with macOS Time Machine

[Art MG]

> On Mon, Feb 24, 2020 at 08:34:11PM +0000, Jeremy Allison via samba wrote:
> We need to keep the overflow checks,
> as this is on user-supplied data I believe.

I appreciate the advice, Jeremy, and have modified the patch as below, in a new branch

I have ADDED the fix, AND left the error checking, but with better explanation 
in the debug log message and in the code itself.
The issue is now unlikely to arise, but at least it won't be fatal if it does :) win / win
My tests indicate that this code now resolves the issue. 

I will raise the PR shortly from
https://gitlab.com/artmg/samba/-/blob/artmg-tmsize-prevent-overflow/source3/modules/vfs_fruit.c

Will I need to create an account on the bugzilla to refer to the PR in the issue, 
or will the release manager arrange that as part of the process?

Thanks
Art

________


diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index b8ede0cdfdb5661cc9579f25b81613f30e410215..3b1dd3ca853329cc156e747df0eefd427511594e 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -6971,15 +6971,22 @@ static bool fruit_tmsize_do_dirent(vfs_handle_struct *handle,
 		return true;
 	}
 
+	/*
+	 * Arithmetic on 32-bit systems may cause overflow, depending on
+	 * size_t precision. First we check its unlikely, then we
+	 * force the precision into target off_t, then we check that
+	 * the total did not overflow either.
+	 */
 	if (bandsize > SIZE_MAX/nbands) {
-		DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n",
+		DBG_ERR("tmsize potential overflow: bandsize [%zu] nbands [%zu]\n",
 			bandsize, nbands);
 		return false;
 	}
-	tm_size = bandsize * nbands;
+
+	tm_size = (off_t)bandsize * (off_t)nbands;
 
 	if (state->total_size + tm_size < state->total_size) {
-		DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n",
+		DBG_ERR("tm total size overflow: bandsize [%zu] nbands [%zu]\n",
 			bandsize, nbands);
 		return false;
 	}