[libcamera-devel] [PATCH 1/2] libcamera: base: log: Fix use of freed name

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Fri, Aug 26, 2022 at 02:39:30PM +0300, Tomi Valkeinen via libcamera-devel wrote:
> LogCategory just stores the char * that was given to it in the
> constructor, i.e. it refers to memory "outside" LogCategory. If the
> LogCategory is defined in a .so that is unloaded, then it leads to the
> LogCategory pointing to freed memory, causing a crash.
> 
> Fix this by taking a copy of the name by using a std::string instead of
> just storing the pointer.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I won't apply this yet though, as it doesn't help much without 2/2, in
case this patch needs to change in v2 due to changes in 2/2.

> ---
>  include/libcamera/base/log.h | 4 ++--
>  src/libcamera/base/log.cpp   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> index 3fc5ced3..8b462767 100644
> --- a/include/libcamera/base/log.h
> +++ b/include/libcamera/base/log.h
> @@ -31,14 +31,14 @@ class LogCategory
>  public:
>  	explicit LogCategory(const char *name);
>  
> -	const char *name() const { return name_; }
> +	const std::string &name() const { return name_; }
>  	LogSeverity severity() const { return severity_; }
>  	void setSeverity(LogSeverity severity);
>  
>  	static const LogCategory &defaultCategory();
>  
>  private:
> -	const char *name_;
> +	const std::string name_;
>  	LogSeverity severity_;
>  };
>  
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 5c359a22..a4a5b452 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -568,7 +568,7 @@ void Logger::logSetLevel(const char *category, const char *level)
>  		return;
>  
>  	for (LogCategory *c : categories_) {
> -		if (!strcmp(c->name(), category)) {
> +		if (c->name() == category) {
>  			c->setSeverity(severity);
>  			break;
>  		}

-- 
Regards,

Laurent Pinchart