From: Elia Pinto Date: Sat, 31 Oct 2026 10:16:31 GMT Subject: [PATCH v3 0/4] use size_t for a few more object and buffer sizes Message-ID: In-Reply-To: Thanks to Patrick Steinhardt for reviewing v2. Changes since v2: - Patch 1: drop the redundant file size check since the OID printed by `hash-object -w` already proves the content is complete, using `cat-file -e` just to verify the object exists. This also avoids reading and writing the same file unnecessarily. - Patch 2 & 3: unchanged. - Patch 4: shortened commit message following Patrick's comments. Link to v2: https://lore.kernel.org/git/cover.1793352943.git.gitter.spiros@gmail.com/ Elia Pinto (4): odb: use size_t for the length of objects to write diff: use size_t for the allocation size of word-diff buffers convert: use size_t for buffer sizes convert: use size_t for text statistics and ident counts attr.c | 2 +- convert.c | 24 ++++++++++++------------ diff.c | 4 ++-- odb.c | 2 +- odb.h | 4 ++-- odb/source-inmemory.c | 2 +- odb/source-loose.c | 2 +- odb/source.h | 2 +- read-cache-ll.h | 2 +- read-cache.c | 4 ++-- t/t1007-hash-object.sh | 11 +++++++++++ 11 files changed, 35 insertions(+), 24 deletions(-) Range-diff against v2: 1: 8fcb9f10f1 ! 1: 4961a1824a odb: use size_t for the length of objects to write @@ Commit message The existing "hash filtered files over 4GB correctly" test in t1007 only hashes, so add a variant that writes the object with "git - hash-object -w" and checks both its object ID and its size. Like the + hash-object -w" and checks its object ID and that it exists. Like the other tests over 4GB, it needs EXPENSIVE and SIZE_T_IS_64BIT. Truncating the length to 32 bits in odb_write_object_ext(), as happens on Windows without this patch, makes it fail with a different object ID. @@ t/t1007-hash-object.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \ + echo "big filter=null-filter" >.gitattributes && + git hash-object -w -- big >actual && + test_cmp expect actual && -+ echo $((5*1024*1024*1024)) >expect && -+ git cat-file -s $(cat actual) >actual && -+ test_cmp expect actual ++ git cat-file -e "$(cat actual)" +' + test_done 2: 42d4eae985 = 2: b27ef2a888 diff: use size_t for the allocation size of word-diff buffers 3: 88ca2ceb80 = 3: 1617177880 convert: use size_t for buffer sizes 4: 84e9e38a3e ! 4: 153463b531 convert: use size_t for text statistics and ident counts @@ Metadata ## Commit message ## convert: use size_t for text statistics and ident counts - gather_stats() counts NULs, CRs, LFs, CRLFs and (non)printable - characters in "unsigned" fields of struct text_stat, and - count_ident() counts "$Id$" markers in an "int". Now that the buffer - sizes they work on are size_t, a large enough buffer overflows them. - Unlike the previous patches, this is not specific to LLP64: "unsigned" - is 32 bits wide on all the platforms we care about. + gather_stats() and count_ident() use "unsigned" and "int" counters to + count characters and markers in buffers whose sizes are measured in + size_t. On platforms where size_t can exceed 32 bits, large inputs can + overflow these counters. - The counters only drive decisions, but a wrapped counter makes the - wrong one. For example, a file with exactly 2^32 lone LFs gets - lonelf == 0, so will_convert_lf_to_crlf() decides there is nothing - to convert, and a wrapped "printable" or "nonprintable" can fool the - heuristic in convert_is_binary(). In count_ident(), going past - INT_MAX is undefined behaviour. + For example, a file with exactly 2^32 lone LFs wraps lonelf back + to zero, causing will_convert_lf_to_crlf() to miss the conversion. - This is not a memory safety problem: the counts are only used as - hints to strbuf_grow() in crlf_to_worktree() and ident_to_worktree(), - and the buffers are then filled with strbuf_add(), which grows them - as needed. + Switch the counters to size_t and check the size hint for overflow. - Use size_t for all of them, and compute the size hint in - ident_to_worktree() with st_add() and st_mult(), so that it dies - instead of wrapping around. - - Triggering any of this needs more than 4GiB of input, so no test is - added. + Triggering this requires processing over 4GiB of input, so no test + is added. Suggested-by: Patrick Steinhardt Signed-off-by: Elia Pinto base-commit: b9e5cff1803ab24582201ef059516eb16014b9d9 -- 2.53.0