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 ebl...@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
