mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Guard against globally shared UNO ref accessed from wrong UNO env

Browse Source 
connectivity/source/drivers/jdbc/jdbc.component has

  environment="@CPPU_ENV@:affine"

to place the com.sun.star.comp.sdbc.JDBCDriver implementation into an affine UNOenvironment.

The com.sun.star.sdbcx.comp.hsqldb.Driver implementation (in the normal C++ UNO
environment), in ODriverDelegator::connect
(connectivity/source/drivers/hsqldb/HDriver.cxx), calls
StorageContainer::registerStorage to store an XStorage in a global map, then
calls the JDBCDriver (i.e., thread enters the affine environment), which calls
via (non-UNO) JNI into hsqldb.jar code, which in turn calls via (non-UNO) JNI
into Java_com_sun_star_sdbcx_comp_hsqldb_StorageFileAccess_isStreamElement
(connectivity/source/drivers/hsqldb/StorageFileAccess.cxx), which uses
StorageContainer::getRegisteredStorage to obtain the XStorage and use it.

But that XStorage is the original C++ object, not a proxy that witnesses the
mapping between the normal C++ and the affine UNO environment.  (And the thread
is still in the affine environment, after having passed through the Java stack
frames via non-UNO JNI.)

That does not necessarily cause any problems immediately (so apparently went
unnoticed for quite a while), but when the XStorage-implementation in turn wants
to obtain the SimpleLogRing singleton, it would now trigger the

  std::abort();//TODO

