On 01/13/2011 08:46 AM, Daniel P. Berrange wrote:

On Thu, Jan 13, 2011 at 12:30:30AM -0500, Laine Stump wrote:

...

When attempting to edit a domain, libvirtd segfaulted in SELinuxSecurityVerify() on this line:

if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) {

because secdef->model was NULL. Although I'm too tired to investigate in depth, I noticed that all the other functions in that file that do the same STREQ() will first check that def->seclabel.label is non-NULL, but this function doesn't. I also noticed that label *is* NULL in my case, so I tried adding that check to SELinuxSecurityVerify(), and the crash goes away.

I have no idea if this is the correct fix, but it allowed me to continue my testing of a new (unrelated) feature. --- src/security/security_selinux.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index d06afde..b97ca4c 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -871,6 +871,10 @@ SELinuxSecurityVerify(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, virDomainDefPtr def) { const virSecurityLabelDefPtr secdef =&def->seclabel; + + if (def->seclabel.label == NULL) + return 0; + if (!STREQ(virSecurityManagerGetModel(mgr), secdef->model)) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("security label driver mismatch: " We don't want to skip a NULL label, but rather a NULL model.

Yeah, it didn't really make sense at the time, but all the other functions were doing it, it stopped the crash, and I was too tired to spend brain cells understanding the code :-)

So I think you actually need to add a check

if (def->seclabel.model == NULL) return 0;

but in the Verify method in src/security/security_manager.c so that all drivers are protected instead of just SELinux.

Okay. Is that needed for the other methods that end up comparing secdef->model, too? Or are you guaranteed a non-null model by the time you get into any of those? (eg virSecurityManagerSetProcessLabel(), virSecurityManagerSetSecuritySocketLabel(),etc)