diff options
author | Jonathan Nieder <jrnieder@gmail.com> | 2021-05-19 04:52:56 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2021-05-19 09:00:30 +0300 |
commit | aafa5df0df39036c6500846acd3db5b75d264a3b (patch) | |
tree | c1ea39d1da88431ecdbb8522ade19fd6c5c19c58 /git-compat-util.h | |
parent | 48bf2fa8bad054d66bd79c6ba903c89c704201f7 (diff) |
xsize_t: avoid implementation defined behavior when len < 0
The xsize_t helper aims to safely convert an off_t to a size_t,
erroring out when a file offset is too large to fit into a memory
address. It does this by using two casts:
size_t size = (size_t) len;
if (len != (off_t) size)
... error out ...
On a platform with sizeof(size_t) < sizeof(off_t), this check is safe
and correct. The first cast truncates to a size_t by finding the
remainder modulo SIZE_MAX+1 (see C99 section 6.3.1.3 Signed and
unsigned integers) and the second promotes to an off_t, meaning the
result is true if and only if len is representable as a size_t.
On other platforms, this two-casts strategy still works well (always
succeeds) for len >= 0. But for len < 0, when the first cast succeeds
and produces SIZE_MAX + 1 + len, the resulting value is too large to
be represented as an off_t, so the second cast produces implementation
defined behavior. In practice, it is likely to produce a result of
true despite len not being representable as size_t.
Simplify by replacing with a more straightforward check: compare len
to the relevant bounds and then cast it. (To avoid a -Wsign-compare
warning, after checking that len >= 0, we explicitly convert to a
sufficiently-large unsigned type before comparing to SIZE_MAX.)
In practice, this is not likely to come up since typical callers use
nonnegative len. Still, it's helpful to handle this case to make the
behavior easy to reason about.
Historical note: the original bounds-checking in 46be82dfd0 (xsize_t:
check whether we lose bits, 2010-07-28) did not produce this
implementation-defined behavior, though it still did not handle
negative offsets. It was not until 73560c793a (git-compat-util.h:
xsize_t() - avoid -Wsign-compare warnings, 2017-09-21) introduced the
double cast that the implementation-defined behavior was triggered.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'git-compat-util.h')
-rw-r--r-- | git-compat-util.h | 6 |
1 files changed, 2 insertions, 4 deletions
diff --git a/git-compat-util.h b/git-compat-util.h index dcc786edaa..acef7f6bcb 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -981,11 +981,9 @@ static inline char *xstrdup_or_null(const char *str) static inline size_t xsize_t(off_t len) { - size_t size = (size_t) len; - - if (len != (off_t) size) + if (len < 0 || (uintmax_t) len > SIZE_MAX) die("Cannot handle files this big"); - return size; + return (size_t) len; } __attribute__((format (printf, 3, 4))) |