[PATCH v3 2/4] security: replace uses of label and VIR_FREE by g_autofree

Moving towards full adoption of GLib APIs in the AppArmor code. Signed-off-by: Georgia Garcia <georgia.garcia(a)canonical.com&gt; Reviewed-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/security/security_apparmor.c | 42 +++++-------- src/security/virt-aa-helper.c | 100 ++++++++++--------------------- 2 files changed, 46 insertions(+), 96 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index ae2175d334..91c51f6395 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -115,37 +115,28 @@ profile_loaded(const char *str) static int profile_status_file(const char *str) { - char *profile = NULL; - char *content = NULL; - char *tmp = NULL; - int rc = -1; + g_autofree char *profile = NULL; + g_autofree char *content = NULL; + g_autofree char *tmp = NULL; int len; profile = g_strdup_printf("%s/%s", APPARMOR_DIR "/libvirt", str); if (!virFileExists(profile)) - goto failed; + return -1; if ((len = virFileReadAll(profile, MAX_FILE_LEN, &content)) < 0) { virReportSystemError(errno, _("Failed to read \'%1$s\'"), profile); - goto failed; + return -1; } /* create string that is ' <str> flags=(complain)\0' */ tmp = g_strdup_printf(" %s flags=(complain)", str); if (strstr(content, tmp) != NULL) - rc = 0; - else - rc = 1; - - failed: - VIR_FREE(tmp); - VIR_FREE(profile); - VIR_FREE(content); - - return rc; + return 0; + return 1; } /* @@ -218,7 +209,7 @@ static int use_apparmor(void) { int rc = -1; - char *libvirt_daemon = NULL; + g_autofree char *libvirt_daemon = NULL; if (virFileResolveLink("/proc/self/exe", &libvirt_daemon) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -232,7 +223,7 @@ use_apparmor(void) return 1; if (access(APPARMOR_PROFILES_PATH, R_OK) != 0) - goto cleanup; + return rc; /* First check profile status using full binary path. If that fails * check using profile name. @@ -247,8 +238,6 @@ use_apparmor(void) rc = -1; } - cleanup: - VIR_FREE(libvirt_daemon); return rc; } @@ -950,7 +939,8 @@ AppArmorSetChardevLabel(virSecurityManager *mgr, virDomainChrSourceDef *dev_source, bool chardevStdioLogd G_GNUC_UNUSED) { - char *in = NULL, *out = NULL; + g_autofree char *in = NULL; + g_autofree char *out = NULL; int ret = -1; virSecurityLabelDef *secdef; @@ -971,11 +961,11 @@ AppArmorSetChardevLabel(virSecurityManager *mgr, out = g_strdup_printf("%s.out", dev_source->data.file.path); if (virFileExists(in)) { if (reload_profile(mgr, def, in, true) < 0) - goto done; + return ret; } if (virFileExists(out)) { if (reload_profile(mgr, def, out, true) < 0) - goto done; + return ret; } ret = reload_profile(mgr, def, dev_source->data.file.path, true); break; @@ -995,9 +985,6 @@ AppArmorSetChardevLabel(virSecurityManager *mgr, break; } - done: - VIR_FREE(in); - VIR_FREE(out); return ret; } @@ -1083,12 +1070,11 @@ AppArmorSetPathLabel(virSecurityManager *mgr, bool allowSubtree) { int rc = -1; - char *full_path = NULL; + g_autofree char *full_path = NULL; if (allowSubtree) { full_path = g_strdup_printf("%s/{,**}", path); rc = reload_profile(mgr, def, full_path, true); - VIR_FREE(full_path); } else { rc = reload_profile(mgr, def, path, true); } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 94a28bf331..1626d5a89c 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -146,9 +146,8 @@ vah_info(const char *str) static int parserCommand(const char *profile_name, const char cmd) { - int result = -1; char flag[3]; - char *profile; + g_autofree char *profile = NULL; int status; int ret; @@ -163,7 +162,7 @@ parserCommand(const char *profile_name, const char cmd) if (!virFileExists(profile)) { vah_error(NULL, 0, _("profile does not exist")); - goto cleanup; + return -1; } else { const char * const argv[] = { "/sbin/apparmor_parser", flag, profile, NULL @@ -175,23 +174,18 @@ parserCommand(const char *profile_name, const char cmd) (WIFEXITED(status) && WEXITSTATUS(status) != 0)) { if (ret != 0) { vah_error(NULL, 0, _("failed to run apparmor_parser")); - goto cleanup; + return -1; } else if (cmd == 'R' && WIFEXITED(status) && WEXITSTATUS(status) == 234) { vah_warning(_("unable to unload already unloaded profile")); } else { vah_error(NULL, 0, _("apparmor_parser exited with error")); - goto cleanup; + return -1; } } } - result = 0; - - cleanup: - VIR_FREE(profile); - - return result; + return 0; } /* @@ -201,18 +195,17 @@ static int update_include_file(const char *include_file, const char *included_files, bool append) { - int rc = -1; int plen, flen = 0; int fd; - char *pcontent = NULL; - char *existing = NULL; + g_autofree char *pcontent = NULL; + g_autofree char *existing = NULL; const char *warning = "# DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT.\n"; if (virFileExists(include_file)) { flen = virFileReadAll(include_file, MAX_FILE_LEN, &existing); if (flen < 0) - return rc; + return -1; } if (append && virFileExists(include_file)) @@ -223,38 +216,31 @@ update_include_file(const char *include_file, const char *included_files, plen = strlen(pcontent); if (plen > MAX_FILE_LEN) { vah_error(NULL, 0, _("invalid length for new profile")); - goto cleanup; + return -1; } /* only update the disk profile if it is different */ if (flen > 0 && flen == plen && STREQLEN(existing, pcontent, plen)) { - rc = 0; - goto cleanup; + return 0; } /* write the file */ if ((fd = open(include_file, O_CREAT | O_TRUNC | O_WRONLY, 0644)) == -1) { vah_error(NULL, 0, _("failed to create include file")); - goto cleanup; + return -1; } if (safewrite(fd, pcontent, plen) < 0) { /* don't write the '\0' */ VIR_FORCE_CLOSE(fd); vah_error(NULL, 0, _("failed to write to profile")); - goto cleanup; + return -1; } if (VIR_CLOSE(fd) != 0) { vah_error(NULL, 0, _("failed to close or write to profile")); - goto cleanup; + return -1; } - rc = 0; - - cleanup: - VIR_FREE(pcontent); - VIR_FREE(existing); - - return rc; + return 0; } /* @@ -574,7 +560,7 @@ caps_mockup(vahControl * ctl, const char *xmlStr) { g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; - char *arch; + g_autofree char *arch = NULL; if (!(xml = virXMLParse(NULL, xmlStr, _("(domain_definition)"), "domain", &ctxt, NULL, false))) { @@ -600,7 +586,6 @@ caps_mockup(vahControl * ctl, const char *xmlStr) ctl->arch = virArchFromHost(); } else { ctl->arch = virArchFromString(arch); - VIR_FREE(arch); } return 0; @@ -685,15 +670,15 @@ get_definition(vahControl * ctl, const char *xmlStr) static int vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive) { - char *tmp = NULL; int rc = -1; bool readonly = true; bool explicit_deny_rule = true; char *sub = NULL; - char *perms_new = NULL; - char *pathdir = NULL; - char *pathtmp = NULL; - char *pathreal = NULL; + g_autofree char *tmp = NULL; + g_autofree char *perms_new = NULL; + g_autofree char *pathdir = NULL; + g_autofree char *pathtmp = NULL; + g_autofree char *pathreal = NULL; if (path == NULL) return rc; @@ -730,7 +715,7 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive if ((pathreal = realpath(pathdir, NULL)) == NULL) { vah_error(NULL, 0, pathdir); vah_error(NULL, 0, _("could not find realpath")); - goto cleanup; + return rc; } tmp = g_strdup_printf("%s%s", pathreal, pathtmp); } @@ -754,7 +739,7 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive vah_error(NULL, 0, path); vah_error(NULL, 0, _("skipped restricted file")); } - goto cleanup; + return rc; } if (tmp[strlen(tmp) - 1] == '/') @@ -771,13 +756,6 @@ vah_add_path(virBuffer *buf, const char *path, const char *perms, bool recursive virBufferAsprintf(buf, " \"%s/\" r,\n", tmp); } - cleanup: - VIR_FREE(pathdir); - VIR_FREE(pathtmp); - VIR_FREE(pathreal); - VIR_FREE(perms_new); - VIR_FREE(tmp); - return rc; } @@ -793,36 +771,28 @@ vah_add_file_chardev(virBuffer *buf, const char *perms, const int type) { - char *pipe_in; - char *pipe_out; - int rc = -1; + g_autofree char *pipe_in = NULL; + g_autofree char *pipe_out = NULL; if (type == VIR_DOMAIN_CHR_TYPE_PIPE) { /* add the pipe input */ pipe_in = g_strdup_printf("%s.in", path); if (vah_add_file(buf, pipe_in, perms) != 0) - goto clean_pipe_in; + return -1; /* add the pipe output */ pipe_out = g_strdup_printf("%s.out", path); if (vah_add_file(buf, pipe_out, perms) != 0) - goto clean_pipe_out; - - rc = 0; - clean_pipe_out: - VIR_FREE(pipe_out); - clean_pipe_in: - VIR_FREE(pipe_in); + return -1; } else { /* add the file */ if (vah_add_file(buf, path, perms) != 0) return -1; - rc = 0; } - return rc; + return 0; } static int @@ -1473,8 +1443,8 @@ main(int argc, char **argv) vahControl _ctl = { 0 }; vahControl *ctl = &_ctl; int rc = -1; - char *profile = NULL; - char *include_file = NULL; + g_autofree char *profile = NULL; + g_autofree char *include_file = NULL; off_t size; bool purged = 0; @@ -1517,7 +1487,7 @@ main(int argc, char **argv) if (ctl->cmd == 'D') unlink(include_file); } else if (ctl->cmd == 'c' || ctl->cmd == 'r') { - char *included_files = NULL; + g_autofree char *included_files = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; if (ctl->cmd == 'c' && virFileExists(profile)) @@ -1579,7 +1549,7 @@ main(int argc, char **argv) /* create the profile from TEMPLATE */ if (ctl->cmd == 'c' || purged) { - char *tmp = NULL; + g_autofree char *tmp = NULL; #if defined(WITH_APPARMOR_3) const char *ifexists = "if exists "; #else @@ -1597,7 +1567,6 @@ main(int argc, char **argv) vah_error(ctl, 0, _("could not create profile")); unlink(include_file); } - VIR_FREE(tmp); } if (rc == 0 && !ctl->dryrun) { @@ -1613,14 +1582,9 @@ main(int argc, char **argv) unlink(profile); } } - cleanup: - VIR_FREE(included_files); } - + cleanup: vahDeinit(ctl); - VIR_FREE(profile); - VIR_FREE(include_file); - exit(rc == 0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- 2.43.0