import tpm2-abrmd-2.3.3-3.el8

This commit is contained in:
CentOS Sources 2022-11-08 01:59:42 -05:00 committed by Stepan Oksanichenko
parent e62278f0aa
commit e46fc2b6eb
7 changed files with 421 additions and 1 deletions

View File

@ -0,0 +1,211 @@
From ff90674fd801dd369231a20c47ebef0d08402e9e Mon Sep 17 00:00:00 2001
From: William Roberts <william.c.roberts@intel.com>
Date: Tue, 12 Jan 2021 14:12:48 -0600
Subject: [PATCH 1/6] tabrmd-options: fix memory leak
The tabrmd_options_t structure is initialized with static char *
strings. These strings can be replaced by g_option_context_parse().
However, g_option_context_parse() replaces the string with allocated
memory and thus needs a call to g_free. Either one would need to keep
track if glib allocated the string and conditionally free it, or just
set all the strings to glib allocated strings. This patch takes the
approach of always allocating the option strings.
Fixes leaks like:
==2677142==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 9 byte(s) in 1 object(s) allocated from:
#8 0x7fbd1acd5da1 in g_option_context_parse (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5eda1)
#9 0x4cc438 in parse_opts /home/wcrobert/workspace/tpm2-abrmd/src/tabrmd-options.c:103:10
#10 0x4c7ffe in main /home/wcrobert/workspace/tpm2-abrmd/src/tabrmd.c:41:10
#11 0x7fbd1a8770b2 in __libc_start_main /build/glibc-ZN95T4/glibc-2.31/csu/../csu/libc-start.c:308:16
#12 0x42004d in _start (/home/wcrobert/workspace/tpm2-abrmd/src/tpm2-abrmd+0x42004d)
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
src/tabrmd-init.c | 2 ++
src/tabrmd-options.c | 53 +++++++++++++++++++++++++++++++++++++++-----
src/tabrmd-options.h | 11 +++++----
src/tabrmd.c | 4 +++-
4 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/src/tabrmd-init.c b/src/tabrmd-init.c
index 2ad7539..58e0103 100644
--- a/src/tabrmd-init.c
+++ b/src/tabrmd-init.c
@@ -99,6 +99,8 @@ gmain_data_cleanup (gmain_data_t *data)
if (data->loop != NULL) {
main_loop_quit (data->loop);
}
+
+ tabrmd_options_free(&data->options);
}
/*
* This function initializes and configures all of the long-lived objects
diff --git a/src/tabrmd-options.c b/src/tabrmd-options.c
index 0dd7b87..22f249c 100644
--- a/src/tabrmd-options.c
+++ b/src/tabrmd-options.c
@@ -16,6 +16,12 @@
#define G_OPTION_FLAG_NONE 0
#endif
+#define SET_STR_IF_NULL(var, value) \
+ do { \
+ var = var == NULL ? g_strdup(value) : var; \
+ g_assert(var); \
+ } while(0)
+
/*
* This is a GOptionArgFunc callback invoked from the GOption processor from
* the parse_opts function below. It will be called when the daemon is
@@ -36,6 +42,22 @@ show_version (const gchar *option_name,
g_print ("tpm2-abrmd version %s\n", VERSION);
exit (0);
}
+
+/**
+ * Frees internal memory associated with a tabrmd_options_t struct.
+ * @param opts
+ * The options to free, note it doesn't free opts itself.
+ */
+void
+tabrmd_options_free(tabrmd_options_t *opts)
+{
+ g_assert(opts);
+
+ g_clear_pointer(&opts->dbus_name, g_free);
+ g_clear_pointer(&opts->prng_seed_file, g_free);
+ g_clear_pointer(&opts->tcti_conf, g_free);
+}
+
/**
* This function parses the parameter argument vector and populates the
* parameter 'options' structure with data needed to configure the tabrmd.
@@ -51,7 +73,7 @@ parse_opts (gint argc,
gchar *argv[],
tabrmd_options_t *options)
{
- gchar *logger_name = "stdout";
+ gchar *logger_name = NULL;
GOptionContext *ctx;
GError *err = NULL;
gboolean session_bus = FALSE;
@@ -105,33 +127,52 @@ parse_opts (gint argc,
return FALSE;
}
g_option_context_free (ctx);
+
+ /*
+ * Set unset STRING options to defaults, we do this so we can free allocated
+ * string options with gfree, having a mix of const and allocated ptr's
+ * causes leaks
+ */
+ SET_STR_IF_NULL(options->dbus_name, TABRMD_DBUS_NAME_DEFAULT);
+ SET_STR_IF_NULL(options->prng_seed_file, TABRMD_ENTROPY_SRC_DEFAULT);
+ SET_STR_IF_NULL(options->tcti_conf, TABRMD_TCTI_CONF_DEFAULT);
+ SET_STR_IF_NULL(logger_name, "stdout");
+
/* select the bus type, default to G_BUS_TYPE_SESSION */
options->bus = session_bus ? G_BUS_TYPE_SESSION : G_BUS_TYPE_SYSTEM;
- if (set_logger (logger_name) == -1) {
+ gint ret = set_logger (logger_name);
+ if (ret == -1) {
g_critical ("Unknown logger: %s, try --help\n", logger_name);
- return FALSE;
+ g_free(logger_name);
+ goto error;
}
+ g_free(logger_name);
+
if (options->max_connections < 1 ||
options->max_connections > TABRMD_CONNECTION_MAX)
{
g_critical ("maximum number of connections must be between 1 "
"and %d", TABRMD_CONNECTION_MAX);
- return FALSE;
+ goto error;
}
if (options->max_sessions < 1 ||
options->max_sessions > TABRMD_SESSIONS_MAX_DEFAULT)
{
g_critical ("max-sessions must be between 1 and %d",
TABRMD_SESSIONS_MAX_DEFAULT);
- return FALSE;
+ goto error;
}
if (options->max_transients < 1 ||
options->max_transients > TABRMD_TRANSIENT_MAX)
{
g_critical ("max-trans-obj parameter must be between 1 and %d",
TABRMD_TRANSIENT_MAX);
- return FALSE;
+ goto error;
}
g_warning ("tcti_conf after: \"%s\"", options->tcti_conf);
return TRUE;
+
+error:
+ tabrmd_options_free(options);
+ return FALSE;
}
diff --git a/src/tabrmd-options.h b/src/tabrmd-options.h
index 4994920..d6bcfe9 100644
--- a/src/tabrmd-options.h
+++ b/src/tabrmd-options.h
@@ -15,10 +15,10 @@
.max_connections = TABRMD_CONNECTIONS_MAX_DEFAULT, \
.max_transients = TABRMD_TRANSIENT_MAX_DEFAULT, \
.max_sessions = TABRMD_SESSIONS_MAX_DEFAULT, \
- .dbus_name = TABRMD_DBUS_NAME_DEFAULT, \
- .prng_seed_file = TABRMD_ENTROPY_SRC_DEFAULT, \
+ .dbus_name = NULL, \
+ .prng_seed_file = NULL, \
.allow_root = FALSE, \
- .tcti_conf = TABRMD_TCTI_CONF_DEFAULT, \
+ .tcti_conf = NULL, \
}
typedef struct tabrmd_options {
@@ -28,7 +28,7 @@ typedef struct tabrmd_options {
guint max_transients;
guint max_sessions;
gchar *dbus_name;
- const gchar *prng_seed_file;
+ gchar *prng_seed_file;
gboolean allow_root;
gchar *tcti_conf;
} tabrmd_options_t;
@@ -38,4 +38,7 @@ parse_opts (gint argc,
gchar *argv[],
tabrmd_options_t *options);
+void
+tabrmd_options_free(tabrmd_options_t *opts);
+
#endif /* TABRMD_OPTIONS_H */
diff --git a/src/tabrmd.c b/src/tabrmd.c
index 7c93e90..e015de3 100644
--- a/src/tabrmd.c
+++ b/src/tabrmd.c
@@ -43,7 +43,8 @@ main (int argc, char *argv[])
}
if (geteuid() == 0 && ! gmain_data.options.allow_root) {
g_print ("Refusing to run as root. Pass --allow-root if you know what you are doing.\n");
- return 1;
+ ret = 1;
+ goto out;
}
g_mutex_init (&gmain_data.init_mutex);
@@ -63,6 +64,7 @@ main (int argc, char *argv[])
if (ret == 0 && gmain_data.ipc_disconnected) {
ret = EX_IOERR;
}
+out:
gmain_data_cleanup (&gmain_data);
return ret;
}
--
2.34.3

