Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 17 Jun 2012 01:25:54 +0200
From: Tavis Ormandy <taviso@...xchg8b.com>
To: john-dev@...ts.openwall.com
Subject: Re: Re: [patch] optional new raw sha1 implemetation

On Sun, Jun 17, 2012 at 12:38:10AM +0200, magnum wrote:
> On 2012-06-17 00:15, magnum wrote:
> >Tavis,
> >
> >I see a problem in binary(). You alloc space for a binary and return it
> >to john, but john will only copy it so this is a memory leak. We could
> >change it to a one-time alloc:
> >
> >static uint32_t *result;
> >if (!result)
> >result = mem_alloc_tiny(SHA1_DIGEST_SIZE, MEM_ALIGN_SIMD);
> >
> >However, as the alignment requirement is not really in binary() itself,
> >we could just as well do this:
> >
> >static uint32_t result[SHA1_DIGEST_SIZE/sizeof(uint32_t)];
> >
> >And actually no matter how we do this, john will (currently) copy it
> >with MEM_ALIGN_WORD in loader.c.
> 
> After reading the rest of the code, MEM_ALIGN_WORD seem to be OK.
> I'll commit the static fix.
> 
> magnum

Oops, the static fix sounds okay, I didn't know john didn't take
ownership of that buffer. Thanks for the benchmark output, those are
encouraging results.

Regarding switching memrchr to strrchr, I dont think this is correct,
they are strings on input, but I store them in a format that can be
converted to SHA-1 input very quickly and there is no guarantee there
is a nul byte at the end.

I guess I can provide my own prototype, or I guess I can just use
__builtin_memchr, if you prefer.

I was just working on implementing one of Simons suggestions, let me
know which you prefer and I'll include it.

Tavis.

-- 
-------------------------------------
taviso@...xchg8b.com | pgp encrypted mail preferred
-------------------------------------------------------

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