[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4F1562C2.4040105@hushmail.com>
Date: Tue, 17 Jan 2012 13:00:02 +0100
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: Recent CVS patches
On 01/17/2012 10:02 AM, Solar Designer wrote:
>> It would be a pity dropping that test. Especially since most of us use
>> intel. Actually I think it's easier to make a format return aligned data
>> (needed or not), than to add the flag (because we'd need to take care
>> that such a flag is proper). From a quick glance there were just six
>> formats that failed this, four binary() and two salt(). I can fix them
>> instead, even if not really needed for those formats.
>
> What are these six?
I have already made trivial fixes to all of them but we can revert some
of them. They were: BFEgg and MYSQL (fmt_default_binary), DOMINOSEC
(using a struct that can't ever have both binary and salt aligned), EPI,
hmailserver and oracle (static char salt[]), PO (static char binary[])
and oracle11 (other bugs, not alignment).
Of these, I believe BFEgg, MYSQL and DOMINOSEC does not really need the
fixes while the others do. It now occurs to me you could test if a
format is using fmt_default_binary or fmt_default_binary_hash, and/or
fmt_default_salt or fmt_default_salt_hash and disable the tests for
these. I believe that would have solved it for these three.
> The self-tests currently pass the pointer returned by split()
> into binary() and salt(). If that pointer happens to be aligned, then a
> binary() or/and salt() merely returning the same pointer won't be
> detected as an error, but this may fail in another build. Perhaps we
> can deliberately misalign the pointer passed into binary() and salt() by
> the self-tests (we'll have to copy the string returned by split() for
> that).
I think that is a very good idea.
> But there can be other cases of binary() or/and salt() happening
> to return an aligned pointer while having no guarantee that it will do
> so in another build. Some of these would be problematic on most non-x86
> architectures anyway, but as you correctly point out some are non-issues
> everywhere. So as it is, this test potentially turns some non-issues
> into issues that will unnecessarily show up on some builds.
>
> Thus, I think the test needs to be disabled by default, maybe put within
> an #ifdef DEBUG block that we'd only re-enable for our own testing, but
> not for releases intended for end-users.
It may be that just skipping the alignment tests for fmt_default_* will
suffice for mitigating this current issue. But DEBUG might be a good
idea regardless. There are other tests and asserts in formats we could
turn on under DEBUG too. It could also be used for format self-tests
that would skew the benchmarks.
magnum
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.
