Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
---
src/security/security_apparmor.c | 91 +++++++++++---------------------
1 file changed, 31 insertions(+), 60 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 36e8ce42b5..64b57f6a22 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -70,9 +70,9 @@ struct SDPDOP {
static int
profile_status(const char *str, const int check_enforcing)
{
- char *content = NULL;
- char *tmp = NULL;
- char *etmp = NULL;
+ g_autofree char *content = NULL;
+ g_autofree char *tmp = NULL;
+ g_autofree char *etmp = NULL;
int rc = -2;
/* create string that is '<str> \0' for accurate matching */
@@ -87,7 +87,7 @@ profile_status(const char *str, const int check_enforcing)
virReportSystemError(errno,
_("Failed to read AppArmor profiles list "
"\'%s\'"), APPARMOR_PROFILES_PATH);
- goto cleanup;
+ return -2;
}
if (strstr(content, tmp) != NULL)
@@ -99,11 +99,6 @@ profile_status(const char *str, const int check_enforcing)
rc = 1; /* return '1' if loaded and enforcing */
}
- VIR_FREE(content);
- cleanup:
- VIR_FREE(tmp);
- VIR_FREE(etmp);
-
return rc;
}
@@ -320,12 +315,11 @@ AppArmorSetSecurityHostLabel(virSCSIVHostDevice *dev G_GNUC_UNUSED,
static int
AppArmorSecurityManagerProbe(const char *virtDriver G_GNUC_UNUSED)
{
- char *template_qemu = NULL;
- char *template_lxc = NULL;
- int rc = SECURITY_DRIVER_DISABLE;
+ g_autofree char *template_qemu = NULL;
+ g_autofree char *template_lxc = NULL;
if (use_apparmor() < 0)
- return rc;
+ return SECURITY_DRIVER_DISABLE;
/* see if template file exists */
template_qemu = g_strdup_printf("%s/TEMPLATE.qemu", APPARMOR_DIR "/libvirt");
@@ -334,20 +328,15 @@ AppArmorSecurityManagerProbe(const char *virtDriver G_GNUC_UNUSED)
if (!virFileExists(template_qemu)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("template \'%s\' does not exist"), template_qemu);
- goto cleanup;
+ return SECURITY_DRIVER_DISABLE;
}
if (!virFileExists(template_lxc)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("template \'%s\' does not exist"), template_lxc);
- goto cleanup;
+ return SECURITY_DRIVER_DISABLE;
}
- rc = SECURITY_DRIVER_ENABLE;
- cleanup:
- VIR_FREE(template_qemu);
- VIR_FREE(template_lxc);
-
- return rc;
+ return SECURITY_DRIVER_ENABLE;
}
/* Security driver initialization. DOI is for 'Domain of Interpretation' and is
@@ -387,8 +376,7 @@ static int
AppArmorGenSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED,
virDomainDef *def)
{
- int rc = -1;
- char *profile_name = NULL;
+ g_autofree char *profile_name = NULL;
virSecurityLabelDef *secdef = virDomainDefGetSecurityLabelDef(def,
SECURITY_APPARMOR_NAME);
@@ -402,18 +390,18 @@ AppArmorGenSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED,
if (secdef->baselabel) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("Cannot set a base label with AppArmour"));
- return rc;
+ return -1;
}
if (secdef->label || secdef->imagelabel) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s",
_("security label already defined for VM"));
- return rc;
+ return -1;
}
if ((profile_name = get_profile_name(def)) == NULL)
- return rc;
+ return -1;
secdef->label = g_strdup(profile_name);
@@ -431,18 +419,13 @@ AppArmorGenSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED,
goto err;
}
- rc = 0;
- goto cleanup;
+ return 0;
err:
VIR_FREE(secdef->label);
VIR_FREE(secdef->imagelabel);
VIR_FREE(secdef->model);
-
- cleanup:
- VIR_FREE(profile_name);
-
- return rc;
+ return -1;
}
static int
@@ -476,33 +459,29 @@ AppArmorGetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
{
int rc = -1;
int status;
- char *profile_name = NULL;
+ g_autofree char *profile_name = NULL;
if ((profile_name = get_profile_name(def)) == NULL)
- return rc;
+ return -1;
status = profile_status(profile_name, 1);
if (status < -1) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("error getting profile status"));
- goto cleanup;
+ return -1;
} else if (status == -1) {
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;
+ return -1;
}
}
sec->enforcing = status == 1;
- rc = 0;
-
- cleanup:
- VIR_FREE(profile_name);
- return rc;
+ return 0;
}
/* Called on VM shutdown and destroy. See AppArmorGenSecurityLabel (above) for
@@ -555,7 +534,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
virDomainDef *def)
{
int rc = -1;
- char *profile_name = NULL;
+ g_autofree char *profile_name = NULL;
virSecurityLabelDef *secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
@@ -563,7 +542,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
return 0;
if ((profile_name = get_profile_name(def)) == NULL)
- return rc;
+ return -1;
if (STRNEQ(SECURITY_APPARMOR_NAME, secdef->model)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -572,21 +551,17 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
"hypervisor driver is \'%s\'."),
secdef->model, SECURITY_APPARMOR_NAME);
if (use_apparmor() > 0)
- goto cleanup;
+ return -1;
}
VIR_DEBUG("Changing AppArmor profile to %s", profile_name);
if (aa_change_profile(profile_name) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("error calling aa_change_profile()"));
- goto cleanup;
+ return -1;
}
- rc = 0;
-
- cleanup:
- VIR_FREE(profile_name);
- return rc;
+ return 0;
}
/* Called directly by API user prior to virCommandRun().
@@ -600,8 +575,8 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
virCommand *cmd)
{
int rc = -1;
- char *profile_name = NULL;
- char *cmd_str = NULL;
+ g_autofree char *profile_name = NULL;
+ g_autofree char *cmd_str = NULL;
virSecurityLabelDef *secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
@@ -615,21 +590,17 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
"hypervisor driver is \'%s\'."),
secdef->model, SECURITY_APPARMOR_NAME);
if (use_apparmor() > 0)
- goto cleanup;
+ return -1;
}
if ((profile_name = get_profile_name(def)) == NULL)
- goto cleanup;
+ return -1;
cmd_str = virCommandToString(cmd, false);
VIR_DEBUG("Changing AppArmor profile to %s on %s", profile_name, cmd_str);
virCommandSetAppArmorProfile(cmd, profile_name);
- rc = 0;
- cleanup:
- VIR_FREE(profile_name);
- VIR_FREE(cmd_str);
- return rc;
+ return 0;
}
static int
--
2.33.0On 1/5/23 6:26 AM, Jiang Jiacheng wrote:
...
> @@ -476,33 +459,29 @@ AppArmorGetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
> {
> int rc = -1;
'rc' variable doesn't seem to be used anymore.
> int status;
> - char *profile_name = NULL;
> + g_autofree char *profile_name = NULL;
>
> if ((profile_name = get_profile_name(def)) == NULL)
> - return rc;
> + return -1;
>
> status = profile_status(profile_name, 1);
> if (status < -1) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("error getting profile status"));
> - goto cleanup;
> + return -1;
> } else if (status == -1) {
> 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;
> + return -1;
> }
> }
>
> sec->enforcing = status == 1;
> - rc = 0;
> -
> - cleanup:
> - VIR_FREE(profile_name);
>
> - return rc;
> + return 0;
> }
>
> /* Called on VM shutdown and destroy. See AppArmorGenSecurityLabel (above) for
> @@ -555,7 +534,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
> virDomainDef *def)
> {
> int rc = -1;
same here. Remove?
> - char *profile_name = NULL;
> + g_autofree char *profile_name = NULL;
> virSecurityLabelDef *secdef =
> virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
>
> @@ -563,7 +542,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
> return 0;
>
> if ((profile_name = get_profile_name(def)) == NULL)
> - return rc;
> + return -1;
>
> if (STRNEQ(SECURITY_APPARMOR_NAME, secdef->model)) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -572,21 +551,17 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
> "hypervisor driver is \'%s\'."),
> secdef->model, SECURITY_APPARMOR_NAME);
> if (use_apparmor() > 0)
> - goto cleanup;
> + return -1;
> }
>
> VIR_DEBUG("Changing AppArmor profile to %s", profile_name);
> if (aa_change_profile(profile_name) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("error calling aa_change_profile()"));
> - goto cleanup;
> + return -1;
> }
> - rc = 0;
> -
> - cleanup:
> - VIR_FREE(profile_name);
>
> - return rc;
> + return 0;
> }
>
> /* Called directly by API user prior to virCommandRun().
> @@ -600,8 +575,8 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
> virCommand *cmd)
> {
> int rc = -1;
...and again
> - char *profile_name = NULL;
> - char *cmd_str = NULL;
> + g_autofree char *profile_name = NULL;
> + g_autofree char *cmd_str = NULL;
> virSecurityLabelDef *secdef =
> virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
>
> @@ -615,21 +590,17 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
> "hypervisor driver is \'%s\'."),
> secdef->model, SECURITY_APPARMOR_NAME);
> if (use_apparmor() > 0)
> - goto cleanup;
> + return -1;
> }
>
> if ((profile_name = get_profile_name(def)) == NULL)
> - goto cleanup;
> + return -1;
>
> cmd_str = virCommandToString(cmd, false);
> VIR_DEBUG("Changing AppArmor profile to %s on %s", profile_name, cmd_str);
> virCommandSetAppArmorProfile(cmd, profile_name);
> - rc = 0;
>
> - cleanup:
> - VIR_FREE(profile_name);
> - VIR_FREE(cmd_str);
> - return rc;
> + return 0;
> }
>
> static int
On 2023/1/6 1:45, Jonathon Jongsma wrote:
> On 1/5/23 6:26 AM, Jiang Jiacheng wrote:
>
> ...
>
>
>> @@ -476,33 +459,29 @@
>> AppArmorGetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>> {
>> int rc = -1;
>
> 'rc' variable doesn't seem to be used anymore.
The 'rc' and the following are not used, and it's strange that my
compiler does not generate warning about them.
I will remove them in next version.
>
>> int status;
>> - char *profile_name = NULL;
>> + g_autofree char *profile_name = NULL;
>> if ((profile_name = get_profile_name(def)) == NULL)
>> - return rc;
>> + return -1;
>> status = profile_status(profile_name, 1);
>> if (status < -1) {
>> virReportError(VIR_ERR_INTERNAL_ERROR,
>> "%s", _("error getting profile status"));
>> - goto cleanup;
>> + return -1;
>> } else if (status == -1) {
>> 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;
>> + return -1;
>> }
>> }
>> sec->enforcing = status == 1;
>> - rc = 0;
>> -
>> - cleanup:
>> - VIR_FREE(profile_name);
>> - return rc;
>> + return 0;
>> }
>> /* Called on VM shutdown and destroy. See AppArmorGenSecurityLabel
>> (above) for
>> @@ -555,7 +534,7 @@ AppArmorSetSecurityProcessLabel(virSecurityManager
>> *mgr G_GNUC_UNUSED,
>> virDomainDef *def)
>> {
>> int rc = -1;
>
> same here. Remove?
>
>> - char *profile_name = NULL;
>> + g_autofree char *profile_name = NULL;
>> virSecurityLabelDef *secdef =
>> virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
>> @@ -563,7 +542,7 @@
>> AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>> return 0;
>> if ((profile_name = get_profile_name(def)) == NULL)
>> - return rc;
>> + return -1;
>> if (STRNEQ(SECURITY_APPARMOR_NAME, secdef->model)) {
>> virReportError(VIR_ERR_INTERNAL_ERROR,
>> @@ -572,21 +551,17 @@
>> AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED,
>> "hypervisor driver is \'%s\'."),
>> secdef->model, SECURITY_APPARMOR_NAME);
>> if (use_apparmor() > 0)
>> - goto cleanup;
>> + return -1;
>> }
>> VIR_DEBUG("Changing AppArmor profile to %s", profile_name);
>> if (aa_change_profile(profile_name) < 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("error calling aa_change_profile()"));
>> - goto cleanup;
>> + return -1;
>> }
>> - rc = 0;
>> -
>> - cleanup:
>> - VIR_FREE(profile_name);
>> - return rc;
>> + return 0;
>> }
>> /* Called directly by API user prior to virCommandRun().
>> @@ -600,8 +575,8 @@
>> AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr
>> G_GNUC_UNUSED,
>> virCommand *cmd)
>> {
>> int rc = -1;
>
> ...and again
>
>> - char *profile_name = NULL;
>> - char *cmd_str = NULL;
>> + g_autofree char *profile_name = NULL;
>> + g_autofree char *cmd_str = NULL;
>> virSecurityLabelDef *secdef =
>> virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
>> @@ -615,21 +590,17 @@
>> AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr
>> G_GNUC_UNUSED,
>> "hypervisor driver is \'%s\'."),
>> secdef->model, SECURITY_APPARMOR_NAME);
>> if (use_apparmor() > 0)
>> - goto cleanup;
>> + return -1;
>> }
>> if ((profile_name = get_profile_name(def)) == NULL)
>> - goto cleanup;
>> + return -1;
>> cmd_str = virCommandToString(cmd, false);
>> VIR_DEBUG("Changing AppArmor profile to %s on %s", profile_name,
>> cmd_str);
>> virCommandSetAppArmorProfile(cmd, profile_name);
>> - rc = 0;
>> - cleanup:
>> - VIR_FREE(profile_name);
>> - VIR_FREE(cmd_str);
>> - return rc;
>> + return 0;
>> }
>> static int
>
© 2016 - 2026 Red Hat, Inc.