Re: [PATCH 3/5] wrapper: reject insanely large byte counts when reading/writing
Hi Johannes
On 14/10/2026 07:09, Johannes Schindelin via GitGitGadget wrote:
Show 10 quoted lines
> 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
Show 55 quoted lines
> 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)