Re: [PATCH 2/3] check_stream_sha1(): handle input underflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 31, 2018 at 01:23:54PM +0900, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
> > The bug comes from commit f6371f9210 (sha1_file: add
> > read_loose_object() function, 2017-01-13), which
> > reimplemented some of the existing loose object functions.
> > So it's worth checking if this bug was inherited from any of
> > those. The answers seems to be no. The two obvious
> > candidates are both OK:
> >
> >   1. unpack_sha1_rest(); this doesn't need to loop on
> >      Z_BUF_ERROR at all, since it allocates the expected
> >      output buffer in advance (which we can't do since we're
> >      explicitly streaming here)
> >
> >   2. check_object_signature(); the streaming path relies on
> >      the istream interface, which uses read_istream_loose()
> >      for this case. That function uses a similar "is our
> >      output buffer full" check with Z_BUF_ERROR (which is
> >      where I stole it from for this patch!)
> 
> See 692f0bc7 to find who did the fix you stole from, and for what
> kind of breakage the original fix was made.

Heh. I did not dig into it, but actually thought "I'll bet Junio had to
get this right when he wrote the streaming code. No wonder he spotted my
mistake so quickly!".

> By the way, a very similar loop for pack_non_delta istream iterates
> while total_read is smaller than sz, but it does not have the same
> check upon BUF_ERROR to see if we've read everything.

Indeed. Did you find that one by inspection, or did you peek at:

  https://public-inbox.org/git/20130325202114.GD16019@xxxxxxxxxxxxxxxxxxxxx/

? :)

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux