Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110510234312.GA8483@openwall.com>
Date: Wed, 11 May 2011 03:43:12 +0400
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: single mode problem

On Wed, May 11, 2011 at 01:00:40AM +0200, Samuele Giovanni Tonon wrote:
> salt = single_db->salts;
> do {
>     single_alloc_keys(&salt->keys);
> } while ((salt = salt->next));
> 
> so single_alloc_keys is called for every password found
> in the file

More correctly, for every salt.  With a good salts distribution, this is
the same as for every password hash loaded, though.

> and it allocates max_keys_per_crypt * password length which

No, it uses min_keys_per_crypt (not max).

> for opencl is SSHA_NUM_KEYS * PLAINTEXT_LENGTH  so
> 1024 * 2048 * 32 bytes for each key .
> 
> i could lower ssha_num_keys but this would hurt performance.
> Any suggestion on how to fix this problem ?

Set only min_keys_per_crypt to a lower value, but keep
max_keys_per_crypt high.

> N.B. i noted that, when single mode is gone memory is still allocated
> cause there is no free(salt) : is it intended ?

Sort of.  IIRC, this pre-dates the introduction of batch mode, so it
did not matter.  Also, these memory allocations are done with
mem_alloc_tiny(), so we're saving on malloc() overhead, but this means
that they can't be freed.  We may try to move to mem_alloc() there,
which will increase JtR's peak memory usage, but allow for the memory to
be freed.  However, it is uncertain whether this memory will actually be
returned back to the OS or not - it depends on libc's memory allocator,
its use of brk/sbrk vs. mmap, and possible fragmentation.  Since the
for-each-salt loop makes these allocations sequentially without
allocating anything else inbetween, I think there's a good chance the
memory will actually be returned to the OS.  An optimization could be to
combine the two allocations made by single_alloc_keys() into one call to
mem_alloc().  An even more effective optimization could be to combine
all of these allocations into one call to mem_alloc().

...Oh, here's a better idea.  We could introduce a way to revert all
mem_alloc_tiny() allocations up to a certain saved state.  This is in
line with JimF's work on being able to detect unintended memory leaks.
Perhaps I will just implement that.

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.