git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Jeff King <peff@peff.net>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Luat Nguyen" <root@l4w.io>,
	git@vger.kernel.org
Subject: Re: [PATCH 1/3] ewah_read_mmap: bounds-check mmap reads
Date: Sat, 16 Jun 2018 16:35:13 +0200	[thread overview]
Message-ID: <20180616143513.10086-1-szeder.dev@gmail.com> (raw)
In-Reply-To: <20180615033112.GA20390@sigill.intra.peff.net>

> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
> index 423c0a475f..237ee6e5fc 100755
> --- a/t/t5310-pack-bitmaps.sh
> +++ b/t/t5310-pack-bitmaps.sh
> @@ -331,4 +331,17 @@ test_expect_success 'pack reuse respects --incremental' '
>  	git show-index <empty.idx >actual &&
>  	test_cmp expect actual
>  '
> +
> +test_expect_success 'truncated bitmap fails gracefully' '
> +	git repack -ad &&
> +	git rev-list --use-bitmap-index --count --all >expect &&
> +	bitmap=$(ls .git/objects/pack/*.bitmap) &&
> +	test_when_finished "rm -f $bitmap" &&
> +	head -c 512 <$bitmap >$bitmap.tmp &&
> +	mv $bitmap.tmp $bitmap &&

This line turns out to be problematic on OSX and ultimately causes the
test to fail.

When OSX's 'mv's destination is read-only, it asks whether to replace
the destination even though in the test its stdin is not a terminal
(and thus doesn't conform to POSIX[1]).  Since the '.bitmap' file is
read-only, and since 'mv' obviously doesn't get an affirmative
response from /dev/null, the original '.bitmap' file is not
overwritten, the subsequent 'git rev-list' doesn't print any error
message, and finally 'test_i18ngrep' causes the test to fail.

The relevant part of the '-x' test output on Travis CI:

  ++mv .git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap.tmp .git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap
  override r--r--r--  travis/staff for .git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap? (y/n [n]) not overwritten
  ++git rev-list --use-bitmap-index --count --all
  ++test_cmp expect actual
  ++diff -u expect actual
  ++test_i18ngrep corrupt stderr
  ++eval 'last_arg=${2}'
  +++last_arg=stderr
  ++test -f stderr
  ++test 2 -lt 2
  ++test 'x!' = xcorrupt
  ++test -n ''
  ++test 'x!' = xcorrupt
  ++grep corrupt stderr
  ++echo 'error: '\''grep corrupt' 'stderr'\'' didn'\''t find a match in:'
  error: 'grep corrupt stderr' didn't find a match in:
  ++test -s stderr
  ++echo '<File '\''stderr'\'' is empty>'
  <File 'stderr' is empty>
  ++return 1
  error: last command exited with $?=1
  not ok 43 - truncated bitmap fails gracefully

As far as I can tell, 'mv -f' appears to make the test work on OSX as
well.

I've run a build job with an additional 'grep ^override
t/test-results/*.out' command following the tests to see whether there
are any other cases where OSX 'mv' doesn't overwrite a read-only file
without causing the tests to fail, but found nothing.  (But note that
the OSX build jobs don't run all tests.)

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mv.html

> +	git rev-list --use-bitmap-index --count --all >actual 2>stderr &&
> +	test_cmp expect actual &&
> +	test_i18ngrep corrupt stderr
> +'
> +

  parent reply	other threads:[~2018-06-16 14:35 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-14 22:59 security: potential out-of-bound read at ewah_io.c |ewah_read_mmap| Luat Nguyen
2018-06-15  3:28 ` Jeff King
2018-06-15  3:31   ` [PATCH 1/3] ewah_read_mmap: bounds-check mmap reads Jeff King
2018-06-15  9:14     ` SZEDER Gábor
2018-06-15 16:20       ` Junio C Hamano
2018-06-15 17:10         ` SZEDER Gábor
2018-06-15 17:21           ` Jeff King
2018-06-15 19:42             ` Junio C Hamano
2018-06-15 17:05     ` Junio C Hamano
2018-06-15 17:26       ` Jeff King
2018-06-15 19:44         ` Junio C Hamano
2018-06-16 14:35     ` SZEDER Gábor [this message]
2018-06-16 19:14       ` Jeff King
2018-06-15  3:31   ` [PATCH 2/3] ewah: drop ewah_deserialize function Jeff King
2018-06-15  3:32   ` [PATCH 3/3] ewah: drop ewah_serialize_native function Jeff King
2018-06-15 13:56     ` Ramsay Jones
2018-06-15 14:07       ` Ramsay Jones
2018-06-15 14:30         ` [PATCH 0/8] Delete unused methods in EWAH bitmap Derrick Stolee
2018-06-15 14:30           ` [PATCH 1/8] ewah/bitmap.c: delete unused 'bitmap_clear()' Derrick Stolee
2018-06-15 14:46             ` Ramsay Jones
2018-06-15 15:11               ` Derrick Stolee
2018-06-15 14:30           ` [PATCH 2/8] ewah/bitmap.c: delete unused 'bitmap_each_bit()' Derrick Stolee
2018-06-15 15:03             ` Ramsay Jones
2018-06-15 14:30           ` [PATCH 3/8] ewah_bitmap: delete unused 'ewah_and()' Derrick Stolee
2018-06-15 14:30           ` [PATCH 4/8] ewah_bitmap: delete unused 'ewah_and_not()' Derrick Stolee
2018-06-15 14:30           ` [PATCH 5/8] ewah_bitmap: delete unused 'ewah_not()' Derrick Stolee
2018-06-15 14:30           ` [PATCH 6/8] ewah_bitmap: delete unused 'ewah_or()' Derrick Stolee
2018-06-15 14:30           ` [PATCH 7/8] ewah_io: delete unused 'ewah_serialize()' Derrick Stolee
2018-06-15 14:30           ` [PATCH 8/8] ewah_io: delete unused 'ewah_serialize_native()' Derrick Stolee
2018-06-15 15:01             ` Ramsay Jones
2018-06-15 15:10               ` Derrick Stolee
2018-06-15 14:35           ` [PATCH 0/8] Delete unused methods in EWAH bitmap Derrick Stolee
2018-06-15 18:27           ` [PATCH v2 0/7] " Derrick Stolee
2018-06-15 18:27             ` [PATCH v2 1/7] ewah/bitmap.c: delete unused 'bitmap_clear()' Derrick Stolee
2018-06-15 18:27             ` [PATCH v2 2/7] ewah/bitmap.c: delete unused 'bitmap_each_bit()' Derrick Stolee
2018-06-15 18:27             ` [PATCH v2 3/7] ewah_bitmap: delete unused 'ewah_and()' Derrick Stolee
2018-06-15 18:27             ` [PATCH v2 4/7] ewah_bitmap: delete unused 'ewah_and_not()' Derrick Stolee
2018-06-15 18:27             ` [PATCH v2 5/7] ewah_bitmap: delete unused 'ewah_not()' Derrick Stolee
2018-06-15 18:27             ` [PATCH v2 6/7] ewah_bitmap: delete unused 'ewah_or()' Derrick Stolee
2018-06-15 18:27             ` [PATCH v2 7/7] ewah_io: delete unused 'ewah_serialize()' Derrick Stolee
2018-06-15 18:51             ` [PATCH v2 0/7] Delete unused methods in EWAH bitmap Junio C Hamano
2018-06-15 18:56               ` Derrick Stolee
2018-06-15 19:48                 ` Junio C Hamano
2018-06-15 20:35                   ` Jeff King
2018-06-15 14:15       ` [PATCH 3/3] ewah: drop ewah_serialize_native function Derrick Stolee
2018-06-15 17:51         ` Jeff King
2018-06-15 18:33           ` Junio C Hamano
2018-06-15 18:46             ` Jeff King
2018-06-15  3:44   ` [PATCH 4/3] ewah: adjust callers of ewah_read_mmap() Jeff King
2018-06-15 11:23     ` Derrick Stolee
2018-06-15 16:41       ` Junio C Hamano
2018-06-15 17:31         ` Jeff King
2018-06-15 18:23           ` Derrick Stolee
2018-06-15 20:38             ` Jeff King
2018-06-15 17:12     ` Junio C Hamano
2018-06-15 16:11   ` security: potential out-of-bound read at ewah_io.c |ewah_read_mmap| Junio C Hamano
2018-06-19 19:00 ` Dyer, Edwin
2018-06-19 19:56   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180616143513.10086-1-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=root@l4w.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).