9:41 p.m.
The first patch fixes the bug. The 2nd patch just updates some code
that I noticed while fixing the bug (because I figured someone would
whine that I was just moving around calls to outdated APIs).
Laine Stump (2):
network: fix crash during cleanup from failure to allocate port
network: replace virSaveLastError() with virErrorPreserveLast()
src/network/bridge_driver.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
--
2.21.0
9:41 p.m.
New subject: [libvirt] [PATCH 1/2] network: fix crash during cleanup from failure to allocate port
During networkPortCreateXML, if networkAllocatePort() failed,
networkReleasePort() would be called, which would (in the case of
network pools of macvtap passthrough devices) attempt to find the
allocated device by comparing port->plug.direct.linkdev to each device
in the pool. Since port->plug.direct.linkdev was still NULL, the
attempted strcmp would result in a SEGV.
Calling networkReleasePort() during error cleanup is something that
should only be done if networkAllocatePort() has already succeeded. It
turns out there is one other possible error exit from
networkPortCreateXML() that happens after networkAllocatePort() has
succeeded, so the code to call networkReleasePort() was just moved
down to there.
Resolves: https://bugzilla.redhat.com/1741390
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/network/bridge_driver.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 1a5d08a00d..dae1def8de 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -5592,20 +5592,20 @@ networkPortCreateXML(virNetworkPtr net,
rc = networkNotifyPort(obj, portdef);
else
rc = networkAllocatePort(obj, portdef);
- if (rc < 0) {
+ if (rc < 0)
+ goto cleanup;
+
+ if (virNetworkObjAddPort(obj, portdef, driver->stateDir) < 0) {
virErrorPtr saved;
+
saved = virSaveLastError();
ignore_value(networkReleasePort(obj, portdef));
+ virNetworkPortDefFree(portdef);
virSetError(saved);
virFreeError(saved);
goto cleanup;
}
- if (virNetworkObjAddPort(obj, portdef, driver->stateDir) < 0) {
- virNetworkPortDefFree(portdef);
- goto cleanup;
- }
-
ret = virGetNetworkPort(net, portdef->uuid);
cleanup:
virNetworkObjEndAPI(&obj);
--
2.21.0
9:42 p.m.
New subject: [libvirt] [PATCH 2/2] network: replace virSaveLastError() with virErrorPreserveLast()
virErrorPreserveLast()/virErrorRestore() (added in commit 8333e7455
back in 2017), do a better better job of saving and restoring the last
libvirt error than virSaveLastError()/virErrorRestore() (they're
simpler, and they also save/restore the system errno).
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/network/bridge_driver.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index dae1def8de..9059296e55 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2987,13 +2987,12 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
cleanup:
if (ret < 0) {
+ virErrorPtr save_err;
+
+ virErrorPreserveLast(&save_err);
virNetworkObjUnsetDefTransient(obj);
- virErrorPtr save_err = virSaveLastError();
- int save_errno = errno;
networkShutdownNetwork(driver, obj);
- virSetError(save_err);
- virFreeError(save_err);
- errno = save_errno;
+ virErrorRestore(&save_err);
}
return ret;
}
@@ -5596,13 +5595,13 @@ networkPortCreateXML(virNetworkPtr net,
goto cleanup;
if (virNetworkObjAddPort(obj, portdef, driver->stateDir) < 0) {
- virErrorPtr saved;
+ virErrorPtr save_err;
- saved = virSaveLastError();
+ virErrorPreserveLast(&save_err);
ignore_value(networkReleasePort(obj, portdef));
virNetworkPortDefFree(portdef);
- virSetError(saved);
- virFreeError(saved);
+ virErrorRestore(&save_err);
+
goto cleanup;
}
--
2.21.0
3:39 a.m.
On 8/16/19 4:41 AM, Laine Stump wrote:
The first patch fixes the bug. The 2nd patch just updates some code
that I noticed while fixing the bug (because I figured someone would
whine that I was just moving around calls to outdated APIs).
Laine Stump (2):
network: fix crash during cleanup from failure to allocate port
network: replace virSaveLastError() with virErrorPreserveLast()
src/network/bridge_driver.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1927
days inactive
1927
days old
3 comments
2 participants
participants (2)
-
Laine Stump -
Michal Privoznik