[PATCH 0/5] secdriver: Misc cleanups
*** BLURB HERE *** Michal Prívozník (5): security: Drop some G_GNUC_UNUSED attributes security_selinux: Drop needless checks for seclabel->model == "selinux" security_dac: Drop needless checks for seclabel->model == "dac" security_apparmor: Drop needless checks for seclabel->model == "apparmor" security_dac: Drop needles typecast in virSecurityDACGenLabel() src/security/security_apparmor.c | 18 +--------- src/security/security_dac.c | 14 ++------ src/security/security_selinux.c | 59 +++----------------------------- 3 files changed, 8 insertions(+), 83 deletions(-) -- 2.49.1
From: Michal Privoznik <mprivozn@redhat.com> Some variables are annotated with G_GNUC_UNUSED attribute so that compiles knows to not complain about unused variable. Well, some variables are annotated and then used. Drop the G_GNUC_UNUSED attribute in such cases. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_apparmor.c | 2 +- src/security/security_dac.c | 4 ++-- src/security/security_selinux.c | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 68ac39611f..40907c9364 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -349,7 +349,7 @@ AppArmorSecurityManagerGetDOI(virSecurityManager *mgr G_GNUC_UNUSED) * called on shutdown. */ static int -AppArmorGenSecurityLabel(virSecurityManager *mgr G_GNUC_UNUSED, +AppArmorGenSecurityLabel(virSecurityManager *mgr, virDomainDef *def) { g_autofree char *profile_name = NULL; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 3ecbc7277d..2210117f12 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1703,7 +1703,7 @@ struct _virSecurityDACChardevCallbackData { static int virSecurityDACRestoreChardevCallback(virDomainDef *def, - virDomainChrDef *dev G_GNUC_UNUSED, + virDomainChrDef *dev, void *opaque) { struct _virSecurityDACChardevCallbackData *data = opaque; @@ -2069,7 +2069,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr, static int virSecurityDACSetChardevCallback(virDomainDef *def, - virDomainChrDef *dev G_GNUC_UNUSED, + virDomainChrDef *dev, void *opaque) { struct _virSecurityDACChardevCallbackData *data = opaque; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index fa5d1568eb..402e0b7737 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1488,7 +1488,7 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) /* Set fcon to the appropriate label for path and mode, or return -1. */ static int -getContext(virSecurityManager *mgr G_GNUC_UNUSED, +getContext(virSecurityManager *mgr, const char *newpath, mode_t mode, char **fcon) { virSecuritySELinuxData *data = virSecurityManagerGetPrivateData(mgr); @@ -2796,7 +2796,7 @@ struct _virSecuritySELinuxChardevCallbackData { static int virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDef *def, - virDomainChrDef *dev G_GNUC_UNUSED, + virDomainChrDef *dev, void *opaque) { struct _virSecuritySELinuxChardevCallbackData *data = opaque; @@ -3071,7 +3071,7 @@ virSecuritySELinuxSetProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED, static int virSecuritySELinuxSetChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED, virDomainDef *def, - bool useBinarySpecificLabel G_GNUC_UNUSED, + bool useBinarySpecificLabel, virCommand *cmd) { /* TODO: verify DOI */ @@ -3232,7 +3232,7 @@ virSecuritySELinuxClearSocketLabel(virSecurityManager *mgr G_GNUC_UNUSED, static int virSecuritySELinuxSetSecurityChardevCallback(virDomainDef *def, - virDomainChrDef *dev G_GNUC_UNUSED, + virDomainChrDef *dev, void *opaque) { struct _virSecuritySELinuxChardevCallbackData *data = opaque; -- 2.49.1
From: Michal Privoznik <mprivozn@redhat.com> In few instances, after virDomainDefGetSecurityLabelDef(SECURITY_SELINUX_NAME) was called, we take the returned secdef and compare secdef->model against SECURITY_SELINUX_NAME. This makes no sense because virDomainDefGetSecurityLabelDef() has already done this comparison. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_selinux.c | 51 --------------------------------- 1 file changed, 51 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 402e0b7737..61a47f23c4 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -899,14 +899,6 @@ virSecuritySELinuxGenLabel(virSecurityManager *mgr, return rc; } - if (seclabel->model && - STRNEQ(seclabel->model, SECURITY_SELINUX_NAME)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label model %1$s is not supported with selinux"), - seclabel->model); - return rc; - } - VIR_DEBUG("type=%d", seclabel->type); switch (seclabel->type) { @@ -3020,13 +3012,6 @@ virSecuritySELinuxVerify(virSecurityManager *mgr G_GNUC_UNUSED, if (secdef == NULL) return 0; - if (STRNEQ(SECURITY_SELINUX_NAME, secdef->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label driver mismatch: '%1$s' model configured for domain, but hypervisor driver is '%2$s'."), - secdef->model, SECURITY_SELINUX_NAME); - return -1; - } - if (secdef->type == VIR_DOMAIN_SECLABEL_STATIC) { if (security_check_context(secdef->label) != 0) { virReportError(VIR_ERR_XML_ERROR, @@ -3049,13 +3034,6 @@ virSecuritySELinuxSetProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED, return 0; VIR_DEBUG("label=%s", secdef->label); - if (STRNEQ(SECURITY_SELINUX_NAME, secdef->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label driver mismatch: '%1$s' model configured for domain, but hypervisor driver is '%2$s'."), - secdef->model, SECURITY_SELINUX_NAME); - if (security_getenforce() == 1) - return -1; - } if (setexeccon_raw(secdef->label) == -1) { virReportSystemError(errno, @@ -3084,13 +3062,6 @@ virSecuritySELinuxSetChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED, return 0; VIR_DEBUG("label=%s", secdef->label); - if (STRNEQ(SECURITY_SELINUX_NAME, secdef->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label driver mismatch: '%1$s' model configured for domain, but hypervisor driver is '%2$s'."), - secdef->model, SECURITY_SELINUX_NAME); - if (security_getenforce() == 1) - return -1; - } /* pick either the common label used by most binaries exec'ed by * libvirt, or the specific label of this binary. @@ -3132,13 +3103,6 @@ virSecuritySELinuxSetDaemonSocketLabel(virSecurityManager *mgr G_GNUC_UNUSED, if (!secdef || !secdef->label) return 0; - if (STRNEQ(SECURITY_SELINUX_NAME, secdef->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label driver mismatch: '%1$s' model configured for domain, but hypervisor driver is '%2$s'."), - secdef->model, SECURITY_SELINUX_NAME); - goto error; - } - if (getcon_raw(&scon) == -1) { virReportSystemError(errno, _("unable to get current process context '%1$s'"), @@ -3175,13 +3139,6 @@ virSecuritySELinuxSetSocketLabel(virSecurityManager *mgr G_GNUC_UNUSED, if (!secdef || !secdef->label) return 0; - if (STRNEQ(SECURITY_SELINUX_NAME, secdef->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label driver mismatch: '%1$s' model configured for domain, but hypervisor driver is '%2$s'."), - secdef->model, SECURITY_SELINUX_NAME); - goto error; - } - VIR_DEBUG("Setting VM %s socket context %s", vm->name, secdef->label); if (setsockcreatecon_raw(secdef->label) == -1) { @@ -3211,14 +3168,6 @@ virSecuritySELinuxClearSocketLabel(virSecurityManager *mgr G_GNUC_UNUSED, if (!secdef || !secdef->label) return 0; - if (STRNEQ(SECURITY_SELINUX_NAME, secdef->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label driver mismatch: '%1$s' model configured for domain, but hypervisor driver is '%2$s'."), - secdef->model, SECURITY_SELINUX_NAME); - if (security_getenforce() == 1) - return -1; - } - if (setsockcreatecon_raw(NULL) == -1) { virReportSystemError(errno, _("unable to clear socket security context '%1$s'"), -- 2.49.1
From: Michal Privoznik <mprivozn@redhat.com> In one instance, after virDomainDefGetSecurityLabelDef(SECURITY_DAC_NAME) was called, we take the returned secdef and compare secdef->model against SECURITY_DAC_NAME. This makes no sense because virDomainDefGetSecurityLabelDef() has already done this comparison. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_dac.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 2210117f12..8fb26168ca 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -2404,14 +2404,6 @@ virSecurityDACGenLabel(virSecurityManager *mgr, return rc; } - if (seclabel->model - && STRNEQ(seclabel->model, SECURITY_DAC_NAME)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label model %1$s is not supported with selinux"), - seclabel->model); - return rc; - } - switch ((virDomainSeclabelType)seclabel->type) { case VIR_DOMAIN_SECLABEL_STATIC: if (seclabel->label == NULL) { -- 2.49.1
From: Michal Privoznik <mprivozn@redhat.com> In few instances, after virDomainDefGetSecurityLabelDef(SECURITY_APPARMOR_NAME) was called, we take the returned secdef and compare secdef->model against SECURITY_APPARMOR_NAME. This makes no sense because virDomainDefGetSecurityLabelDef() has already done this comparison. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_apparmor.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 40907c9364..c7412a76c0 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -520,14 +520,6 @@ AppArmorSetSecurityProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED, if ((profile_name = get_profile_name(def)) == NULL) return -1; - if (STRNEQ(SECURITY_APPARMOR_NAME, secdef->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label driver mismatch: \'%1$s\' model configured for domain, but hypervisor driver is \'%2$s\'."), - secdef->model, SECURITY_APPARMOR_NAME); - if (use_apparmor() > 0) - return -1; - } - VIR_DEBUG("Changing AppArmor profile to %s", profile_name); if (aa_change_profile(profile_name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -557,14 +549,6 @@ AppArmorSetSecurityChildProcessLabel(virSecurityManager *mgr G_GNUC_UNUSED, if (!secdef || !secdef->label) return 0; - if (STRNEQ(SECURITY_APPARMOR_NAME, secdef->model)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("security label driver mismatch: \'%1$s\' model configured for domain, but hypervisor driver is \'%2$s\'."), - secdef->model, SECURITY_APPARMOR_NAME); - if (use_apparmor() > 0) - return -1; - } - if ((profile_name = get_profile_name(def)) == NULL) return -1; -- 2.49.1
From: Michal Privoznik <mprivozn@redhat.com> Inside of virSecurityDACGenLabel() there's a switch() statement and the variable it uses is typecasted to virDomainSeclabelType. Well, as of v7.10.0-rc1~26 the variable is already of that type rendering the typecast needless. Drop it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_dac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 8fb26168ca..98a64bd0ce 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -2404,7 +2404,7 @@ virSecurityDACGenLabel(virSecurityManager *mgr, return rc; } - switch ((virDomainSeclabelType)seclabel->type) { + switch (seclabel->type) { case VIR_DOMAIN_SECLABEL_STATIC: if (seclabel->label == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.49.1
On a Wednesday in 2025, Michal Privoznik via Devel wrote:
*** BLURB HERE ***
Michal Prívozník (5): security: Drop some G_GNUC_UNUSED attributes security_selinux: Drop needless checks for seclabel->model == "selinux" security_dac: Drop needless checks for seclabel->model == "dac" security_apparmor: Drop needless checks for seclabel->model == "apparmor" security_dac: Drop needles typecast in virSecurityDACGenLabel()
s/needles/needless/ in 5/5's commit summary
src/security/security_apparmor.c | 18 +--------- src/security/security_dac.c | 14 ++------ src/security/security_selinux.c | 59 +++----------------------------- 3 files changed, 8 insertions(+), 83 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik