RepoMirrors/selinux
1
0
Fork 0
mirror of https://github.com/SELinuxProject/selinux synced 2024-12-25 23:42:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Implement destructors for thread-local heap data.

Browse Source 
Description of problem:
Use of __thread variables is great for creating a thread-safe variable, but
only insofar as the contents of that variable can safely be abandoned on
pthread_exit().  The moment you store malloc()d data into a __thread void*
variable, you have leaked memory when the thread exits, since there is no way
to associate a destructor with __thread variables.

The _only_ safe way to use thread-local caching of malloc()d data is to use
pthread_key_create, and associate a destructor that will call free() on the
resulting data when the thread exits.

libselinux is guilty of abusing __thread variables to store malloc()d data as a
form of a cache, to minimize computation by reusing earlier results from the
same thread.  As a result of this memory leak, repeated starting and stopping
of domains via libvirt can result in the OOM killer triggering, since libvirt
fires up a thread per domain, and each thread uses selinux calls such as
fgetfilecon.

Version-Release number of selected component (if applicable):
libselinux-2.0.94-2.el6.x86_64
libvirt-0.8.1-27.el6.x86_64

How reproducible:
100%

Steps to Reproduce:
0. These steps are run as root, assuming hardware kvm support and existence of
a VM named fedora (adjust the steps below as appropriate); if desired, I can
reduce this to a simpler test case that does not rely on libvirt, by using a
single .c file that links against libselinux and repeatedly spawns threads.
1. service libvirtd stop
2. valgrind --quiet --leak-check=full /usr/sbin/libvirtd& pid=$!
3. virsh start fedora
4. kill $pid

Actual results:
The biggest leak reported is due to libselinux' abuse of __thread:

==26696== 829,730 (40 direct, 829,690 indirect) bytes in 1 blocks are
definitely lost in loss record 500 of 500
==26696==    at 0x4A0515D: malloc (vg_replace_malloc.c:195)
==26696==    by 0x3022E0D48C: selabel_open (label.c:165)
==26696==    by 0x3022E11646: matchpathcon_init_prefix (matchpathcon.c:296)
==26696==    by 0x3022E1190D: matchpathcon (matchpathcon.c:317)
==26696==    by 0x3033ED7FB5: SELinuxRestoreSecurityFileLabel (security_selinux.c:381)
==26696==    by 0x3033ED8539: SELinuxRestoreSecurityAllLabel (security_selinux.c:749)
==26696==    by 0x459153: qemuSecurityStackedRestoreSecurityAllLabel (qemu_security_stacked.c:257)
==26696==    by 0x43F0C5: qemudShutdownVMDaemon (qemu_driver.c:4311)
==26696==    by 0x4555C9: qemudStartVMDaemon (qemu_driver.c:4234)
==26696==    by 0x458416: qemudDomainObjStart (qemu_driver.c:7268)
==26696==    by 0x45896F: qemudDomainStart (qemu_driver.c:7308)
==26696==    by 0x3033E75412: virDomainCreate (libvirt.c:4881)
==26696==

Basically, libvirt created a thread that used matchpathcon during 'virsh start
fedora', and matchpathcon stuffed over 800k of malloc'd data into:

static __thread char **con_array;

which are then inaccessible when libvirt exits the thread as part of shutting
down on SIGTERM.

Expected results:
valgrind should not report any memory leaks related to libselinux.

Signed-off-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
Reported-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Eamon Walsh 2010-12-02 14:08:59 -05:00
parent 7bb6003219
commit a29ff33baf
3 changed files with 52 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
libselinux/src/matchpathcon.c
View File

