I'm starting samba with fruit:time machine max size = 700G and get error: fruit_tmsize_do_dirent: tmsize overflow: bandsize [8388608] nbands [82339] bandsize and nbands values looks right and this config was working with previous samba version I've found this commit: vfs_fruit: Fix CID 1433613 Operands don't affect result https://git.samba.org/samba.git/?p=samba.git;a=commit;h=f3e98f41de25e3b52fd38f86317e428bfb53b287 6558 if (bandsize > SIZE_MAX/nbands) { 6559 DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n", 6560 bandsize, nbands); 6561 return false; and it seems SIZE_MAX is only 4GB-1: 4294967295 I'm building samba on debian buster in docker container on armv7l hardware
Hi, Ralph! It seems that much of the time machine stuff uses size_t, which on this platform looks like just 32-bit. A fix would be to convert all of that to uint64_t. Correct?
Or maybe off_t which is what struct stat uses for the file size member.
This problem still exists, with samba 4.9.3 I've got an error: fruit_tmsize_do_dirent: tmsize overflow: bandsize [8388608] nbands [102927] from this code: if (bandsize > SIZE_MAX/nbands) { DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n", bandsize, nbands); return false; } tm_size = bandsize * nbands; This "if" tries to protect tm_size from overflow. And the type of tm_size is off_t already. So it's just wrong assumption here, that SIZE_MAX == OFF_T_MAX Unfortunately there is no cross-platform OFF_T_MAX macros, so I've declared it this way: #ifndef OFF_T_MAX #if SIZEOF_OFF_T == SIZEOF_INT8_T #define OFF_T_MAX INT8_MAX #elif SIZEOF_OFF_T == SIZEOF_INT16_T #define OFF_T_MAX INT16_MAX #elif SIZEOF_OFF_T == SIZEOF_INT32_T #define OFF_T_MAX INT32_MAX #elif SIZEOF_OFF_T == SIZEOF_INT64_T #define OFF_T_MAX INT64_MAX #endif #endif and replaced SIZE_MAX with OFF_T_MAX in that "if" this fixed issue for me
Created attachment 14726 [details] Fixes tm_size overflow check code on armv7l platform
With current file sizes having tm_size of 32 bits is useless, so it should be at least 64 bits. As such, the following code seems to be unnecessary: if (bandsize > SIZE_MAX/nbands) { DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n", bandsize, nbands); return false; } This, perhaps, too: if (state->total_size + tm_size < state->total_size) { DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n", bandsize, nbands); return false; } And after removing the first (or both) only the following line should be fixed: tm_size = bandsize * nbands; replaced by: tm_size = (off_t)bandsize * (off_t)nbands; Otherwise tm_size is calculated in 32 bits and loses higher 32 bits (looks like only up to 4G is used instead of 100-200+). Other option is to do all calculations in uint64_t instead of off_t, but it depends on personal taste (use off_t for file sizes or uint64_t).
Removed both 'if' clauses and added (off_t) as shown above. Built package for Debian, armel architecture, sizeof(off_t)=8, sizeof(size_t)=4. Confirmed, free and used space reported correctly for TM shares.
Hai, I've gotten a report also thats this is seen on Debian 9, 64bit. samba 4.9.7 I've ask the list reporter to update the bugreport here also. his report. MacOS can’t seem to get the size of the disk. I’ve traced it down to this config option fruit:time machine max size = 326G Which results in [Sun May 5 15:47:28 2019] traps: smbd[9658] trap divide error ip:7f8d722a84bb sp:7ffdf4e93cb0 error:0 [Sun May 5 15:47:28 2019] in fruit.so[7f8d7229f000+1c000] If I remove the max size option, all is well
From the code: if (bandsize > SIZE_MAX/nbands) { DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n", bandsize, nbands); return false; } it follows that when you have no TM bands yet (before the 1st backup), it will zero divide. And, as I said, this 'if' is useless when you operate with 64 bit values (it always will be less than uint64_t max value). Removing both 'if' and adding type conversion should fix all reported issues and make max size option work. At least, it worked for me for both new and incremental backups.
This still seem to-be an issue in 4.11.6, is there a reason this has not been patched? The last comment on this and fix seem rather trivial.
Created attachment 15850 [details] git-am fix for 4.12.next, 4.11.next Cherry-picked from master.
Reassigning to Karolin for inclusion in 4.11 and 4.12.
(In reply to Ralph Böhme from comment #11) Pushed to autobuild-v4-{12,11}-test.
(In reply to Karolin Seeger from comment #12) Pushed to both branches. Closing out bug report. Thanks!
(In reply to Pavel Volkovitskiy from comment #3) How about using uint64_t there for the size, instead of relying on a definition of off_t? Isn't this a samba protocol level thing (or macos struct size thing), which is not related to the OS off_t? Besides, isn't samba supposed to be compiled with LFS (large file support) on 32bits, where off_t is 64bit automatically?
(In reply to Michael Tokarev from comment #14) hm, but this has already been said by Oleg, I missed that part. N/m.