Hi folks,
There have been numerous reports in Debian and derivatives of
programs linked with libselinux intermittently getting segfaults.
There is, for instance, the Debian report 505920[0], and Ubuntu
reports[1], [3] and [5], and Gnome [2]. I have not been able to
reproduce the error myself, though I have run the test cases a number
of times.
The common thread in unclutter, libavg, gst-inspect et al. is a
segmentation fault in libselinux1, in the 'fini' destructor functions,
referencing the thread local variables.
The Ubuntu bug log reference my old patch for libselinux from
1.X days, where I replaced the thread local storage with regular
variables and mutexes, and people report success with that. I suspect
that something is corrupting the thread local storage. From the ubuntu
report:
--8<---------------cut here---------------start------------->8---
Valgrind reports:
=29183== Invalid read of size 8
==29183== at 0xE29B9DD: fini_context_translations (setrans_client.c:211)
==29183== by 0xE28F1F1: (within /lib/libselinux.so.1)
==29183== by 0xE29D040: (within /lib/libselinux.so.1)
==29183== by 0x570010F: exit (exit.c:75)
505920==29183== by 0x56E91CA: (below main) (libc-start.c:252)
==29183== Address 0x80 is not stack'd, malloc'd or (recently) free'd
==29183==
==29183== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==29183== Access not within mapped region at address 0x80
==29183== at 0xE29B9DD: fini_context_translations (setrans_client.c:211)
==29183== by 0xE28F1F1: (within /lib/libselinux.so.1)
==29183== by 0xE29D040: (within /lib/libselinux.so.1)==29183== by 0x570010F: exit (exit.c:75)
==29183== by 0x56E91CA: (below main) (libc-start.c:252)
(gdb) bt
#0 0x00007f3ae812a9dd in fini_context_translations () at setrans_client.c:211
#1 0x00007f3ae811e1f2 in __do_global_dtors_aux () from /lib/libselinux.so.1
#2 0x00007ffff9097700 in ?? ()
#3 0x00007f3ae812c041 in _fini () from /lib/libselinux.so.1
#4 0x00007ffff9097700 in ?? ()
#5 0x00007f3af0e88796 in _dl_fini () from /lib64/ld-linux-x86-64.so.2
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
--8<---------------cut here---------------end--------------->8---
There have been two sets of patches proposed for this; first one
merely initializes the variables in the init function, and this works
for a number of people, but at least one person has reported a second
segfault even with the patch installed[6]
The second patch below converts a thread local cache to a
process wide cache, with mutex guards, which makes the cache slower,
and non-thread local caches means that cache misses are more likely.
I'll try and follow up with people who can reproduce the
problems to see if either one of the patches solve their problems
without triggering other segmentation faults, but I'd appreciate
comments if anyone has insight into the issue.
manoj
[0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=505920
[1] http://bugs.launchpad.net/ubuntu/+source/libselinux/+bug/237156
[2] http://mail.gnome.org/archives/gnome-shell-list/2008-November/msg00001.html
[3] https://bugs.launchpad.net/ubuntu/+source/nspluginwrapper/+bug/357965
[4] https://bugs.launchpad.net/ubuntu/+source/libselinux/+bug/237156/comments/6
[5] https://bugs.launchpad.net/ubuntu/+source/libselinux/+bug/99949
>From fa50efa65aad2efa0f59af10027140c571efab00 Mon Sep 17 00:00:00 2001
From: Manoj Srivastava <srivasta@xxxxxxxxxx>
Date: Wed, 6 May 2009 01:22:19 -0500
Subject: [PATCH] [init-static-variables] Initialize thread variable
The crash occurs when the libselinux1 library is trying to do its 'fini'
destructor functions when unloading. It references some thread local
storage variables that were statically initialized to NULL. That first
reference fails when it actually tries to allocate the variables. This
patch explicity initializes those variables during the init function,
making the fini function work. It seems that the real defect is in a
corner case of the implementation of "__thread" feature.
Signed-Off-By: Mike Stroyan
Acked-By: Manoj Srivastava <srivasta@xxxxxxxxxx>
---
src/setrans_client.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/setrans_client.c b/src/setrans_client.c
index 500225e..d9af05c 100644
--- a/src/setrans_client.c
+++ b/src/setrans_client.c
@@ -251,6 +251,14 @@ hidden void fini_context_translations(void)
hidden int init_context_translations(void)
{
mls_enabled = is_selinux_mls_enabled();
+ // set initial values to work around crash from very late first reference in fini.
+ // The "static __thread .. = NULL" initialization isn't working.
+ prev_r2t_trans = NULL;
+ prev_r2t_raw = NULL;
+ prev_t2r_trans = NULL;
+ prev_t2r_raw = NULL;
+ prev_r2c_trans = NULL;
+ prev_r2c_raw = NULL;
return 0;
}
--
1.6.2.4
>From 50da3dc55758c0977c6bf445a8eb48617ad2eda7 Mon Sep 17 00:00:00 2001
From: Manoj Srivastava <srivasta@xxxxxxxxxx>
Date: Wed, 6 May 2009 00:59:01 -0500
Subject: [PATCH] [tls-fix] Convert cache to static cache, with mutex guards
There have been numerous reports in Debian and derivatives of
programs linked with libselinux intermittently getting segfaults.
There is, for instance, the Debian report 505920, and Ubuntu
reports 237156 and 357965, and also from Gnome. The problem seems to
be in the fini function in src/setrans_client.c, and the bug reports
show attempts to free addresses not " not stack'd, malloc'd or
(recently) free'd".
This commit converts a thread local cache to a process wide cache,
with mutex guards, which makes the cache slower, and non-thread local
caches means that cache misses are more likely.
Signed-off-by: Manoj Srivastava <srivasta@xxxxxxxxxx>
---
src/setrans_client.c | 24 ++++++++++++++++++------
1 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/setrans_client.c b/src/setrans_client.c
index 500225e..01ba955 100644
--- a/src/setrans_client.c
+++ b/src/setrans_client.c
@@ -18,6 +18,7 @@
#include <string.h>
#include <ctype.h>
#include <unistd.h>
+#include <pthread.h>
#include "dso.h"
#include "selinux_internal.h"
#include "setrans_internal.h"
@@ -26,12 +27,17 @@
static int mls_enabled = -1;
// Simple cache
-static __thread security_context_t prev_t2r_trans = NULL;
-static __thread security_context_t prev_t2r_raw = NULL;
-static __thread security_context_t prev_r2t_trans = NULL;
-static __thread security_context_t prev_r2t_raw = NULL;
-static __thread char *prev_r2c_trans = NULL;
-static __thread security_context_t prev_r2c_raw = NULL;
+static security_context_t prev_t2r_trans = NULL;
+static security_context_t prev_t2r_raw = NULL;
+static security_context_t prev_r2t_trans = NULL;
+static security_context_t prev_r2t_raw = NULL;
+static char *prev_r2c_trans = NULL;
+static security_context_t prev_r2c_raw = NULL;
+
+/* Mutexes used to guard the cache */
+static pthread_mutex_t trans_to_raw_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t raw_to_trans_mutex = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t raw_to_color_mutex = PTHREAD_MUTEX_INITIALIZER;
/*
* setransd_open
@@ -267,6 +273,7 @@ int selinux_trans_to_raw_context(security_context_t trans,
goto out;
}
+ pthread_mutex_lock(&trans_to_raw_mutex);
if (prev_t2r_trans && strcmp(prev_t2r_trans, trans) == 0) {
*rawp = strdup(prev_t2r_raw);
} else {
@@ -288,6 +295,7 @@ int selinux_trans_to_raw_context(security_context_t trans,
}
}
out:
+ pthread_mutex_unlock(&trans_to_raw_mutex);
return *rawp ? 0 : -1;
}
@@ -306,6 +314,7 @@ int selinux_raw_to_trans_context(security_context_t raw,
goto out;
}
+ pthread_mutex_lock(&raw_to_trans_mutex);
if (prev_r2t_raw && strcmp(prev_r2t_raw, raw) == 0) {
*transp = strdup(prev_r2t_trans);
} else {
@@ -327,6 +336,7 @@ int selinux_raw_to_trans_context(security_context_t raw,
}
}
out:
+ pthread_mutex_unlock(&raw_to_trans_mutex);
return *transp ? 0 : -1;
}
@@ -339,6 +349,7 @@ int selinux_raw_context_to_color(security_context_t raw, char **transp)
return -1;
}
+ pthread_mutex_lock(&raw_to_color_mutex);
if (prev_r2c_raw && strcmp(prev_r2c_raw, raw) == 0) {
*transp = strdup(prev_r2c_trans);
} else {
@@ -360,6 +371,7 @@ int selinux_raw_context_to_color(security_context_t raw, char **transp)
}
}
out:
+ pthread_mutex_unlock(&raw_to_color_mutex);
return *transp ? 0 : -1;
}
--
1.6.2.4
--
Manoj Srivastava <manoj.srivastava@xxxxxxxx> <srivasta@xxxxxxx>
1024D/BF24424C print 4966 F272 D093 B493 410B 924B 21BA DABB BF24 424C