in cppuhelper/source/servicemanager.cxx (where the invocation-by-constructor
case hasn't yet been implemented for differing environments), when that
singleton is changed to use the constructor feature in
<https://gerrit.libreoffice.org/#/c/22020/> "tdf#74608: Constructor function for
SimpleLogRing singleton."

So just do any necessary mapping every time an XStorage stored in the static
StorageContainer is accessed.

Change-Id: I91a62fd7e1cec29026f70a2c3acdfe051885c0fa
This commit is contained in:
Stephan Bergmann
2016-02-23 16:39:48 +01:00
parent 9610a5aebd
commit 03a271901c
3 changed files with 51 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
connectivity/source/drivers/hsqldb/HStorageMap.cxx
View File

@@ -25,6 +25,7 @@
#include <com/sun/star/lang/DisposedException.hpp>
#include <osl/diagnose.h>
#include <osl/thread.h>
#include <uno/mapping.hxx>
#include <algorithm>
namespace connectivity
@@ -99,6 +100,28 @@ namespace connectivity
return m_xSeek;
}
css::uno::Reference<css::embed::XStorage> StorageData::mapStorage()
const
{
css::uno::Environment env(css::uno::Environment::getCurrent());
if (!(env.is() && storageEnvironment.is())) {
throw css::uno::RuntimeException("cannot get environments");
}
if (env.get() == storageEnvironment.get()) {
return storage;
} else {
css::uno::Mapping map(storageEnvironment, env);
if (!map.is()) {
throw css::uno::RuntimeException("cannot get mapping");
}
css::uno::Reference<css::embed::XStorage> mapped;
map.mapInterface(
reinterpret_cast<void **>(&mapped), storage.get(),
cppu::UnoType<css::embed::XStorage>::get());
return mapped;
}
}
TStorages& lcl_getStorageMap()
{
static TStorages s_aMap;
@@ -170,12 +193,12 @@ namespace connectivity
// check if the storage is already in our map
TStorages::iterator aFind = ::std::find_if(rMap.begin(),rMap.end(),
[&_xStorage] (const TStorages::value_type& storage) {
return storage.second.storage == _xStorage;
return storage.second.mapStorage() == _xStorage;
});
if ( aFind == rMap.end() )
{
aFind = rMap.insert(TStorages::value_type(lcl_getNextCount(), {_xStorage, _sURL, TStreamMap()})).first;
aFind = rMap.insert(TStorages::value_type(lcl_getNextCount(), {_xStorage, css::uno::Environment::getCurrent(), _sURL, TStreamMap()})).first;
}
return aFind->first;
@@ -201,7 +224,7 @@ namespace connectivity
// check if the storage is already in our map
TStorages::iterator aFind = ::std::find_if(rMap.begin(),rMap.end(),
[&_xStorage] (const TStorages::value_type& storage) {
return storage.second.storage == _xStorage;
return storage.second.mapStorage() == _xStorage;
});
if ( aFind != rMap.end() )
@@ -219,10 +242,10 @@ namespace connectivity
{
if ( _xListener.is() )
{
Reference<XTransactionBroadcaster> xBroad(aFind->second.storage,UNO_QUERY);
Reference<XTransactionBroadcaster> xBroad(aFind->second.mapStorage(),UNO_QUERY);
if ( xBroad.is() )
xBroad->removeTransactionListener(_xListener);
Reference<XTransactedObject> xTrans(aFind->second.storage,UNO_QUERY);
Reference<XTransactedObject> xTrans(aFind->second.mapStorage(),UNO_QUERY);
if ( xTrans.is() )
xTrans->commit();
}
@@ -244,8 +267,9 @@ namespace connectivity
if ( aFind != rMap.end() )
{
TStorages::mapped_type aStoragePair = StorageContainer::getRegisteredStorage(sKey);
OSL_ENSURE(aStoragePair.storage.is(),"No Storage available!");
if ( aStoragePair.storage.is() )
auto storage = aStoragePair.mapStorage();
OSL_ENSURE(storage.is(),"No Storage available!");
if ( storage.is() )
{
OUString sOrgName = StorageContainer::jstring2ustring(env,name);
OUString sName = removeURLPrefix(sOrgName,aStoragePair.url);
@@ -261,7 +285,7 @@ namespace connectivity
{
try
{
pHelper.reset(new StreamHelper(aStoragePair.storage->openStreamElement(sName,_nMode)));
pHelper.reset(new StreamHelper(storage->openStreamElement(sName,_nMode)));
}
catch(const Exception&)
{
@@ -272,7 +296,7 @@ namespace connectivity
bool bIsStream = true;
try
{
bIsStream = aStoragePair.storage->isStreamElement(sStrippedName);
bIsStream = storage->isStreamElement(sStrippedName);
}
catch(const Exception&)
{
@@ -281,7 +305,7 @@ namespace connectivity
if ( !bIsStream )
return pHelper; // readonly file without data stream
}
pHelper.reset( new StreamHelper(aStoragePair.storage->openStreamElement( sStrippedName, _nMode ) ) );
pHelper.reset( new StreamHelper(storage->openStreamElement( sStrippedName, _nMode ) ) );
}
aFind->second.streams.insert(TStreamMap::value_type(sName,pHelper));
}

19
connectivity/source/drivers/hsqldb/StorageFileAccess.cxx
View File

@@ -43,7 +43,8 @@ extern "C" SAL_JNI_EXPORT jboolean JNICALL Java_com_sun_star_sdbcx_comp_hsqldb_S
(JNIEnv * env, jobject /*obj_this*/,jstring key, jstring name)
{
TStorages::mapped_type aStoragePair = StorageContainer::getRegisteredStorage(StorageContainer::jstring2ustring(env,key));
if ( aStoragePair.storage.is() )
auto storage = aStoragePair.mapStorage();
if ( storage.is() )
{
try
{
@@ -51,11 +52,11 @@ extern "C" SAL_JNI_EXPORT jboolean JNICALL Java_com_sun_star_sdbcx_comp_hsqldb_S
try
{
OUString sOldName = StorageContainer::removeOldURLPrefix(sName);
if ( aStoragePair.storage->isStreamElement(sOldName) )
if ( storage->isStreamElement(sOldName) )
{
try
{
aStoragePair.storage->renameElement(sOldName,StorageContainer::removeURLPrefix(sName,aStoragePair.url));
storage->renameElement(sOldName,StorageContainer::removeURLPrefix(sName,aStoragePair.url));
}
catch(const Exception&)
{
@@ -68,7 +69,7 @@ extern "C" SAL_JNI_EXPORT jboolean JNICALL Java_com_sun_star_sdbcx_comp_hsqldb_S
catch(const IllegalArgumentException&)
{
}
return aStoragePair.storage->isStreamElement(StorageContainer::removeURLPrefix(sName,aStoragePair.url));
return storage->isStreamElement(StorageContainer::removeURLPrefix(sName,aStoragePair.url));
}
catch(const NoSuchElementException&)
{
@@ -101,11 +102,12 @@ extern "C" SAL_JNI_EXPORT void JNICALL Java_com_sun_star_sdbcx_comp_hsqldb_Stora
}
#endif
TStorages::mapped_type aStoragePair = StorageContainer::getRegisteredStorage(StorageContainer::jstring2ustring(env,key));
if ( aStoragePair.storage.is() )
auto storage = aStoragePair.mapStorage();
if ( storage.is() )
{
try
{
aStoragePair.storage->removeElement(StorageContainer::removeURLPrefix(StorageContainer::jstring2ustring(env,name),aStoragePair.url));
storage->removeElement(StorageContainer::removeURLPrefix(StorageContainer::jstring2ustring(env,name),aStoragePair.url));
}
catch(const NoSuchElementException&)
{
@@ -137,11 +139,12 @@ extern "C" SAL_JNI_EXPORT void JNICALL Java_com_sun_star_sdbcx_comp_hsqldb_Stora
}
#endif
TStorages::mapped_type aStoragePair = StorageContainer::getRegisteredStorage(StorageContainer::jstring2ustring(env,key));
if ( aStoragePair.storage.is() )
auto storage = aStoragePair.mapStorage();
if ( storage.is() )
{
try
{
aStoragePair.storage->renameElement(
storage->renameElement(
StorageContainer::removeURLPrefix(StorageContainer::jstring2ustring(env,oldname),aStoragePair.url),
StorageContainer::removeURLPrefix(StorageContainer::jstring2ustring(env,newname),aStoragePair.url)
);

6
connectivity/source/inc/hsqldb/HStorageMap.hxx
View File

@@ -31,6 +31,7 @@
#include <com/sun/star/io/XInputStream.hpp>
#include <com/sun/star/io/XSeekable.hpp>
#include <jni.h>
#include <uno/environment.hxx>
namespace connectivity
{
@@ -54,11 +55,16 @@ namespace connectivity
typedef std::map< OUString, std::shared_ptr<StreamHelper> > TStreamMap;
struct StorageData {
css::uno::Reference<css::embed::XStorage> storage;
css::uno::Environment storageEnvironment;
OUString url;
TStreamMap streams;
css::uno::Reference<css::embed::XStorage> mapStorage() const;
};
typedef std::map<OUString, StorageData> TStorages;
/** contains all storages so far accessed.
*/