[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Fix crash in SELinuxSecurityVerify



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.
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.

Daniel


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]