md5sum makes unaligned word accesses...

[Rob Landley]

On Monday 15 May 2006 5:35 pm, Andre wrote:
> Rob Landley <rob at landley.net> wrote:
> > I just changed my copy of md5_hash() to look like:
> >
> > void md5_hash(const void *data, size_t length, md5_ctx_t *ctx)
> > {
> >     /* We can only call the hash function directly on data that's
> >      * 32-bit aligned and a multiple of 64 bytes. */
> >
> >     if ((length&63) || (data&3)) md5_hash_bytes(data, length, ctx);
> >     else md5_hash_block(data, length, ctx);
> > }
>
> Did you miss my message about the current md5_hash(),
> md5_hash_bytes(), md5_hash_block() API being broken regardless to
> alignment issues ??

Apparently I did, but I planned on running it against both a large file and a 
tree of smaller files and comparing the output with ubuntu's md5 command 
before checking it in.

> The following fails on all platforms, including x86...
>
> # echo -n "1234" > 4bytes
> # echo -n "abcdefghijklmnopabcdefghijklmnop" > 32bytes
> # cat 32bytes 32bytes > 64bytes
> #
> # ( cat 4bytes; sleep 1; cat 64bytes ) | md5sum
>
> Correct answer: 0314c5f1e2d1f87b9481e3a2e08ab452
> Busybox answer: 12076da5776df62e0c913dd3dad5d032

Still, a specific test we fail is always good. :)

> Please consider applying the attached patch before any alignment
> fixes.

Hmmm...  Force all access through the buffering function?  Ah, I see.  Small 
chunks of data followed by large aligned chunks of data can get out of 
sequence because one's bypassing the buffer and the other's not.

Good catch.

From a performance standpoint having one codepath is actually a bonus.  The 
memory allocation for the buffer always happens anyway, and from a 
performance standpoint the small buffer is cache-local and cache hot when 
it's processed.  It's more or less doing a DRAM prefetch, really.

(I'm dreadfully tempted to clean up the tangled #ifdefs for the various 
performance levels, but I'm resisting mightily.  If nothing else, that can be 
a second pass after this is working and checked in...)

> Andre

Hmmm...  Running md5sum on a large tree, I'm getting a segfault and it's not 
in the code I just changed...  Oh good grief.  You know all the big 
"performance range" stuff for libbb/md5.c that was put in because our md5sum 
performance sucks?  You know _WHY_ our md5sum performance sucks?  Because 
md5_sha1_sum.c is reading and processing the file in 4 byte chunks.  
(sizeof() and RESERVE_CONFIG_BUFFER() don't mix.)

Right.  I _am_ ripping out the tangled #ifdefs.  After I track down this 
segfault...

Rob
-- 
Never bet against the cheap plastic solution.