In 4674fc6afd6 I've implemented transactions for selinux driver.
Well, now that I am working in this area I've notice a subtle
bug: @ret is initialized to 0 instead of -1. Facepalm.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
I wonder how this could survive this long (~2y) not being noticed.
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 467d1e6bfe..c09404f6f8 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1091,7 +1091,7 @@ virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
pid_t pid)
{
virSecuritySELinuxContextListPtr list;
- int ret = 0;
+ int ret = -1;
list = virThreadLocalGet(&contextList);
if (!list)
--
2.18.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 13, 2018 at 04:55 PM +0100, Michal Privoznik <mprivozn@redhat.com> wrote: > In 4674fc6afd6 I've implemented transactions for selinux driver. > Well, now that I am working in this area I've notice a subtle > bug: @ret is initialized to 0 instead of -1. Facepalm. > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > > I wonder how this could survive this long (~2y) not being noticed. > > 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 467d1e6bfe..c09404f6f8 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -1091,7 +1091,7 @@ virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, > pid_t pid) > { > virSecuritySELinuxContextListPtr list; > - int ret = 0; > + int ret = -1; > > list = virThreadLocalGet(&contextList); > if (!list) > -- > 2.18.1 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com> Actually, I had the same fix in my pipeline :) -- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Return -1 and report an error message if no transaction is set and
virSecuritySELinuxTransactionCommit is called.
The function description of virSecuritySELinuxTransactionCommit says:
"Also it is considered as error if there's no transaction set and this
function is called."
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
Please apply this patch after the patch
"virSecuritySELinuxTransactionCommit: Don't mask error" from Michal.
---
src/security/security_selinux.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index c09404f6f833..780d650c69ea 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1094,8 +1094,11 @@ virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
int ret = -1;
list = virThreadLocalGet(&contextList);
- if (!list)
- return 0;
+ if (!list) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("No transaction is set"));
+ return -1;
+ }
if (virThreadLocalSet(&contextList, NULL) < 0) {
virReportSystemError(errno, "%s",
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11/13/2018 05:32 PM, Marc Hartmayer wrote: > Return -1 and report an error message if no transaction is set and > virSecuritySELinuxTransactionCommit is called. > > The function description of virSecuritySELinuxTransactionCommit says: > > "Also it is considered as error if there's no transaction set and this > function is called." > > Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> > --- > > Please apply this patch after the patch > "virSecuritySELinuxTransactionCommit: Don't mask error" from Michal. > > --- > src/security/security_selinux.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c > index c09404f6f833..780d650c69ea 100644 > --- a/src/security/security_selinux.c > +++ b/src/security/security_selinux.c > @@ -1094,8 +1094,11 @@ virSecuritySELinuxTransactionCommit(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, > int ret = -1; > > list = virThreadLocalGet(&contextList); > - if (!list) > - return 0; > + if (!list) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("No transaction is set")); > + return -1; > + } > > if (virThreadLocalSet(&contextList, NULL) < 0) { > virReportSystemError(errno, "%s", > He he. ACKed and pushed both. Thanks for the review. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.