[libvirt] [PATCHv2 0/2] fix segfault in virSecuritySELinuxMCSGetProcessRange
Diff to v2: Changed char * to const char * after actually compiling it with selinux. Added a test. James Gilliland (1): selinux: fix segfault in virSecuritySELinuxMCSGetProcessRange Ján Tomko (1): tests: add test for a selinux label without a range src/security/security_selinux.c | 9 ++++++++- tests/securityselinuxtest.c | 44 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 49 insertions(+), 4 deletions(-) -- 1.8.1.5
From: James Gilliland <neclimdul@gmail.com> https://bugzilla.redhat.com/show_bug.cgi?id=969878 --- src/security/security_selinux.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b862fbf..3c67f24 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -189,6 +189,7 @@ virSecuritySELinuxMCSGetProcessRange(char **sens, context_t ourContext = NULL; char *cat = NULL; char *tmp; + const char *contextRange; int ret = -1; if (getcon_raw(&ourSecContext) < 0) { @@ -202,8 +203,14 @@ virSecuritySELinuxMCSGetProcessRange(char **sens, ourSecContext); goto cleanup; } + if (!(contextRange = context_range_get(ourContext))) { + virReportSystemError(errno, + _("Unable to parse current SELinux context range '%s'"), + ourSecContext); + goto cleanup; + } - if (VIR_STRDUP(*sens, context_range_get(ourContext)) < 0) + if (VIR_STRDUP(*sens, contextRange) < 0) goto cleanup; /* Find and blank out the category part (if any) */ -- 1.8.1.5
On Tue, Jun 04, 2013 at 01:23:59PM +0200, Ján Tomko wrote:
From: James Gilliland <neclimdul@gmail.com>
https://bugzilla.redhat.com/show_bug.cgi?id=969878 --- src/security/security_selinux.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b862fbf..3c67f24 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -189,6 +189,7 @@ virSecuritySELinuxMCSGetProcessRange(char **sens, context_t ourContext = NULL; char *cat = NULL; char *tmp; + const char *contextRange; int ret = -1;
if (getcon_raw(&ourSecContext) < 0) { @@ -202,8 +203,14 @@ virSecuritySELinuxMCSGetProcessRange(char **sens, ourSecContext); goto cleanup; } + if (!(contextRange = context_range_get(ourContext))) { + virReportSystemError(errno, + _("Unable to parse current SELinux context range '%s'"), + ourSecContext); + goto cleanup; + }
Re-thinking this again. Raising an error here will technically be a regression in functionality vs older libvirt. I think we need to automatically fill in "s0" for *sens if contextRange is NULL, instead of raising an error. Also add to the comment before this function that 'system_u:system_r:virtd_t' is a valid context too.
- if (VIR_STRDUP(*sens, context_range_get(ourContext)) < 0) + if (VIR_STRDUP(*sens, contextRange) < 0) goto cleanup;
/* Find and blank out the category part (if any) */
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- tests/securityselinuxtest.c | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index bdf248b..a53b4ee 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -58,6 +58,8 @@ struct testSELinuxGenLabelData { int sensMax; int catMin; int catMax; + + bool shouldFail; }; static virDomainDefPtr @@ -230,10 +232,18 @@ testSELinuxGenLabel(const void *opaque) goto cleanup; if (virSecurityManagerGenLabel(data->mgr, def) < 0) { + if (data->shouldFail) { + ret = 0; + goto cleanup; + } virErrorPtr err = virGetLastError(); fprintf(stderr, "Cannot generate label: %s\n", err->message); goto cleanup; } + if (data->shouldFail) { + fprintf(stderr, "Label generation was expected to fail but didn't"); + goto cleanup; + } VIR_DEBUG("label=%s imagelabel=%s", def->seclabels[0]->label, def->seclabels[0]->imagelabel); @@ -282,21 +292,43 @@ mymain(void) return EXIT_FAILURE; } -#define DO_TEST_GEN_LABEL(desc, pidcon, \ +#define DO_TEST_GEN_LABEL_FULL(desc, pidcon, \ dynamic, label, baselabel, \ user, role, imageRole, \ type, imageType, \ - sensMin, sensMax, catMin, catMax) \ + sensMin, sensMax, catMin, catMax, shouldFail) \ do { \ struct testSELinuxGenLabelData data = { \ mgr, pidcon, dynamic, label, baselabel, \ user, role, imageRole, type, imageType, \ - sensMin, sensMax, catMin, catMax \ + sensMin, sensMax, catMin, catMax, shouldFail \ }; \ if (virtTestRun("GenLabel " # desc, 1, testSELinuxGenLabel, &data) < 0) \ ret = -1; \ } while (0) +#define DO_TEST_GEN_LABEL(desc, pidcon, \ + dynamic, label, baselabel, \ + user, role, imageRole, \ + type, imageType, \ + sensMin, sensMax, catMin, catMax) \ + DO_TEST_GEN_LABEL_FULL(desc, pidcon, \ + dynamic, label, baselabel, \ + user, role, imageRole, \ + type, imageType, \ + sensMin, sensMax, catMin, catMax, false) + +#define DO_TEST_GEN_LABEL_FAIL(desc, pidcon, \ + dynamic, label, baselabel, \ + user, role, imageRole, \ + type, imageType, \ + sensMin, sensMax, catMin, catMax) \ + DO_TEST_GEN_LABEL_FULL(desc, pidcon, \ + dynamic, label, baselabel, \ + user, role, imageRole, \ + type, imageType, \ + sensMin, sensMax, catMin, catMax, true) + DO_TEST_GEN_LABEL("dynamic unconfined, s0, c0.c1023", "unconfined_u:unconfined_r:unconfined_t:s0", true, NULL, NULL, @@ -333,6 +365,12 @@ mymain(void) "system_u", "system_r", "object_r", "svirt_t", "svirt_image_t", 2, 3, 0, 1023); + DO_TEST_GEN_LABEL_FAIL("dynamic virtd, missing range", + "system_u:system_r:virtd_t", + true, NULL, NULL, + "system_u", "system_r", "object_r", + "svirt_t", "svirt_image_t", + 0, 0, 0, 0); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.1.5
participants (2)
-
Daniel P. Berrange -
Ján Tomko