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.