9:53 a.m.
From: Jiri Denemark <jdenemar(a)redhat.com>
---
src/remote/remote_driver.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 990bfce..c62e3d6 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2849,8 +2849,12 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
goto done;
if (ret.cookie.cookie_len > 0) {
- *cookie = ret.cookie.cookie_val; /* Caller frees. */
- *cookielen = ret.cookie.cookie_len;
+ if (cookie && cookielen) {
+ *cookie = ret.cookie.cookie_val; /* Caller frees. */
+ *cookielen = ret.cookie.cookie_len;
+ } else {
+ VIR_FREE(ret.cookie.cookie_val);
+ }
}
if (ret.uri_out)
*uri_out = *ret.uri_out; /* Caller frees. */
--
1.7.1
10:02 a.m.
New subject: [libvirt] [PATCH] Fix NULL dereference in remoteDomainMigratePrepare2
On 05/12/2010 08:53 AM, jdenemar(a)redhat.com wrote:
From: Jiri Denemark <jdenemar(a)redhat.com>
---
src/remote/remote_driver.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 990bfce..c62e3d6 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2849,8 +2849,12 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
goto done;
if (ret.cookie.cookie_len > 0) {
- *cookie = ret.cookie.cookie_val; /* Caller frees. */
- *cookielen = ret.cookie.cookie_len;
+ if (cookie && cookielen) {
+ *cookie = ret.cookie.cookie_val; /* Caller frees. */
+ *cookielen = ret.cookie.cookie_len;
+ } else {
+ VIR_FREE(ret.cookie.cookie_val);
+ }
ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:03 a.m.
On Wed, May 12, 2010 at 04:53:13PM +0200, jdenemar(a)redhat.com wrote:
From: Jiri Denemark <jdenemar(a)redhat.com>
---
src/remote/remote_driver.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 990bfce..c62e3d6 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2849,8 +2849,12 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
goto done;
if (ret.cookie.cookie_len > 0) {
- *cookie = ret.cookie.cookie_val; /* Caller frees. */
- *cookielen = ret.cookie.cookie_len;
+ if (cookie && cookielen) {
+ *cookie = ret.cookie.cookie_val; /* Caller frees. */
+ *cookielen = ret.cookie.cookie_len;
+ } else {
+ VIR_FREE(ret.cookie.cookie_val);
+ }
}
What code would call this with cookie == NULL ? Any such caller
is a bug I believe.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
10:22 a.m.
> diff --git a/src/remote/remote_driver.c
b/src/remote/remote_driver.c
> index 990bfce..c62e3d6 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -2849,8 +2849,12 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
> goto done;
>
> if (ret.cookie.cookie_len > 0) {
> - *cookie = ret.cookie.cookie_val; /* Caller frees. */
> - *cookielen = ret.cookie.cookie_len;
> + if (cookie && cookielen) {
> + *cookie = ret.cookie.cookie_val; /* Caller frees. */
> + *cookielen = ret.cookie.cookie_len;
> + } else {
> + VIR_FREE(ret.cookie.cookie_val);
> + }
> }
What code would call this with cookie == NULL ? Any such caller
is a bug I believe.
Yeah, most likely but we shouldn't crash in that case anyway. Although perhaps
the call should rather fail instead of just silently dropping the cookie if
the caller is not interested in seeing it, what do you think?
Jirka
3:05 a.m.
New subject: [libvirt] [PATCH v2] Fix potential NULL dereference in remoteDomainMigratePrepare2
---
src/remote/remote_driver.c | 19 ++++++++++++++++++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 990bfce..80977a3 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2849,17 +2849,34 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
goto done;
if (ret.cookie.cookie_len > 0) {
+ if (!cookie || !cookielen) {
+ remoteError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("caller ignores cookie or cookielen"));
+ goto error;
+ }
*cookie = ret.cookie.cookie_val; /* Caller frees. */
*cookielen = ret.cookie.cookie_len;
}
- if (ret.uri_out)
+ if (ret.uri_out) {
+ if (!uri_out) {
+ remoteError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("caller ignores uri_out"));
+ goto error;
+ }
*uri_out = *ret.uri_out; /* Caller frees. */
+ }
rv = 0;
done:
remoteDriverUnlock(priv);
return rv;
+error:
+ if (ret.cookie.cookie_len)
+ VIR_FREE(ret.cookie.cookie_val);
+ if (ret.uri_out)
+ VIR_FREE(*ret.uri_out);
+ goto done;
}
static virDomainPtr
--
1.7.1
9:08 a.m.
New subject: [libvirt] [PATCH v2] Fix potential NULL dereference in remoteDomainMigratePrepare2
On 05/13/2010 02:05 AM, Jiri Denemark wrote:
---
src/remote/remote_driver.c | 19 ++++++++++++++++++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 990bfce..80977a3 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2849,17 +2849,34 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
goto done;
if (ret.cookie.cookie_len > 0) {
+ if (!cookie || !cookielen) {
+ remoteError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("caller ignores cookie or cookielen"));
For a moment, I wondered if VIR_ERR_INVALID_ARG would be any better
here, but decided you probably did the right thing since this is
implementation rather than a direct public API.
+ goto error;
+ }
*cookie = ret.cookie.cookie_val; /* Caller frees. */
*cookielen = ret.cookie.cookie_len;
}
- if (ret.uri_out)
+ if (ret.uri_out) {
+ if (!uri_out) {
+ remoteError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("caller ignores uri_out"));
+ goto error;
+ }
*uri_out = *ret.uri_out; /* Caller frees. */
+ }
rv = 0;
done:
remoteDriverUnlock(priv);
return rv;
+error:
+ if (ret.cookie.cookie_len)
+ VIR_FREE(ret.cookie.cookie_val);
+ if (ret.uri_out)
+ VIR_FREE(*ret.uri_out);
+ goto done;
}
ACK; looks nicer than v1 for diagnosing a logic bug in the caller,
rather than papering over it.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:01 p.m.
New subject: [libvirt] [PATCH v2] Fix potential NULL dereference in remoteDomainMigratePrepare2
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -2849,17 +2849,34 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn,
> goto done;
>
> if (ret.cookie.cookie_len > 0) {
> + if (!cookie || !cookielen) {
> + remoteError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("caller ignores cookie or cookielen"));
> + goto error;
> + }
...
ACK; looks nicer than v1 for diagnosing a logic bug in the caller,
rather than papering over it.
Thanks, pushed.
Jirka
5302
days inactive
5308
days old
6 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
jdenemar＠redhat.com -
Jiri Denemark