View File

@ -0,0 +1,49 @@
From ec7116d0e4de535a90c1dc5edabe821f04a0f8e0 Mon Sep 17 00:00:00 2001
From: William Roberts <william.c.roberts@intel.com>
Date: Wed, 13 Jan 2021 12:21:47 -0600
Subject: [PATCH 2/6] resource-manager: rm ref count inc of handle_entry
Per:
- https://developer.gnome.org/gobject/stable/gobject-memory.html
g_object_new sets the ref count to 1, so their is no need to bump it
again, we already have ownership.
Fixes leaks like:
Direct leak of 10480 byte(s) in 2 object(s) allocated from:
#0 0x7f1aa88aabc8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
#1 0x7f1aa848acd8 in g_malloc (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57cd8)
#2 0x7f1aa84a32c5 in g_slice_alloc (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x702c5)
#3 0x7f1aa84a38ed in g_slice_alloc0 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x708ed)
#4 0x7f1aa85970cf in g_type_create_instance (/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x3b0cf)
#5 0x7f1aa857634c (/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x1a34c)
#6 0x7f1aa8578377 in g_object_new_valist (/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x1c377)
#7 0x7f1aa85786cc in g_object_new (/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0+0x1c6cc)
#8 0x561e13d667d3 in handle_map_entry_new src/handle-map-entry.c:138
#9 0x561e13d540d3 in create_context_mapping_transient src/resource-manager.c:1160
#10 0x561e13d547b1 in resource_manager_create_context_mapping src/resource-manager.c:1261
#11 0x561e13d54ec8 in resource_manager_process_tpm2_command src/resource-manager.c:1359
#12 0x561e13d55365 in resource_manager_thread src/resource-manager.c:1424
#13 0x7f1aa8384608 in start_thread /build/glibc-ZN95T4/glibc-2.31/nptl/pthread_create.c:477
#14 0x7f1aa82ab292 in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x122292)
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
src/resource-manager.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/resource-manager.c b/src/resource-manager.c
index 904f683..050436f 100644
--- a/src/resource-manager.c
+++ b/src/resource-manager.c
@@ -1167,7 +1167,6 @@ create_context_mapping_transient (ResourceManager *resmgr,
handle_map_insert (handle_map, vhandle, handle_entry);
g_object_unref (handle_map);
tpm2_response_set_handle (response, vhandle);
- g_object_ref (handle_entry);
}
/*
* This function after a Tpm2Command is sent to the TPM and:
--
2.34.3

View File

@ -0,0 +1,51 @@
From 62ae28635ada2a74b526244e8ea69cef74c6c022 Mon Sep 17 00:00:00 2001
From: William Roberts <william.c.roberts@intel.com>
Date: Wed, 13 Jan 2021 13:52:06 -0600
Subject: [PATCH 3/6] tabrmd-init.c: fix leaks on main to thread tpm2 instance
Theirs a case where the Tpm2 object coming in from main to the thread
fails setup and the cleanup function doesn't unref it. Move it to the
main cleanup routine and use g_clear_object to be *clear* on whom owns
the reference.
Fixes leaks like:
Indirect leak of 4176 byte(s) in 1 object(s) allocated from:
#0 0x7f652e71cdc6 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10ddc6)
#1 0x7f652e25ad30 in g_malloc0 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57d30)
#2 0x555ebb1a1c5f in sapi_context_init src/tpm2.c:162
#3 0x555ebb1a2fa8 in tpm2_new src/tpm2.c:438
#4 0x555ebb19d665 in init_thread_func src/tabrmd-init.c:178
#5 0x555ebb19bede in init_thread_func_tpm2_init_fail test/tabrmd-init_unit.c:199
#6 0x7f652e6074e0 (/usr/lib/x86_64-linux-gnu/libcmocka.so.0+0x54e0)
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
src/tabrmd-init.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/tabrmd-init.c b/src/tabrmd-init.c
index 58e0103..866c852 100644
--- a/src/tabrmd-init.c
+++ b/src/tabrmd-init.c
@@ -99,6 +99,9 @@ gmain_data_cleanup (gmain_data_t *data)
if (data->loop != NULL) {
main_loop_quit (data->loop);
}
+ if (data->tpm2) {
+ g_clear_object (&data->tpm2);
+ }
tabrmd_options_free(&data->options);
}
@@ -208,7 +211,7 @@ init_thread_func (gpointer user_data)
g_clear_object (&session_list);
data->response_sink = response_sink_new ();
g_object_unref (command_attrs);
- g_object_unref (data->tpm2);
+ g_clear_object (&data->tpm2);
/*
* Wire up the TPM command processing pipeline. TPM command buffers
* flow from the CommandSource, to the Tab then finally back to the
--
2.34.3

View File

@ -0,0 +1,28 @@
From 545287019c1b9689c92900330be058b5ab9cf5d6 Mon Sep 17 00:00:00 2001
From: William Roberts <william.c.roberts@intel.com>
Date: Wed, 13 Jan 2021 15:11:42 -0600
Subject: [PATCH 4/6] init_thread_func: fix deadlock
The caller locks the mutex and never releases on the error path, only
the success path.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
src/tabrmd-init.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/tabrmd-init.c b/src/tabrmd-init.c
index 866c852..ea71155 100644
--- a/src/tabrmd-init.c
+++ b/src/tabrmd-init.c
@@ -249,6 +249,7 @@ init_thread_func (gpointer user_data)
return GINT_TO_POINTER (0);
err_out:
+ g_mutex_unlock (&data->init_mutex);
g_debug ("%s: calling gmain_data_cleanup", __func__);
gmain_data_cleanup (data);
return GINT_TO_POINTER (ret);
--
2.34.3

View File

@ -0,0 +1,30 @@
From a97e07d5a5947f5749e4ea25d0f538eeee8997bb Mon Sep 17 00:00:00 2001
From: Jerry Snitselaar <jsnitsel@redhat.com>
Date: Mon, 23 Nov 2020 11:45:31 -0700
Subject: [PATCH 5/6] ResourceManager: Avoid double free in resource-manager.c
Clean up potential double free found by coverity in
resource_manager_load_session_from_handle. If flush_session has been
called, don't call session_list_remove which is already called in
flush_session.
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
src/resource-manager.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/resource-manager.c b/src/resource-manager.c
index 050436f..556184b 100644
--- a/src/resource-manager.c
+++ b/src/resource-manager.c
@@ -239,6 +239,7 @@ resource_manager_load_session_from_handle (ResourceManager *resmgr,
rc = tpm2_response_get_code (response);
if (rc != TSS2_RC_SUCCESS) {
flush_session (resmgr, session_entry);
+ goto out;
}
}
if (will_flush) {
--
2.34.3

View File

@ -0,0 +1,40 @@
From a645f8c656b47568072351f4bfa58960016fbbac Mon Sep 17 00:00:00 2001
From: Nicolas Iooss <nicolas.iooss@ledger.fr>
Date: Mon, 27 Sep 2021 16:46:42 +0200
Subject: [PATCH 6/6] tcti: initialize GError to NULL
When an error happens in `tcti_tabrmd_read`, Glib reports:
(process:905338): GLib-WARNING **: 06:59:08.971: GError set over the
top of a previous GError or uninitialized memory.
This indicates a bug in someone's code. You must ensure an error is
NULL before it's set.
The overwriting error message was: Error receiving data: Connection
reset by peer
This warning was reported on
https://github.com/tpm2-software/tpm2-pkcs11/issues/705
Fix the warning by initializing `error` correctly.
Signed-off-by: Nicolas Iooss <nicolas.iooss@ledger.fr>
---
src/tcti-tabrmd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tcti-tabrmd.c b/src/tcti-tabrmd.c
index d96709e..d0ab74d 100644
--- a/src/tcti-tabrmd.c
+++ b/src/tcti-tabrmd.c
@@ -187,7 +187,7 @@ tcti_tabrmd_read (TSS2_TCTI_TABRMD_CONTEXT *ctx,
size_t size,
int32_t timeout)
{
- GError *error;
+ GError *error = NULL;
ssize_t num_read;
int ret;
--
2.34.3

View File

@ -2,7 +2,7 @@
Name: tpm2-abrmd
Version: 2.3.3
Release: 2%{?dist}
Release: 3%{?dist}
Summary: A system daemon implementing TPM2 Access Broker and Resource Manager
License: BSD
@ -21,6 +21,13 @@ BuildRequires: pkgconfig(tss2-sys)
# tpm2-abrmd depends on tpm2-tss-devel for tss2-mu/sys libs
BuildRequires: tpm2-tss-devel >= 2.3.1-2%{?dist}
Patch0: 0001-tabrmd-options-fix-memory-leak.patch
Patch1: 0002-resource-manager-rm-ref-count-inc-of-handle_entry.patch
Patch2: 0003-tabrmd-init.c-fix-leaks-on-main-to-thread-tpm2-insta.patch
Patch3: 0004-init_thread_func-fix-deadlock.patch
Patch4: 0005-ResourceManager-Avoid-double-free-in-resource-manage.patch
Patch5: 0006-tcti-initialize-GError-to-NULL.patch
# tpm2-abrmd depends on the package that contains its SELinux policy module
Requires: (%{name}-selinux >= 2.0.0-1%{?dist} if selinux-policy-%{selinuxtype})
@ -90,6 +97,10 @@ required to build applications that use tpm2-abrmd.
%systemd_postun tpm2-abrmd.service
%changelog
* Thu Aug 11 2022 Štěpán Horáček <shoracek@redhat.com> - 2.3.3-3
- Fix memory leaks and double free
resolves: rhbz#2041912
* Mon Nov 23 2020 Jerry Snitselaar <jsnitsel@redhat.com> - 2.3.3-2
- Update tpm2-tss-devel BuildRequires
resolves: rhbz#1855177