read(2) and thus bsdiff is limited to 2^31 bytes

[Konstantin Belousov]

On Sun, May 22, 2016 at 04:23:32PM -0700, Conrad Meyer wrote:
> On Sun, May 22, 2016 at 4:09 PM, Konstantin Belousov
> <kostikbel at gmail.com> wrote:
> > On Sun, May 22, 2016 at 03:56:33PM -0700, Conrad Meyer wrote:
> >> On Sun, May 22, 2016 at 1:54 PM, Dirk Engling <erdgeist at erdgeist.org> wrote:
> >> > When trying to bsdiff two DVD images, I noticed it failing due to
> >> > read(2) returning EINVAL to the tool. man 2 read says, this would only
> >> > happen for a negative value for fildes, which clearly was not true.
> >>
> >> Actually, it's documented at the very bottom of the first section:
> >>
> >> ERRORS
> >>      The read(), readv(), pread() and preadv() system calls will succeed
> >>      unless:
> >> ...
> >>      [EINVAL]           The value nbytes is greater than INT_MAX.
> >>
> >> It does seem silly to me given nbytes is a size_t.  I think it should
> >> error if nbytes is greater than SSIZE_T_MAX, but on platforms where
> >> size_t is larger than int (e.g. amd64) it shouldn't error for nbytes
> >> in [INT_MAX, SSIZE_T_MAX - 1].
> > It does not look silly to me, due to the typical
> >         if (read() < 0)
> > checks in the code.  Even
> >         if (read() == -1)
> > is vulnerable.
> 
> read(2) returns ssize_t; SSIZE_MAX is not a negative result.  I agree
> that nbytes in [SSIZE_MAX+1, SIZE_MAX] should be disallowed (negative
> ssize_t value after cast from size_t).
> 
> >
> >>
> >> As far as I can tell, this INT_MAX behavior is not required by POSIX.
> > From POSIX page for read():
> > RETURN VALUE
> >             Upon successful completion, these functions shall return a non-negative integer indicating the
> >             number of bytes actually read. Otherwise, the functions shall return -1 and set errno to indicate
> >             the error.
> 
> There is a difference between int and ssize_t.  They have different
> ranges on e.g. amd64.

Well, on amd64 there is sysctl debug.iosize_max_clamp, allowing large
but still non-negative ios (allowed in current). Except for devices
io, which are controlled by debug.devfs_iosize_max_clamp, disabled by
default since I was not able to audit that many drivers.