@ -15,6 +15,9 @@ static __thread char **con_array;
static __thread int con_array_size;
static __thread int con_array_used;
static pthread_once_t once = PTHREAD_ONCE_INIT;
static pthread_key_t destructor_key;
static int add_array_elt(char *con)
{
if (con_array_size) {
@ -42,7 +45,7 @@ static int add_array_elt(char *con)
return con_array_used++;
}
static void free_array_elts(void)
static void free_array_elts(void __attribute__((unused)) *unused)
{
con_array_size = con_array_used = 0;
free(con_array);
@ -263,7 +266,7 @@ void matchpathcon_filespec_destroy(void)
file_spec_t *fl, *tmp;
int h;
free_array_elts();
free_array_elts(NULL);
if (!fl_head)
return;
@ -282,11 +285,19 @@ void matchpathcon_filespec_destroy(void)
fl_head = NULL;
}
static void matchpathcon_init_once(void)
{
__selinux_key_create(&destructor_key, free_array_elts);
}
int matchpathcon_init_prefix(const char *path, const char *subset)
{
if (!mycanoncon)
mycanoncon = default_canoncon;
__selinux_once(once, matchpathcon_init_once);
__selinux_setspecific(destructor_key, (void *)1);
options[SELABEL_OPT_SUBSET].type = SELABEL_OPT_SUBSET;
options[SELABEL_OPT_SUBSET].value = subset;
options[SELABEL_OPT_PATH].type = SELABEL_OPT_PATH;

13
libselinux/src/selinux_internal.h
View File

@ -97,6 +97,8 @@ extern int selinux_page_size hidden;
/* Make pthread_once optional */
#pragma weak pthread_once
#pragma weak pthread_key_create
#pragma weak pthread_setspecific
/* Call handler iff the first call. */
#define __selinux_once(ONCE_CONTROL, INIT_FUNCTION) \
@ -109,4 +111,15 @@ extern int selinux_page_size hidden;
} \
} while (0)
/* Pthread key macros */
#define __selinux_key_create(KEY, DESTRUCTOR) \
do { \
if (pthread_key_create != NULL) \
pthread_key_create(KEY, DESTRUCTOR); \
} while (0)
#define __selinux_setspecific(KEY, VALUE) \
do { \
if (pthread_setspecific != NULL) \
pthread_setspecific(KEY, VALUE); \
} while (0)

26
libselinux/src/setrans_client.c
View File

@ -34,6 +34,8 @@ static __thread char *prev_r2c_trans = NULL;
static __thread security_context_t prev_r2c_raw = NULL;
static pthread_once_t once = PTHREAD_ONCE_INIT;
static pthread_key_t destructor_key;
static __thread char destructor_initialized;
/*
* setransd_open
@ -240,8 +242,27 @@ out:
return ret;
}
static void setrans_thread_destructor(void __attribute__((unused)) *unused)
{
free(prev_t2r_trans);
free(prev_t2r_raw);
free(prev_r2t_trans);
free(prev_r2t_raw);
free(prev_r2c_trans);
free(prev_r2c_raw);
}
static inline void init_thread_destructor(void)
{
if (destructor_initialized == 0) {
__selinux_setspecific(destructor_key, (void *)1);
destructor_initialized = 1;
}
}
static void init_context_translations(void)
{
__selinux_key_create(&destructor_key, setrans_thread_destructor);
mls_enabled = is_selinux_mls_enabled();
}
@ -254,6 +275,7 @@ int selinux_trans_to_raw_context(const security_context_t trans,
}
__selinux_once(once, init_context_translations);
init_thread_destructor();
if (!mls_enabled) {
*rawp = strdup(trans);
@ -295,6 +317,7 @@ int selinux_raw_to_trans_context(const security_context_t raw,
}
__selinux_once(once, init_context_translations);
init_thread_destructor();
if (!mls_enabled) {
*transp = strdup(raw);
@ -334,6 +357,9 @@ int selinux_raw_context_to_color(const security_context_t raw, char **transp)
return -1;
}
__selinux_once(once, init_context_translations);
init_thread_destructor();
if (prev_r2c_raw && strcmp(prev_r2c_raw, raw) == 0) {
*transp = strdup(prev_r2c_trans);
} else {