Re: [PATCH 3/5] wrapper: reject insanely large byte counts when reading/writing

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Johannes

On 14/10/2026 07:09, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> Both the `read_in_full()` as well as the `write_in_full()` function
> accumulate the bytes transferred count in a variable of type `ssize_t`,
> but accept a `size_t` count parameter. When count exceeds `SSIZE_MAX`,
> that total count overflows after processing `SSIZE_MAX` bytes, producing
> a negative return value that callers interpret as an error (or worse, a
> small positive value after further wrapping).
> 
> In practice, git never passes such large counts because file sizes and
> buffer allocations are bounded well below SSIZE_MAX on all target
> platforms. 

Are we sure about that? In the past we've had bug fixes such as 
c14e5a1a50 (transport-helper: use xread instead of read, 2019-01-03)
on platforms where ssize_t is narrower that size_t (POSIX does not 
require them to be the same width). I've cc'd Randall to see if ssize_t 
is still narrower than size_t on the NonStop platforms git supports.

Thanks

Phillip

> The POSIX specification for `read()` and `write()` also
> limits single operations to `SSIZE_MAX` bytes, and git's
> `xread()`/`xwrite()` wrappers further clamp to `MAX_IO_SIZE`. But the
> accumulation in the `*_in_full()` wrappers means that even with per-call
> clamping, total can exceed `SSIZE_MAX` when count does.
> 
> Reject a total count that exceeds the range of `ssize_t` upfront with
> `EINVAL` rather than silently producing wrong return values, matching
> the POSIX convention for invalid arguments.
> 
> Pointed out by Coverity.
> 
> Assisted-by: Claude Opus 4.6
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   git-compat-util.h |  4 ++++
>   wrapper.c         | 10 ++++++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/git-compat-util.h b/git-compat-util.h
> index a0f901ce79..afce8b85b8 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -740,6 +740,10 @@ static inline uint64_t u64_add(uint64_t a, uint64_t b)
>    */
>   #define DEFAULT_IO_BUFFER_SIZE (128 * 1024)
>   
> +#ifndef SSIZE_MAX
> +# define SSIZE_MAX ((ssize_t)(((size_t)-1) >> 1))
> +#endif
> +
>   #ifdef HAVE_ALLOCA_H
>   # include <alloca.h>
>   # define xalloca(size)      (alloca(size))
> diff --git a/wrapper.c b/wrapper.c
> index 05a9cd369c..ea9c14f834 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -288,6 +288,11 @@ ssize_t read_in_full(int fd, void *buf, size_t count)
>   	char *p = buf;
>   	ssize_t total = 0;
>   
> +	if (count > SSIZE_MAX) {
> +		errno = EINVAL;
> +		return -1;
> +	}
> +
>   	while (count > 0) {
>   		ssize_t loaded = xread(fd, p, count);
>   		if (loaded < 0)
> @@ -307,6 +312,11 @@ ssize_t write_in_full(int fd, const void *buf, size_t count)
>   	const char *p = buf;
>   	ssize_t total = 0;
>   
> +	if (count > SSIZE_MAX) {
> +		errno = EINVAL;
> +		return -1;
> +	}
> +
>   	while (count > 0) {
>   		ssize_t written = xwrite(fd, p, count);
>   		if (written < 0)