[PATCH 0/1] Checking the value returned by the function
The code compares the value of the variable 'ret' with 1. I assume that it is necessary to compare the value of 'ret' with -1 in this code fragment. Sergey Mironov (1): Checking the value returned by the function src/security/security_selinux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.31.1
The virSecuritySELinuxSetFilecon function (by definition) always returns values 0 or -1. The result of this function is written to 'ret'. The code compares the value of the variable 'ret' with 1. Signed-off-by: Sergey Mironov <mironov@fintech.ru> --- src/security/security_selinux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 7914aba84d..7bff780ddf 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1988,7 +1988,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManager *mgr, ret = virSecuritySELinuxSetFilecon(mgr, path, use_label, remember); } - if (ret == 1 && !disk_seclabel) { + if (ret == -1 && !disk_seclabel) { /* If we failed to set a label, but virt_use_nfs let us * proceed anyway, then we don't need to relabel later. */ disk_seclabel = virSecurityDeviceLabelDefNew(SECURITY_SELINUX_NAME); -- 2.31.1
On Wed, Oct 11, 2023 at 15:24:17 +0300, Sergey Mironov wrote:
The virSecuritySELinuxSetFilecon function (by definition) always returns values 0 or -1. The result of this function is written to 'ret'. The code compares the value of the variable 'ret' with 1.
Signed-off-by: Sergey Mironov <mironov@fintech.ru> --- src/security/security_selinux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 7914aba84d..7bff780ddf 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1988,7 +1988,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManager *mgr, ret = virSecuritySELinuxSetFilecon(mgr, path, use_label, remember); }
- if (ret == 1 && !disk_seclabel) { + if (ret == -1 && !disk_seclabel) {
Based on the git history it appears that this condition is impossible to satisfy for more than 10 years. If that is so we need to first assess if we even want to make this code active, given that it wans't a problem for such a long time. It's also possible that it became impossible to reach after a change outside of the context I've checked, but either way it should not be blindly fixed even if it appears to be what the author intended originally
participants (2)
-
Peter Krempa -
Sergey Mironov