Re: [PATCH] fetch-pack: disregard invalid pack lockfiles

[Taylor Blau <ttaylorr@github.com>]

On Mon, Nov 30, 2020 at 09:15:47PM +0100, René Scharfe wrote:
> Am 30.11.20 um 20:53 schrieb Taylor Blau:
> > On Mon, Nov 30, 2020 at 08:27:15PM +0100, René Scharfe wrote:
> >> index_pack_lockfile() can return NULL if it doesn't like the contents it
> >> reads from the file descriptor passed to it.  unlink(2) is declared to
> >> not accept NULL pointers (at least with glibc).  Undefined Behavior
> >> Sanitizer together with Address Sanitizer detects a case where a NULL
> >> lockfile name is passed to unlink(2) by transport_unlock_pack() in t1060
> >> (make SANITIZE=address,undefined; cd t; ./t1060-object-corruption.sh).
> >
> > Which test in t1060? I tried to reproduce this myself, but couldn't seem
> > to coax out a failure. (Initially I thought that my ccache wasn't
> > letting me recompile with the SANITIZE options, but running 'ccache
> > clear' and then trying again left the test still passing).
>
> 15 - fetch into corrupted repo with index-pack
>
>    $ cat trash\ directory.t1060-object-corruption/bit-error-cp/stderr
>    error: inflate: data stream error (invalid distance too far back)
>    error: unable to unpack d95f3ad14dee633a758d2e331151e950dd13e4ed header
>    fatal: cannot read existing object info d95f3ad14dee633a758d2e331151e950dd13e4ed
>    fatal: index-pack failed
>    wrapper.c:568:52: runtime error: null pointer passed as argument 1, which is declared to never be null
>    /usr/include/unistd.h:825:48: note: nonnull attribute specified here
>    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior wrapper.c:568:52 in
>    Aborted
>
> Compiled with:
>    Debian clang version 11.0.0-5+b1
>    Target: x86_64-pc-linux-gnu
>    Thread model: posix
>    InstalledDir: /usr/bin

I see. I was compiling with: gcc 10.2.0, so setting CC=clang does
reproduce the error for me.

  Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor