[libcamera-devel,v2,1/2] libcamera: base: log: Fix use of freed name
diff mbox series

Message ID 20220829084445.61585-2-tomi.valkeinen@ideasonboard.com
State Accepted
Headers show
Series
  • LogCategory fixing
Related show

Commit Message

Tomi Valkeinen Aug. 29, 2022, 8:44 a.m. UTC 
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@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/base/log.h | 4 ++--
 src/libcamera/base/log.cpp   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Kieran Bingham Oct. 19, 2022, 12:59 p.m. UTC | #1 
Quoting Tomi Valkeinen via libcamera-devel (2022-08-29 09:44:44)
> 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@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  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;
>                 }
> -- 
> 2.34.1
>

Patch
diff mbox series 

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;
 		}