Re: [libvirt PATCH 3/4] virFileReadLimFD: Limit maximum file size to INT_MAX - 1

On Wed, Jul 21, 2021 at 14:46:42 +0200, Tim Wiederhake wrote: > virFileReadLimFD always returns null-terminated data. To that end, it has to > add one to the maximum file size. If the maxium file size is INT_MAX, this > triggers a signed integer overflow. > > There is no instance left where a caller would call virFileReadLimFD with a > maxium file size of INT_MAX. Make virFileReadLimFD error out if the maximum > file size is INT_MAX to prevent the reintroduction of this issue. > > Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; > --- > src/util/virfile.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/util/virfile.c b/src/util/virfile.c > index 723e1ca6e5..b5600658d5 100644 > --- a/src/util/virfile.c > +++ b/src/util/virfile.c > @@ -1418,7 +1418,7 @@ virFileReadLimFD(int fd, int maxlen, char **buf) > size_t len; > char *s; > > - if (maxlen <= 0) { > + if ((maxlen <= 0) || (maxlen >= INT_MAX)) { > errno = EINVAL; > return -1; While '< 0' is common sense here, limiting to INT_MAX -1 should be mentioned in the docs. Or better, why aren't we converting this to 'size_t' instead? saferead_lim is already operating on 'size_t' and I think we could simply get rid of the overflow checks altogether when working with size_t.