The pointer related to uml_driver needs to be checked before its usage
inside the function. Some attributes of the driver are being accessed
while the pointer is NULL considering the current logic.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
src/uml/uml_driver.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index fcd468243e..d1c71d8521 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1193,6 +1193,13 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn,
{
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+ /* URI was good, but driver isn't active */
+ if (uml_driver == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("uml state driver is not active"));
+ return VIR_DRV_OPEN_ERROR;
+ }
+
/* Check path and tell them correct path if they made a mistake */
if (uml_driver->privileged) {
if (STRNEQ(conn->uri->path, "/system") &&
@@ -1211,13 +1218,6 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn,
}
}
- /* URI was good, but driver isn't active */
- if (uml_driver == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("uml state driver is not active"));
- return VIR_DRV_OPEN_ERROR;
- }
-
if (virConnectOpenEnsureACL(conn) < 0)
return VIR_DRV_OPEN_ERROR;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I reworded the commit subject a little and pushed. One more thing to note is that we don't put a 'period' at the end of a commit subject, I've noticed it in a few of your patches, it's just I kept removing them intuitively and always forgot to mention it :). Reviewed-by: Erik Skultety <eskultet@redhat.com> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 9/28/18 12:13 AM, Julio Faracco wrote: > The pointer related to uml_driver needs to be checked before its usage > inside the function. Some attributes of the driver are being accessed > while the pointer is NULL considering the current logic. > I see it's pushed already... Perhaps something in the future to consider - when finding stuff like this, look for the commit that broke the original logic, then indicate in the commit message what you found. For this commit 0420a0324 changed to check @uml_driver->privileged before !uml_driver was checked. > Signed-off-by: Julio Faracco <jcfaracco@gmail.com> > --- > src/uml/uml_driver.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c > index fcd468243e..d1c71d8521 100644 > --- a/src/uml/uml_driver.c > +++ b/src/uml/uml_driver.c > @@ -1193,6 +1193,13 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn, > { > virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); > > + /* URI was good, but driver isn't active */ ^^ This comment is wrong now ;-) No big deal... In any case the logic at least is the same as most other drivers now. John > + if (uml_driver == NULL) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("uml state driver is not active")); > + return VIR_DRV_OPEN_ERROR; > + } > + > /* Check path and tell them correct path if they made a mistake */ > if (uml_driver->privileged) { > if (STRNEQ(conn->uri->path, "/system") && > @@ -1211,13 +1218,6 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn, > } > } > > - /* URI was good, but driver isn't active */ > - if (uml_driver == NULL) { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("uml state driver is not active")); > - return VIR_DRV_OPEN_ERROR; > - } > - > if (virConnectOpenEnsureACL(conn) < 0) > return VIR_DRV_OPEN_ERROR; > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.