shlibloadtest failure on non-Linux

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

shlibloadtest failure on non-Linux

Nick Reilly
Hi,
I just ported OpenSSL 1.1.0e to QNX and ran in to an issue on
shlibloadtest and it trying to unload the shared libraries in
conjunction with the code in crypto/init.c

1) crypto/init.c at line 106 deliberately leaks a reference to prevent
the library from unloading unless OPENSSL_USE_NODELETE is defined, yet
shlibloadtest doesn't test for whether this is set. I think the
shlibloadtest should only test the dlclose() return value on if
OPENSSL_USE_NODELETE is set.

2) crypto/init.c at line 77 does "atexit(OPENSSL_cleanup)". If I try
defining OPENSSL_USE_NODELETE then this atexit() handler is meant to
cleanup on unload of the shared library, but this meaning of atexit() is
Linux specific. It is not required in POSIX and the Linux manpage lists
this usage under the "Linux notes" section. In QNX there must be no
atexit() handlers registered if the library is being unloaded, otherwise
there is a forced abort() called from the dynamic linker. I think that
rather than using the atexit() mechanism some other mechanism e.g.
function attribute of destructor on gcc should be used.

Regards,
Nick.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: shlibloadtest failure on non-Linux

OpenSSL - Dev mailing list
> doesn't test for whether this is set. I think the shlibloadtest should only test
> the dlclose() return value on if OPENSSL_USE_NODELETE is set.

Please see https://github.com/openssl/openssl/pull/3399
 
> 2) crypto/init.c at line 77 does "atexit(OPENSSL_cleanup)". If I try defining
> OPENSSL_USE_NODELETE then this atexit() handler is meant to cleanup on
> unload of the shared library, but this meaning of atexit() is Linux specific. It is
> not required in POSIX and the Linux manpage lists this usage under the
> "Linux notes" section.

Does changing the guard to this work?
#if !defined(OPENSSL_SYS_UEFI) &&  !defined(OPENSSL_SYS_QNX)

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: shlibloadtest failure on non-Linux

Matt Caswell-2
In reply to this post by Nick Reilly


On 09/05/17 14:43, Nick Reilly wrote:

> Hi,
> I just ported OpenSSL 1.1.0e to QNX and ran in to an issue on
> shlibloadtest and it trying to unload the shared libraries in
> conjunction with the code in crypto/init.c
>
> 1) crypto/init.c at line 106 deliberately leaks a reference to prevent
> the library from unloading unless OPENSSL_USE_NODELETE is defined, yet
> shlibloadtest doesn't test for whether this is set. I think the
> shlibloadtest should only test the dlclose() return value on if
> OPENSSL_USE_NODELETE is set.

I'm not sure why this makes a difference. The return value of dlclose()
tells you whether there has been an error or not. Not whether the
library has actually been removed from address space. Or am I missing
your point?

>
> 2) crypto/init.c at line 77 does "atexit(OPENSSL_cleanup)". If I try
> defining OPENSSL_USE_NODELETE then this atexit() handler is meant to
> cleanup on unload of the shared library, but this meaning of atexit() is
> Linux specific.

No. The whole point of OPENSSL_USE_NODELETE is to indicate that the
library won't be unloaded until process exit because we've ensured that
through some other mechanism (e.g. because we built it using
"-znodelete" on Linux). We do not rely on the linux specific behaviour.
If your platform doesn't have some other mechanism then you need to use
the default "deliberate leak" approach.

Matt
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: shlibloadtest failure on non-Linux

Matt Caswell-2
In reply to this post by OpenSSL - Dev mailing list


On 09/05/17 15:03, Salz, Rich via openssl-dev wrote:

>> doesn't test for whether this is set. I think the shlibloadtest should only test
>> the dlclose() return value on if OPENSSL_USE_NODELETE is set.
>
> Please see https://github.com/openssl/openssl/pull/3399
>  
>> 2) crypto/init.c at line 77 does "atexit(OPENSSL_cleanup)". If I try defining
>> OPENSSL_USE_NODELETE then this atexit() handler is meant to cleanup on
>> unload of the shared library, but this meaning of atexit() is Linux specific. It is
>> not required in POSIX and the Linux manpage lists this usage under the
>> "Linux notes" section.
>
> Does changing the guard to this work?
> #if !defined(OPENSSL_SYS_UEFI) &&  !defined(OPENSSL_SYS_QNX)
>

