md5sum makes unaligned word accesses...

[Rob Landley]

On Friday 12 May 2006 4:56 am, Andre wrote:
> --- Mike Frysinger <vapier at gentoo.org> wrote:
> > > On Thursday 11 May 2006 2:05 am, Andre wrote:
> > > > Anyway, comparing the faulting PC with a disassembly of
> > > > busybox, it looks like the fault happens in md5_hash_block()
> > > >
> > > > void md5_hash_block(const void *buffer, size_t len,
> > > >                     md5_ctx_t *ctx)
> > > > {
> > > >     uint32_t correct_words[16];
> > > >     const uint32_t *words = buffer;
> > > >     ....
> > > >
> > > > Casting a void * to an unsigned int * is highly suspect unless
> > > > all callers of md5_has_block() somehow know that 'buffer' has
> > > > implicit alignment constraints...
> >
> > considering there is only one caller of md5_has_block(), this is
> > possible
>
> Currently, that one caller doesn't respect the constrains...
>
> In md5_hash_bytes(), if ctx->buflen is non-zero, bytes are stolen
> from the beginning of 'buffer' to create a complete hash block. Even
> if 'buffer' is initially aligned, alignment is lost if the number of
> bytes required to complete a previous hash block isn't a multiple of
> 4.
>
> > or in md5_hash_block(), make use of posix_memalign()
>
> Allocating a bounce buffer in md5_hash_block() would be a solution if
> md5_hash_block() really needs to be public (it used to be static in
> 1.1.0 ??)

My stalled passwd cleanup (too late to get it into 1.2.0) is calling that and 
sha1sum directly.  Then you can configure out support for legacy crypt and 
don't need libcrypt at all.

> and potential callers can't be trusted to respect 
> alignment...

Asking potential callers care about alignment when it only intermittently 
breaks on some arm systems is unlikely to avoid future problems.  At the very 
least we'd need some kind of DEBUG thing that would reliably break it when 
given an unaligned pointer.

> However, it might be less overhead to make md5_hash_block() static
> again and then fixup alignment in md5_hash_bytes() - at the point
> 'buffer' becomes misaligned, ctx->buffer is empty and can therefore
> be used as a bounce buffer without needing to allocate anything.
>
> Andre

Rob
-- 
Never bet against the cheap plastic solution.