Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130127231701.GA6692@openwall.com>
Date: Mon, 28 Jan 2013 03:17:01 +0400
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Bug in a number of "crack array" formats

magnum, Dhiru -

On Sun, Jan 27, 2013 at 11:35:44PM +0100, magnum wrote:
> You can't reset the array in set_salt() because if we only have one salt, set_salt() will only be called once and never again. I get a deja-vu now, I am pretty sure we have told you this before.

We discussed similar issues before, but maybe not this specific one.

> I think you should do this:
> 
> 1. If using an "any_cracked" variable, reset it in crypt_all() before the OMP loop.

I agree.

> 2. Always set *or reset* cracked[index] in crypt_all(). Ie. do not just set it on successfull crack but also reset it otherwise.

This may have performance impact with fast (non-)hashes and OpenMP, due
to false sharing, which may occur due to the cracked[] elements being
tiny and thus close together.

It may be better to memset() the entire cracked array to 0, but only if
any_cracked was previously set.  That is, crypt_all() may start with:

	if (any_cracked) {
		memset(cracked, 0, cracked_size);
		any_cracked = 0;
	}

before it proceeds with the loop and OpenMP stuff.

IIRC, it's the exact same code that is currently in set_salt() in some
formats.  It only needs to be moved to the beginning of crypt_all(),
because of the special case of having only one salt.

Thanks,

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.