[libvirt] [PATCH] conf: fix use after free in virChrdevOpen
Don't free the stream on error if we've successfully added it to the hash table, since it will be freed by virChrdevHashEntryFree callback. Preserve the error message before calling virStreamFree, since it resets the error. Reported by Sergey Fionov on libvir-list. --- src/conf/virchrdev.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 025d4a8..879c27c 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -343,6 +343,8 @@ int virChrdevOpen(virChrdevsPtr devs, virStreamPtr savedStream; const char *path; int ret; + bool added = false; + virErrorPtr savedError; switch (source->type) { case VIR_DOMAIN_CHR_TYPE_PTY: @@ -399,6 +401,7 @@ int virChrdevOpen(virChrdevsPtr devs, if (virHashAddEntry(devs->hash, path, st) < 0) goto error; + added = true; cbdata->devs = devs; if (!(cbdata->path = strdup(path))) { @@ -433,8 +436,16 @@ int virChrdevOpen(virChrdevsPtr devs, return 0; error: - virStreamFree(st); - virHashRemoveEntry(devs->hash, path); + savedError = virSaveLastError(); + + if (added) + virHashRemoveEntry(devs->hash, path); + else + virStreamFree(st); + + virSetError(savedError); + virFreeError(savedError); + if (cbdata) VIR_FREE(cbdata->path); VIR_FREE(cbdata); -- 1.8.1.5
On 05/22/2013 05:37 AM, Ján Tomko wrote:
Don't free the stream on error if we've successfully added it to the hash table, since it will be freed by virChrdevHashEntryFree callback.
Preserve the error message before calling virStreamFree, since it resets the error.
Reported by Sergey Fionov on libvir-list. --- src/conf/virchrdev.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
ACK. It might help if you track down which commit introduced the problem and mention that in the commit message. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/22/2013 04:44 PM, Eric Blake wrote:
On 05/22/2013 05:37 AM, Ján Tomko wrote:
Don't free the stream on error if we've successfully added it to the hash table, since it will be freed by virChrdevHashEntryFree callback.
Preserve the error message before calling virStreamFree, since it resets the error.
Reported by Sergey Fionov on libvir-list. --- src/conf/virchrdev.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
ACK. It might help if you track down which commit introduced the problem and mention that in the commit message.
Thanks, I've amended the commit message and pushed it. Jan
participants (2)
-
Eric Blake -
Ján Tomko