[PATCH 0/1] Use a log identity always valid while logging

[PATCH 0/1] Use a log identity always valid while logging

[Christian Meusel]

I encountered garbled and changing log identifiers from connmand 1.42
running on Yocto Kirkstone.

    Feb 09 02:40:18 foo time[590]: Connection Manager version 1.42
    Feb 09 02:40:18 foo time[590]: ../connman-1.42/src/dbus.c:__connman_dbus_init()
    Feb 09 02:40:18 foo time[590]: ../connman-1.42/src/main.c:parse_config() parsing main.conf
    Feb 09 02:40:18 foo time[590]: Online check disabled by main config.
    Feb 09 02:40:19 foo U[590]: ../connman-1.42/src/inotify.c:__connman_inotify_init()
    Feb 09 02:40:19 foo U[590]: ../connman-1.42/src/technology.c:__connman_technology_init()

This variant is slightly patched (see
https://git.yoctoproject.org/poky/tree/meta/recipes-connectivity/connman/connman/0001-src-log.c-Include-libgen.h-for-basename-API.patch)
which creates a use-after-free situation. But nevertheless there is no
lifetime requirement communicated for the 'program' argument passed to
'__connman_log_init' and on the other hand 'basename' might as well
return a pointer to an internal buffer.

In either case, a locally allocated log identity string will prevent
potential issues.

Best regards,

Christian


Christian Meusel (1):
  log: Use a log identity always valid while logging

 src/log.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/1] log: Use a log identity always valid while logging

[Christian Meusel]

The result from basename is not guaranteed to be.

I'm running connmand 1.42 on Yocto Kirkstone and in this setup log
identities ending up in the journal are garbage and change during the
run of a connmand process.

Explicitly allocating the log identity fixes this behaviour and gives
the expected 'connmand'.
---
 src/log.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/log.c b/src/log.c
index f7483194..108dcb0b 100644
--- a/src/log.c
+++ b/src/log.c
@@ -36,6 +36,11 @@
 
 static const char *program_exec;
 static const char *program_path;
+/*
+ * Provide a log identy to syslog whose lifetime is greater than the timespan
+ * we're logging. The result from basename does not guarantee that.
+ */
+static char *syslog_identity;
 
 /* This makes sure we always have a __debug section. */
 CONNMAN_DEBUG_ALIAS(dummy);
@@ -212,7 +217,11 @@ int __connman_log_init(const char *program, const char *debug,
 	if (backtrace)
 		signal_setup(signal_handler);
 
-	openlog(basename(program), option, LOG_DAEMON);
+	/* Clean up any previos identity and set the new one. */
+	g_free(syslog_identity);
+	syslog_identity = g_path_get_basename(program);
+
+	openlog(syslog_identity, option, LOG_DAEMON);
 
 	syslog(LOG_INFO, "%s version %s", program_name, program_version);
 
@@ -228,5 +237,8 @@ void __connman_log_cleanup(gboolean backtrace)
 	if (backtrace)
 		signal_setup(SIG_DFL);
 
+	g_free(syslog_identity);
+	syslog_identity = NULL;
+
 	g_strfreev(enabled);
 }
-- 
2.34.1

Re: [PATCH 0/1] Use a log identity always valid while logging

[Denis Kenzior]

Hi Christian,

On 2/22/24 17:23, Christian Meusel wrote:
> I encountered garbled and changing log identifiers from connmand 1.42
> running on Yocto Kirkstone.
> 
>      Feb 09 02:40:18 foo time[590]: Connection Manager version 1.42
>      Feb 09 02:40:18 foo time[590]: ../connman-1.42/src/dbus.c:__connman_dbus_init()
>      Feb 09 02:40:18 foo time[590]: ../connman-1.42/src/main.c:parse_config() parsing main.conf
>      Feb 09 02:40:18 foo time[590]: Online check disabled by main config.
>      Feb 09 02:40:19 foo U[590]: ../connman-1.42/src/inotify.c:__connman_inotify_init()
>      Feb 09 02:40:19 foo U[590]: ../connman-1.42/src/technology.c:__connman_technology_init()
> 
> This variant is slightly patched (see
> https://git.yoctoproject.org/poky/tree/meta/recipes-connectivity/connman/connman/0001-src-log.c-Include-libgen.h-for-basename-API.patch)

Ugh.  I think the Yocto folks didn't read the basename manpage carefully enough 
:)  See [1] for an explanation.

> which creates a use-after-free situation. But nevertheless there is no
> lifetime requirement communicated for the 'program' argument passed to
> '__connman_log_init' and on the other hand 'basename' might as well
> return a pointer to an internal buffer.
> 
> In either case, a locally allocated log identity string will prevent
> potential issues.

argv[0] is being passed in as 'program'.  So getting rid of basename use and 
running strrchr should be sufficient.

Regards,
-Denis

[1] 
https://git.kernel.org/pub/scm/network/wireless/iwd.git/commit/?id=ba5a6df2d170fce96354f0c9ba281fe049d2f0a3