Junio C Hamano wrote: > "Victoria Dye via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> From: Victoria Dye <vdye@xxxxxxxxxx> >> >> Avoid size_t overflow when reporting the available disk space in >> 'get_disk_info' by casting the block size and available block count to >> 'uint64_t' before multiplying them. Without this change, 'st_mult' would >> (correctly) report size_t overflow on 32-bit systems at or exceeding 2^32 >> bytes of available space. > > Sane. But shouldn't the cast be to off_t, which is what > strbuf_humanise_bytes() takes anyway? > I chose 'uint64_t' to mimic 'throughput_string()' [1], but the signed 'off_t' is a better choice given its use in 'strbuf_humanise_bytes()'. On a related note, while writing this I made the (unsubstantiated) assumption that 'off_t' would be a 64-bit int, even on 32-bit systems. Your comment prompted me to confirm that assumption; while 'off_t' is not always guaranteed to be an int64 by default [2], Git is compiled with '#define _FILE_OFFSET_BITS 64' [3] so 'off_t' is equivalent to 'off64_t'. I'll update the casts to 'off_t' in V2. Thanks! [1] https://lore.kernel.org/git/20171110173956.25105-4-newren@xxxxxxxxx/ [2] https://www.gnu.org/software/libc/manual/html_mono/libc.html#index-off_005ft [3] https://lore.kernel.org/git/7vr6smc1de.fsf@xxxxxxxxxxxxxxxxxxxxxxxx/ >> >> Signed-off-by: Victoria Dye <vdye@xxxxxxxxxx> >> --- >> builtin/bugreport.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/builtin/bugreport.c b/builtin/bugreport.c >> index 35b1fc48bf1..720889a37ad 100644 >> --- a/builtin/bugreport.c >> +++ b/builtin/bugreport.c >> @@ -258,7 +258,7 @@ static int get_disk_info(struct strbuf *out) >> } >> >> strbuf_addf(out, "Available space on '%s': ", buf.buf); >> - strbuf_humanise_bytes(out, st_mult(stat.f_bsize, stat.f_bavail)); >> + strbuf_humanise_bytes(out, (uint64_t)stat.f_bsize * (uint64_t)stat.f_bavail); >> strbuf_addf(out, " (mount flags 0x%lx)\n", stat.f_flag); >> strbuf_release(&buf); >> #endif
