Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 18 Mar 2012 02:14:03 +0400
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: SSH thread-safety

On Sat, Mar 17, 2012 at 10:01:19AM +0530, Dhiru Kholia wrote:
> On Sat, Mar 17, 2012 at 9:45 AM, Solar Designer <solar@...nwall.com> wrote:
> > The speed is slightly better than it was before this round of changes
> > (that is, before your introduction of locking). :-)  I am getting almost
> > 700k c/s on 8-core.  I am running the high thread count tests now.
> 
> Wish I had an 8-core (or 16-core) CPU. :-)

Actually, I run these tests on a dual quad-core machine now (so it has 8
cores across two CPU chips), not on one 8-core CPU.  (I'll try on my
so-called 8-core FX-8120 later.)

> > My biggest concern is that the code is highly non-portable to different
> > versions of OpenSSL now.  I am not sure what we can do about that.
> 
> The only way to ensure compatibility is to test with all major version
> of OpenSSL between 0.9.7 and 1.0.1 (current). I think that the changes
> required to ensure portability wouldn't be very hard to do.

Well, we can only test for versions of OpenSSL that currently exist.
Since we're using undocumented interfaces (right?), things are bound to
break in future versions even when OpenSSL developers do not intend to
break backwards compatibility (for their documented interfaces).

As an alternative, we could consider enhancing OpenSSL itself such that
it'd scale to high thread counts even with our proper use of documented
interfaces only (with locking callbacks provided if needed).  We'd need
to submit such enhancements upstream.

We could also combine both approaches: for versions of OpenSSL that do
not yet have our enhancements, use the undocumented interfaces, and for
newer versions with the enhancements use the documented interfaces only.

Anyway, the code currently in magnum-jumbo works well for me.  I've
attached a very minor patch against it.  This patch is not strictly
needed, but it avoids unnecessary calls to cmp_one() and memset().
It also no longer makes the assumption that assignments to int type
variables are atomic and have no side-effects (although I am not aware
of relevant platforms where this would not be the case; I was only aware
of the issue with pre-BWX Alpha for smaller-than-int variables).

Alexander

View attachment "ssh_fmt.c.diff" of type "text/plain" (2241 bytes)

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.