On Fri, Mar 31, 2017 at 02:16:29PM -0700, Junio C Hamano wrote:

> Before we forget...
> 
> -- >8 --
> From: Jeff King <p...@peff.net>
> 
> When sha1_loose_object_info() finds that a loose object file cannot
> be stat(2)ed or mmap(2)ed, it returns -1 to signal an error to the
> caller.  However, if it found that the loose object file is corrupt
> and the object data cannot be used from it, it forgets to return -1.
> 
> This can confuse the caller, who may be lead to mistakenly think
> that there is a loose object and possibly gets an incorrect type and
> size from the function.  The SHA-1 collision detection codepath, for
> example, when it gets an object over the wire and tries to see the
> data is the same as what is available as a loose object locally, can
> get confused when the loose object is correupted due to this bug.

Unfortunately this isn't quite right. I was able to reproduce the
problem that Lars saw, and this patch doesn't fix it.

So here's a two-patch series. The first fixes the problem described
above, along with a simpler test that demonstrates it. The second fixes
Lars's problem on top.

I know you're heading out for a week, so I'll post it now for review,
and then hold and repost when you get back.

  [1/2]: sha1_loose_object_info: return error for corrupted objects
  [2/2]: index-pack: detect local corruption in collision check

 builtin/index-pack.c         |  2 ++
 sha1_file.c                  |  2 +-
 t/t1060-object-corruption.sh | 24 ++++++++++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

-Peff

Reply via email to