[botan-devel] Multithreaded Execution Crash - access violation in get_cipher(...)

Raja kalx91 at gmail.com
Tue Jul 5 06:21:33 EDT 2011


I took a look at the Botan source (for v1.8.11) and think I may have
found the culprit, a race condition within the AlgorithmFactory. I
have little experience with Botan code and terminology so bare with me
:)

The call to get_cipher eventually makes its way down the chain to:
   Keyed_Filter* default_Engine::get_cipher(const std::string&
algo_spec, Cipher_Dir direction, Algorithm_Factory& af) //def_mode.cpp

In this method, there is a line which grabs the BlockCipher from the
shared AlgorithmFactory object:
   const BlockCipher* block_cipher = af.prototype_block_cipher(cipher_name);

which makes it way to a factory method that searches the
AlgorithmFactory for the desired BlockCipher/Algorithm:
   template<typename T> const T* factory_prototype(const std::string&
algo_spec, const std::string& provider, const std::vector<Engine*>&
engines, Algorithm_Factory& af, Algorithm_Cache<T>* cache)

This function operates in this manner:
   1. Look for the desired cipher in the 'cache' and return it if it's there
   2. Otherwise, find it and if found add it to the cache based on
name. When adding it to the cache, *it calls delete on the cache slot*
before adding the value.
   3. Look for the desired cipher in the 'cache' again and return it

The cache->get(..)  and cache->add(..) methods are synchronized
internally with a mutex, but there is no synchronization on the
outside in the factory_prototype method.

I believe the situation I'm seeing is when the program starts, two
threads arrive to step1 above at the same time. Both can't find the
desired cipher in the cache, so they move on to step2. Thread#1
finishes step2 and step3 and returns the cipher pointer. Thread#2 then
starts step2 and calls cache->add() and here's the problem, calls
delete on the existing cache item, and replaces it with the new one.
This means Thread#1's block_cipher reference is now a bad pointer.

The disassembly I included before showed the crash occurred trying to
call another function immediately after calling get_bc_pad(...). The
call it failed on was likely "block_cipher->clone()" as block_cipher
would have been a bad pointer.

Assuming this is all correct, then there are two workarounds:
1. Add a mutex lock around the get_cipher(..) call.
2. Or, on program startup, after initializing the LibraryInitializer,
I can call get_cipher(...) to force the desired cipher into the cache.
After this, any threads that are started would avoid the race
condition described above.

Thanks,
 Raja

On Mon, Jul 4, 2011 at 9:00 AM,  <botan-devel-request at randombit.net> wrote:
> Send botan-devel mailing list submissions to
>        botan-devel at randombit.net
>
> To subscribe or unsubscribe via the World Wide Web, visit
>        http://lists.randombit.net/mailman/listinfo/botan-devel
> or, via email, send a message with subject or body 'help' to
>        botan-devel-request at randombit.net
>
> You can reach the person managing the list at
>        botan-devel-owner at randombit.net
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of botan-devel digest..."
>
>
> Today's Topics:
>
>   1. Multithreaded Execution Crash - access violation in
>      get_cipher(...) (Raja)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Sun, 3 Jul 2011 17:07:37 -0700
> From: Raja <kalx91 at gmail.com>
> To: botan-devel at randombit.net
> Subject: [botan-devel] Multithreaded Execution Crash - access
>        violation in    get_cipher(...)
> Message-ID:
>        <CAKoeDn5CfOjTHs77ds1HFE=wPnXbhit4625aV=g4yo3mXjegqA at mail.gmail.com>
> Content-Type: text/plain; charset=ISO-8859-1
>
> Environment:
> Visual Studio 2010
> Compiling against pre-built library for Botan-1.8.11
>
> Hi,
>
> >From my understanding, using Botan in a multithreaded fashion has two
> requirements:
> 1. The LibraryInitializer needs to be initalized with the
> 'thread_safe' parameter, eg:
> Botan::LibraryInitializer::initialize("thread_safe=true");
> 2. Explicit locking must be used if I create and share Botan objects
> across multiple threads
>
> In my test program, I initialize the LibraryInitializer as indicated
> above at the start of the program's main(). I am then launching two
> threads to perform AES/CBC/PKCS7 encryption. Each thread has it's own
> encryption helper class instance with it's own set of Botan objects
> (SymmetricKey, InitializationVector, Pipe, etc). When each thread
> finishes, I launch another thread to repeat. Threads are all
> encrypting the same source file but outputting to different target
> files.
>
> //Class Members
> Botan::SecureVector<Botan::byte> *m_pRawKey;
> Botan::SymmetricKey *m_pKey;
> Botan::SecureVector<Botan::byte> *m_pRawIV;
> Botan::InitializationVector *m_pIV;
> Botan::Pipe *m_pProcessPipe;
>
> //Example init function
> bool AESEncryptor::initialize(unsigned char * iv, int ivLen, unsigned
> char * key, int keyLen)
> {
>        // iv points to a valid unsigned char[16] buffer, ivLen is always 16
>        m_pRawIV = new Botan::SecureVector<Botan::byte>(iv, ivLen);
>        m_pIV = new Botan::InitializationVector(*m_pRawIV);
>
>        // key points to a valid unsigned char[16] buffer, keyLen is always 16
>        m_pRawKey = new Botan::SecureVector<Botan::byte>(key, keyLen));
>        m_pKey = new Botan::SymmetricKey(*m_pRawKey);
>
>        // create encryptor
>        m_pProcessPipe = new Botan::Pipe(Botan::get_cipher("AES/CBC/PKCS7",
> *m_pKey, *m_pIV, Botan::ENCRYPTION)); //Crashing line
>        m_pProcessPipe->start_msg();
> }
>
> I am seeing what appears to be a random Access Violation crash in the
> call to Botan::get_cipher(...) above. Are there any known issues with
> this method and multi-threading? Do I need to lock around it?
>
> After further testing, I am able to repro the crash 90% of the time if
> I make the two threads break on the get_cipher(..) line, freeze them,
> and then continue execution (by freezing both threads there and
> continuing execution, my assumption is I'm increasing the likeliness
> both threads enter the get_ciper function at the same time thus
> resulting in the crash). The crash only seems to occur at the
> beginning on the first set of execution threads. If it makes it past
> that, none of the future launched threads seem to have any problems,
> even when using the freeze trick I mentioned. Is there some shared
> object being allocated inside get_cipher(...)?
>
> I am compiling against a prebuilt Botan lib so I wasn't able to step
> in to the Botan source, however when the Access Violation occurs,
> Visual Studio shows the thread as being inside
> "Default_Engine::get_cipher()". Disassembly shows:
> 01150C9D  call        Botan::`anonymous namespace'::get_bc_pad (10DD87Eh)
> 01150CA2  mov         edx,dword ptr [esi]
> 01150CA4  add         esp,4
> 01150CA7  push        eax
> ---> 01150CA8  mov         eax,dword ptr [edx+0Ch]  <--- Debugger
> points to this instruction execution. EDX appears to have a bad value
> of FEEEFEEE
> 01150CAB  mov         ecx,esi
> 01150CAD  call        eax
>
> Thanks in advance for any help in this matter. :)
>
> Thanks,
>  Raja
>
>
> ------------------------------
>
> _______________________________________________
> botan-devel mailing list
> botan-devel at randombit.net
> http://lists.randombit.net/mailman/listinfo/botan-devel
>
>
> End of botan-devel Digest, Vol 78, Issue 2
> ******************************************
>



More information about the botan-devel mailing list