[openssl-dev] 90-test_secmem.t hangs the machine for good

[Benjamin Kaduk]

On 05/12/2017 02:05 PM, Blumenthal, Uri - 0553 - MITLL wrote:
> On 5/12/17, 2:49 PM, "openssl-dev on behalf of Benjamin Kaduk via openssl-dev" <openssl-dev-bounces at openssl.org on behalf of openssl-dev at openssl.org> wrote:
>
> ➢ I’m sorry to report that in the current OpenSSL 1.1 master running “make test”
> ➢  freezes up the machine. Mac OS X 10.11.6, Xcode-8.2, current Github master. 
> ➢ Here’s the configuration:
>
>       A commit hash would be more useful than "current github master"
>
> I thought you know what’s in the current master right now. But here’s the last few hashes for your pleasure:

Well, you're right to think that I do, and I do trust you to be accurate
when you make that claim.  But there are many people for which I would
not fully trust the veracity of such a claim, the commit hash is
completely unambiguous, and it is a whole lot easier for those poor
folks reading this thread in the archive two years from now who are
trying to track down a similar-looking issue.  So, on the whole, I
recommend always using commit hashes, with an optional annotation of how
it relates to a branch.

> $ git log
> commit 80a2fc4100daf6f1001eee33ef2f9b9eee05bedf (HEAD -> master, origin/master, origin/HEAD)
> Author: Todd Short <tshort at akamai.com>
> Date:   Wed May 10 11:44:55 2017 -0400
>
>     Clean up SSL_OP_* a bit
>     
>     Reviewed-by: Matt Caswell <matt at openssl.org>
>     Reviewed-by: Rich Salz <rsalz at openssl.org>
>     (Merged from https://github.com/openssl/openssl/pull/3439)
>
> commit 33242d9d79e7f06151e905b83dc8f995006fa7cd
> Author: Rich Salz <rsalz at openssl.org>
> Date:   Thu May 11 20:42:32 2017 -0400
>
>     Use scalar, not length; fixes test_evp
>     
>     Reviewed-by: Stephen Henson <steve at openssl.org>
>     Reviewed-by: Richard Levitte <levitte at openssl.org>
>     (Merged from https://github.com/openssl/openssl/pull/3452)
>
>
>       I can understand not wanting to have to power-cycle the machine again,
>       but the 'make TESTS=test_secmem V=1 test' output (or some dtruss/similar)
>       would be helpful in tracking things down.

The obvious candidate for closer inspection is a few commits previous,

commit 7031ddac94d0ae616d1b0670263a9265ce672cd2
Author: Todd Short <tshort at akamai.com>
Date:   Thu May 11 15:48:10 2017 -0400

    Fix infinite loops in secure memory allocation.
   
    Issue 1:
   
    sh.bittable_size is a size_t but i is and int, which can result in
    freelist == -1 if sh.bittable_size exceeds an int.
   
    This seems to result in an OPENSSL_assert due to invalid allocation
    size, so maybe that is "ok."
   
    Worse, if sh.bittable_size is exactly 1<<31, then this becomes an
    infinite loop (because 1<<31 is a negative int, so it can be shifted
    right forever and sticks at -1).
   
    Issue 2:
   
    CRYPTO_secure_malloc_init() sets secure_mem_initialized=1 even when
    sh_init() returns 0.
   
    If sh_init() fails, we end up with secure_mem_initialized=1 but
    sh.minsize=0. If you then call secure_malloc(), which then calls,
    sh_malloc(), this then enters an infite loop since 0 << anything will
    never be larger than size.
   
    Issue 3:
   
    That same sh_malloc loop will loop forever for a size greater
    than size_t/2 because i will proceed (assuming sh.minsize=16):
    i=16, 32, 64, ..., size_t/8, size_t/4, size_t/2, 0, 0, 0, 0, ....
    This sequence will never be larger than "size".
   
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3449)

which adds test cases intended to trigger the edge cases being fixed.

> Sorry.
>
>       How locked up is the machine?  Can you get memory usage stats or is it completely unresponsive?
>
> Completely unresponsive. Totally. No memory usage. The only thing that works at this point is the power button.

It seems that there should also be a bug report against OS X, as regular
userspace code running as non-root should not be able to hang a machine
like that.

>From just looking at the code, the only question that comes to mind is
whether you have a 32- or 64-bit size_t in the build environment in
question, which is unlikely to cause a eureka moment :(

-Ben
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20170512/88097768/attachment-0001.html>