Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

USE_RWLOCK option freeze with multi-fork software #5376

Closed
mickar opened this issue Feb 15, 2018 · 7 comments
Closed

USE_RWLOCK option freeze with multi-fork software #5376

mickar opened this issue Feb 15, 2018 · 7 comments

Comments

@mickar
Copy link

mickar commented Feb 15, 2018

Hi,
When I used the RAND_bytes function in my software with multi-fork, this one was frozen because the pthread_rwlock_init was not initialised with PTHREAD_PROCESS_SHARED attribute.
How can I resolve this issue ?
Best regards,

crypto/threads_pthread.c:26
@mspncp
Copy link
Contributor

mspncp commented Feb 16, 2018

Thank your for taking the time to report this issue. Could you please provide some more details to help analyzing the problem?

  • What is your operating system and how did you configure openssl?
  • What precisely does your application do before it 'freezes'?
  • Can you provide a call stack or - even better - a minimal example to reproduce the issue?
  • You claim that the problem is caused by the missing PTHREAD_PROCESS_SHARED flag. Did you verify that adding the flag resolves the problem?

@mickar
Copy link
Author

mickar commented Feb 16, 2018

I use Linux (Debian - kernel 4.10.15) with the OpenSSL version 1.1.0f (25 May 2017).
You can use the following sample of function to reproduce the bug only with a multi-fork process.

int generate_transaction_id(unsigned char *id, unsigned short size)
{
        int err;
        char tmp[TRANSACTION_ID];
        if (size != TRANSACTION_ID || id == NULL){
                return (int) -1;
        }
        if (RAND_bytes((unsigned char*)tmp, (int)size) == 1) {
                memcpy(id, tmp, size);
                return (int) 0;
        }
        return (int) -1;
}

My application used the RAND_bytes function to generate a transaction ID.

(gdb) backtrace
#0  0x00007f6037a3e450 in futex_wait (private=<optimized out>, expected=8, futex_word=0x7f60386d4ebc) at ../sysdeps/unix/sysv/linux/futex-internal.h:61
#1  futex_wait_simple (private=<optimized out>, expected=8, futex_word=0x7f60386d4ebc) at ../sysdeps/nptl/futex-internal.h:135
#2  __pthread_rwlock_wrlock_slow (rwlock=0x7f60386d4eb0) at pthread_rwlock_wrlock.c:67
#3  0x00007f6037e0cee9 in CRYPTO_THREAD_write_lock () from /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1
#4  0x00007f6037ddbfd3 in ?? () from /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1
#5  0x00007f602dfb05e2 in generate_transaction_id (id=0x7ffd544ab524 "\026MW\251\020p~X`\177", size=12) at rand_generator.c:26
#..........
#..........

In fact, all my forks using this function can be block if the pthread_rwlock_wrlock function is called at the same time (withtout an initialisation with PTHREAD_PROCESS_SHARED attribute).
I fixed this issue by adding another pthread_rwlock using the PTHREAD_PROCESS_SHARED attribute before calling RAND_bytes function.

https://linux.die.net/man/3/pthread_mutexattr_init

.....Synchronization variables that are initialized with the PTHREAD_PROCESS_PRIVATE process-shared attribute may only be operated on by threads in the process that initialized them. Synchronization variables that are initialized with the PTHREAD_PROCESS_SHARED process-shared attribute may be operated on by any thread in any process that has access to it. In particular, these processes may exist beyond the lifetime of the initializing process......

@kaduk
Copy link
Contributor

kaduk commented Feb 16, 2018

Can you read the commit message for commit 63ab5ea and confirm which of the enumerated cases your application fits into? I fear that it sounds like you are in case (4), which is basically hopeless to attempt to support.

@mickar
Copy link
Author

mickar commented Feb 27, 2018

Hi,
Indeed, we are in a similar case.
Thx.

@mspncp
Copy link
Contributor

mspncp commented Feb 27, 2018 •
edited

Then good look! A quick internet search will show you that mixing threads and forks (i.e., forks without immediate exec) means looking for trouble. It is extremely hard to get everything right. Maybe you should reconsider your application design whether it is possible to stick with one of the two.

Can this issue be closed now?

@mickar
Copy link
Author

mickar commented Feb 27, 2018

Yes of course, my software use only multi-forks (like Kamailio). I don't use multi-threads in forks, but the issue is the same. In my case it is more the case(2) with 'N' forks.
So, we cannot use OpenSSL with multi-fork software safely. We can close this issue.
Thx

@mickar mickar closed this as completed Feb 27, 2018
@mspncp
Copy link
Contributor

mspncp commented Feb 27, 2018

In general, OpenSSL can be used safely in case (2) without problems. However, it looks like the first sentence of the paragraph below applies.

(2) Single-threaded, calls into OpenSSL after fork()

The application must ensure that it does not fork() with an unexpected
lock held (that is, one that would get unlocked in the parent but
accidentally remain locked in the child and cause deadlock).  Since
OpenSSL does not expose any of its internal locks to the application
and the application is single-threaded, the OpenSSL internal locks
will be unlocked for the fork(), and the state will be consistent.
(OpenSSL will need to reseed its PRNG in the child, but that is
an orthogonal issue.)  If the application makes use of locks from
libcrypto, proper handling for those locks is the responsibility of
the application, as for any other locking primitive that is available
for application programming.

It would be nice to know for sure whether it's a bug in your application or in OpenSSL that triggers the double lock. Could you provide us with a minimalistic compilable example that exhibits this bug?

If yes, then please reopen this issue and post the code here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kaduk @mickar @mspncp