On 11/24/2010 02:57 PM, Eric Blake wrote:
security_context_t happens to be a typedef for char*, and happens to
begin with a string usable as a raw context string. But in reality,
it is an opaque type that may or may not have additional information
after the first NUL byte, where that additional information can
include pointers that can only be freed via freecon().
Proof is from this valgrind run of daemon/libvirtd:
==6028== 839,169 (40 direct, 839,129 indirect) bytes in 1 blocks are definitely lost in
loss record 274 of 274
==6028== at 0x4A0515D: malloc (vg_replace_malloc.c:195)
==6028== by 0x3022E0D48C: selabel_open (label.c:165)
==6028== by 0x3022E11646: matchpathcon_init_prefix (matchpathcon.c:296)
==6028== by 0x3022E1190D: matchpathcon (matchpathcon.c:317)
==6028== by 0x4F9D842: SELinuxRestoreSecurityFileLabel (security_selinux.c:382)
800k is a lot of memory to be leaking.
* src/security/security_selinux.c
(SELinuxReserveSecurityLabel, SELinuxGetSecurityProcessLabel)
(SELinuxRestoreSecurityFileLabel): Use correct function to free
security_context_t.
---
src/security/security_selinux.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
Actually, it turns out that even with this patch, libvirtd is still
suffering from the same severe 800k leak. libselinux has a bug where it
uses __thread variables to store malloc'd memory, which is a no-no
(since __thread has no destructor, you can't ever release the memory on
pthread_exit(), in contrast to the heavier-weight
pthread_key_create/pthread_getspecific interface).
https://bugzilla.redhat.com/show_bug.cgi?id=658571
I'm wondering if adding a matchpathcon_fini() call would help reduce the
leak [to pair up with the implicit matchpathcon_init(NULL) during our
first use of matchpathcon()], although it won't eliminate it.
Also, I peeked at the libselinux source; for now, security_context_t is
just a NUL-terminated string grabbed from fgetxattr(XATTR_NAME_SELINUX),
and freecon() is a thin wrapper around free(); but there's no guarantee
that this won't change in a future version of the libselinux library.
Therefore, using VIR_FREE() interchangeably with freecon() is not a bug
today, but is not future-proof, so I'm still happy this patch went in.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org