git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@osdl.org>
To: Martin Langhoff <martin.langhoff@gmail.com>,
	Paul Mackerras <paulus@samba.org>
Cc: Nicolas Pitre <nico@cam.org>, Jon Smirl <jonsmirl@gmail.com>,
	git <git@vger.kernel.org>
Subject: Broken PPC sha1.. (Re: Figured out how to get Mozilla into git)
Date: Sun, 18 Jun 2006 15:51:02 -0700 (PDT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0606181543270.5498@g5.osdl.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0606181532130.5498@g5.osdl.org>




On Sun, 18 Jun 2006, Linus Torvalds wrote:
> 
> On Mon, 19 Jun 2006, Martin Langhoff wrote:
> > 
> > No problems here with my latest import run. fsck-objects --full comes
> > clean, takes 14m:
> >
> > /usr/bin/time git-fsck-objects --full
> > 737.22user 38.79system 14:09.40elapsed 91%CPU (0avgtext+0avgdata 0maxresident)k
> > 0inputs+0outputs (20807major+19483471minor)pagefaults 0swaps
> 
> It takes much less than that for me: 
> 
> 	408.40user 32.56system 7:22.07elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
> 	0inputs+0outputs (145major+13455672minor)pagefaults 0swaps

Ok, re-building the thing with MOZILLA_SHA1=1 rather than my default 
PPC_SHA1=1 fixes the problem. I no longer get that "SHA1 mismatch with 
itself" on the pack-file.

Sadly, it also takes a _lot_ longer to fsck.

Paul - I think the ppc SHA1_Update() overflows in 32 bits, when the length 
of the memory area to be checksummed is huge.

In particular, the pack-file is 535MB in size, and the way we check the 
SHA1 checksum is by just mapping it all, doing a single SHA1_Update() over 
the whole pack-file, and comparing the end result with the internal SHA1 
at the end of the pack-file.

The PPC SHA1_Update() function starts off with:

	int SHA1_Update(SHA_CTX *c, const void *ptr, unsigned long n)
	{
	...
		c->len += n << 3;

which will obviously overflow if "n" is bigger than 29 bits, ie 512MB.

So doing the length in bits (or whatever that "<<3" is there for) doesn't 
seem to be such a great idea.

I guess we could make the caller just always chunk it up, but wouldn't it 
be nice to fix the PPC SHA1 implementation instead?

That said, the _only_ thing this will ever trigger on in practice is 
exactly this one case: a large packfile whose checksum was _correctly_ 
generated - because pack-file generation does it in IO chunks using the 
csum-file interfaces - but that will be incorrectly checked because we 
check it all at once.

So as bugs go, it's a fairly benign one.

			Linus

  reply	other threads:[~2006-06-18 22:51 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-06-09  2:17 Figured out how to get Mozilla into git Jon Smirl
2006-06-09  2:56 ` Nicolas Pitre
2006-06-09  3:06 ` Martin Langhoff
2006-06-09  3:28   ` Jon Smirl
2006-06-09  7:17     ` Jakub Narebski
2006-06-09 15:01       ` Linus Torvalds
2006-06-09 16:11         ` Nicolas Pitre
2006-06-09 16:30           ` Linus Torvalds
2006-06-09 17:38             ` Nicolas Pitre
2006-06-09 17:49               ` Linus Torvalds
2006-06-09 17:10           ` Jakub Narebski
2006-06-09 18:13   ` Jon Smirl
2006-06-09 19:00     ` Linus Torvalds
2006-06-09 20:17       ` Jon Smirl
2006-06-09 20:40         ` Linus Torvalds
2006-06-09 20:56           ` Jon Smirl
2006-06-09 21:57             ` Linus Torvalds
2006-06-09 22:17               ` Linus Torvalds
2006-06-09 23:16               ` Greg KH
2006-06-09 23:37               ` Martin Langhoff
2006-06-09 23:43                 ` Linus Torvalds
2006-06-10  0:00                   ` Jon Smirl
2006-06-10  0:11                     ` Linus Torvalds
2006-06-10  0:16                       ` Jon Smirl
2006-06-10  0:45                         ` Jon Smirl
2006-06-09 20:44         ` Jakub Narebski
2006-06-09 21:05         ` Nicolas Pitre
2006-06-09 21:46           ` Jon Smirl
2006-06-10  1:23         ` Martin Langhoff
2006-06-10  1:14   ` Martin Langhoff
2006-06-10  1:33     ` Linus Torvalds
2006-06-10  1:43       ` Linus Torvalds
2006-06-10  1:48         ` Jon Smirl
2006-06-10  1:59           ` Linus Torvalds
2006-06-10  2:21             ` Jon Smirl
2006-06-10  2:34               ` Carl Worth
2006-06-10  3:08                 ` Linus Torvalds
2006-06-10  8:21                   ` Jakub Narebski
2006-06-10  9:00                     ` Junio C Hamano
2006-06-10  8:36                   ` Rogan Dawes
2006-06-10  9:08                     ` Junio C Hamano
2006-06-10 14:47                       ` Rogan Dawes
2006-06-10 14:58                         ` Jakub Narebski
2006-06-10 15:14                         ` Nicolas Pitre
2006-06-10 17:53                     ` Linus Torvalds
2006-06-10 18:02                       ` Jon Smirl
2006-06-10 18:36                       ` Rogan Dawes
2006-06-10  3:01               ` Linus Torvalds
2006-06-10  2:30             ` Jon Smirl
2006-06-10  3:41             ` Martin Langhoff
2006-06-10  3:55               ` Junio C Hamano
2006-06-10  4:02               ` Linus Torvalds
2006-06-10  4:11                 ` Linus Torvalds
2006-06-10  6:02                   ` Jon Smirl
2006-06-10  6:15                     ` Junio C Hamano
2006-06-10 15:44                       ` Jon Smirl
2006-06-10 16:15                         ` Timo Hirvonen
2006-06-10 18:37                         ` Petr Baudis
2006-06-10 18:55                         ` Lars Johannsen
2006-06-11 22:00       ` Nicolas Pitre
2006-06-18 19:26         ` Linus Torvalds
2006-06-18 21:40           ` Martin Langhoff
2006-06-18 22:36             ` Linus Torvalds
2006-06-18 22:51               ` Linus Torvalds [this message]
2006-06-18 23:25                 ` [PATCH] Fix PPC SHA1 routine for large input buffers Paul Mackerras
2006-06-19  5:02                   ` Linus Torvalds
2006-06-09  3:12 ` Figured out how to get Mozilla into git Pavel Roskin
  -- strict thread matches above, loose matches on Subject: below --
2006-06-19  8:41 Broken PPC sha1.. (Re: Figured out how to get Mozilla into git) linux
2006-06-19  8:50 ` Johannes Schindelin

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

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

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0606181543270.5498@g5.osdl.org \
    --to=torvalds@osdl.org \
    --cc=git@vger.kernel.org \
    --cc=jonsmirl@gmail.com \
    --cc=martin.langhoff@gmail.com \
    --cc=nico@cam.org \
    --cc=paulus@samba.org \
    /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.
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 NNTP newsgroup(s).