On 9/9/19 10:26 AM, Jim Fehlig wrote:
> AppArmorGetSecurityProcessLabel copies the VM's profile name to the
> label member of virSecurityLabel struct. If the profile is not loaded,
> the name is set empty before calling virStrcpy to copy it. However,
> virStrcpy will fail if src is empty (0 length), causing
> AppArmorGetSecurityProcessLabel to needlessly fail. Simple operations
> that report security driver information will subsequently fail
>
> virsh dominfo test
> Id: 248
> Name: test
> ...
> Security model: apparmor
> Security DOI: 0
> error: internal error: error copying profile name
>
> Avoid copying an empty profile name when the profile is not loaded.
Any comments on this patch? There are a lot of ways to skin the cat, but I think
we can agree that copying an empty profile name should be avoided.
Regards,
Jim
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
> src/security/security_apparmor.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
> index 6d16b15c65..77eee9410c 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -525,14 +525,13 @@ AppArmorGetSecurityProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
> "%s", _("error getting profile status"));
> goto cleanup;
> } else if (status == -1) {
> - profile_name[0] = '\0';
> - }
> -
> - if (virStrcpy(sec->label, profile_name,
> - VIR_SECURITY_LABEL_BUFLEN) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("error copying profile name"));
> - goto cleanup;
> + sec->label[0] = '\0';
> + } else {
> + if (virStrcpy(sec->label, profile_name, VIR_SECURITY_LABEL_BUFLEN) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("error copying profile name"));
> + goto cleanup;
> + }
> }
>
> sec->enforcing = status == 1;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