That presumably would mean that the library would not clean up at all on
QNX, which is probably undesirable.

Matt
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: shlibloadtest failure on non-Linux

Nick Reilly
In reply to this post by Matt Caswell-2


On 2017-05-09 10:08 AM, Matt Caswell wrote:
>
> I'm not sure why this makes a difference. The return value of dlclose()
> tells you whether there has been an error or not. Not whether the
> library has actually been removed from address space. Or am I missing
> your point?

dlclose() is returning an error and dlerror() reports that the library
cannot be closed as it is still in use (from the leaked reference).

> No. The whole point of OPENSSL_USE_NODELETE is to indicate that the
> library won't be unloaded until process exit because we've ensured that
> through some other mechanism (e.g. because we built it using
> "-znodelete" on Linux). We do not rely on the linux specific behaviour.
> If your platform doesn't have some other mechanism then you need to use
> the default "deliberate leak" approach.

Ok, misunderstood the intention of OPENSSL_USE_NODELETE, didn't realise
it meant -znodelete.

Thanks,
Nick.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: shlibloadtest failure on non-Linux

Nick Reilly
In reply to this post by OpenSSL - Dev mailing list


On 2017-05-09 10:03 AM, Salz, Rich via openssl-dev wrote:
>> doesn't test for whether this is set. I think the shlibloadtest should only test
>> the dlclose() return value on if OPENSSL_USE_NODELETE is set.
>
> Please see https://github.com/openssl/openssl/pull/3399
>

Sense of OPENSSL_USE_NODELETE has been reversed i.e. you want #ifdef and
not #ifndef

Also I think its still fair game to try a dlclose() just that the
dlclose() may return an error if OPENSSL_USE_NODELETE is not defined.

Regards,
Nick.
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: shlibloadtest failure on non-Linux

OpenSSL - Dev mailing list
> Sense of OPENSSL_USE_NODELETE has been reversed i.e. you want #ifdef
> and not #ifndef

Argh, you're right.
 
> Also I think its still fair game to try a dlclose() just that the
> dlclose() may return an error if OPENSSL_USE_NODELETE is not defined.

I'll just leave it as-is.  Thanks for looking at this!
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: shlibloadtest failure on non-Linux

Matt Caswell-2
In reply to this post by Nick Reilly


On 09/05/17 18:12, Nick Reilly wrote:

>
>
> On 2017-05-09 10:08 AM, Matt Caswell wrote:
>>
>> I'm not sure why this makes a difference. The return value of dlclose()
>> tells you whether there has been an error or not. Not whether the
>> library has actually been removed from address space. Or am I missing
>> your point?
>
> dlclose() is returning an error and dlerror() reports that the library
> cannot be closed as it is still in use (from the leaked reference).

Hmmm. Arguably that is broken dlclose() behaviour. dlclose() is just
supposed to inform the system that a handle previously returned by
dlopen() is no longer required by the application. AFAIK there is no
requirement for all other handles returned from other dlopen() open
calls for the same shared library to be closed first (how would that
work anyway...unless there was a strict requirement for the handle
returned from the first call to dlopen() to always be the last one to be
used in a call to dlclose()?).

This is not a problem on other non-linux Unix based systems that we have
tried it on, so it looks to be something peculiar to QNX.

Does QNX have a "-znodelete" equivalent? That would be the most
preferable fix.

Another option might be something like this:

diff --git a/test/shlibloadtest.c b/test/shlibloadtest.c
index 6f220ba530..bc701b4333 100644
--- a/test/shlibloadtest.c
+++ b/test/shlibloadtest.c
@@ -65,8 +65,16 @@ static int shlib_sym(SHLIB lib, const char *symname,
SHLIB_SYM *sym)

 static int shlib_close(SHLIB lib)
 {
+#ifdefined OPENSSL_SYS_QNX
+    /*
+     * Ignore errors from dlclose() on QNX to avoid spurious complaints
about
+     * being unable to close it due to it still being in use.
+     */
+    dlclose(lib);
+#else
     if (dlclose(lib) != 0)
         return 0;
+#endif

     return 1;
 }

Matt

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev