[libcamera-devel,v3,5/7] libcamera: ipa_manager: use proxy

Message ID 20190709184450.32023-6-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • Add IPA process isolation
Related show

Commit Message

Paul Elder July 9, 2019, 6:44 p.m. UTC
Make IPAManager isolate an IPA in a Proxy if the IPA's license is not
open source, before returning the IPA to the caller. For now, only use
the default Linux IPA proxy, and only LGPL 2.1+ is considered open
source.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes in v3:
- license checking is done with SPDX license strings, and only LGPL 2.1+
  is accepted for now

New in v2
- replaces adding shims
- since Proxies are not external shared objects like the shims in v1
  were, there is no longer a list of shims that is treated like
  IPAModules
- instead the matching is done by searching the list of proxy factories

 src/libcamera/ipa_manager.cpp | 46 +++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart July 11, 2019, 6:22 a.m. UTC | #1
Hi Paul,

Thank you for the patch.

I think you missed my review on v2.

On Wed, Jul 10, 2019 at 03:44:48AM +0900, Paul Elder wrote:
> Make IPAManager isolate an IPA in a Proxy if the IPA's license is not
> open source, before returning the IPA to the caller. For now, only use
> the default Linux IPA proxy, and only LGPL 2.1+ is considered open
> source.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> Changes in v3:
> - license checking is done with SPDX license strings, and only LGPL 2.1+
>   is accepted for now
> 
> New in v2
> - replaces adding shims
> - since Proxies are not external shared objects like the shims in v1
>   were, there is no longer a list of shims that is treated like
>   IPAModules
> - instead the matching is done by searching the list of proxy factories
> 
>  src/libcamera/ipa_manager.cpp | 46 +++++++++++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index 532b77d..49a12ca 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -14,6 +14,7 @@
>  #include "ipa_module.h"
>  #include "log.h"
>  #include "pipeline_handler.h"
> +#include "ipa_proxy.h"
>  #include "utils.h"
>  
>  /**
> @@ -25,6 +26,18 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPAManager)
>  
> +namespace {
> +
> +bool isOpenSource(const char *license)
> +{
> +	if (!strcmp(license, "LGPL-2.1-or-later"))
> +		return true;
> +
> +	return false;
> +}
> +
> +} /* namespace */
> +
>  /**
>   * \class IPAManager
>   * \brief Manager for IPA modules
> @@ -129,7 +142,7 @@ int IPAManager::addDir(const char *libDir)
>   * \param[in] maxVersion Maximum acceptable version of IPA module
>   *
>   * \return A newly created IPA interface, or nullptr if no matching
> - * IPA module is found
> + * IPA module is found or if the IPA interface fails to initialize
>   */
>  std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,
>  						    uint32_t maxVersion,
> @@ -144,7 +157,36 @@ std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,
>  		}
>  	}
>  
> -	if (!m || !m->load())
> +	if (!m)
> +		return nullptr;
> +
> +	if (!isOpenSource(m->info().license)) {
> +		IPAProxyFactory *pf = nullptr;
> +		std::vector<IPAProxyFactory *> &factories = IPAProxyFactory::factories();
> +
> +		for (IPAProxyFactory *factory : factories) {
> +			/* TODO: Better matching */
> +			if (!strcmp(factory->name().c_str(), "IPAProxyLinux")) {
> +				pf = factory;
> +				break;
> +			}
> +		}
> +
> +		if (!pf) {
> +			LOG(IPAManager, Error) << "Failed to get proxy factory";
> +			return nullptr;
> +		}
> +
> +		std::unique_ptr<IPAProxy> proxy = pf->create(m);
> +		if (!proxy->isValid()) {
> +			LOG(IPAManager, Error) << "Failed to load proxy";
> +			return nullptr;
> +		}
> +
> +		return proxy;
> +	}
> +
> +	if (!m->load())
>  		return nullptr;
>  
>  	return m->createInstance();

Patch

diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index 532b77d..49a12ca 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -14,6 +14,7 @@ 
 #include "ipa_module.h"
 #include "log.h"
 #include "pipeline_handler.h"
+#include "ipa_proxy.h"
 #include "utils.h"
 
 /**
@@ -25,6 +26,18 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(IPAManager)
 
+namespace {
+
+bool isOpenSource(const char *license)
+{
+	if (!strcmp(license, "LGPL-2.1-or-later"))
+		return true;
+
+	return false;
+}
+
+} /* namespace */
+
 /**
  * \class IPAManager
  * \brief Manager for IPA modules
@@ -129,7 +142,7 @@  int IPAManager::addDir(const char *libDir)
  * \param[in] maxVersion Maximum acceptable version of IPA module
  *
  * \return A newly created IPA interface, or nullptr if no matching
- * IPA module is found
+ * IPA module is found or if the IPA interface fails to initialize
  */
 std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,
 						    uint32_t maxVersion,
@@ -144,7 +157,36 @@  std::unique_ptr<IPAInterface> IPAManager::createIPA(PipelineHandler *pipe,
 		}
 	}
 
-	if (!m || !m->load())
+	if (!m)
+		return nullptr;
+
+	if (!isOpenSource(m->info().license)) {
+		IPAProxyFactory *pf = nullptr;
+		std::vector<IPAProxyFactory *> &factories = IPAProxyFactory::factories();
+
+		for (IPAProxyFactory *factory : factories) {
+			/* TODO: Better matching */
+			if (!strcmp(factory->name().c_str(), "IPAProxyLinux")) {
+				pf = factory;
+				break;
+			}
+		}
+
+		if (!pf) {
+			LOG(IPAManager, Error) << "Failed to get proxy factory";
+			return nullptr;
+		}
+
+		std::unique_ptr<IPAProxy> proxy = pf->create(m);
+		if (!proxy->isValid()) {
+			LOG(IPAManager, Error) << "Failed to load proxy";
+			return nullptr;
+		}
+
+		return proxy;
+	}
+
+	if (!m->load())
 		return nullptr;
 
 	return m->createInstance();