[libvirt] [PATCH] remote_driver.c: also zero out ->saslDecodedOffset member

Technically, the initialization of ->saslDecodedOffset is not required, but... What could go wrong if saslDecodedOffset is left with some positive value in the if-block below? Well, in that same if-block priv->saslDecoded is set to NULL, and in that case, the code above *usually* sets priv->saslDecodedOffset to 0, but not if we hit one of the intervening returns. In that case, you have to wonder if priv->saslDecoded can be set to non-NULL, yet with priv->saslDecodedOffset still left with the now-bogus positive value. In that case, with an initial saslDecodedLength of 0, we'd compute this: wantData = (priv->saslDecodedLength - priv->saslDecodedOffset and that would be negative. The very next statement would cause trouble, as the negative value treated-as-size_t would wreak havoc: memcpy(priv->buffer + priv->bufferOffset, priv->saslDecoded + priv->saslDecodedOffset, wantData); Luckily, the semantics of sasl_decode ensure that does not happen. Hence, an alternate (though riskier) patch would be to skip that of priv->saslDecodedOffset altogether.
From a3578fc03714a210678e6bcf62d162d92c9d07c0 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 14 Dec 2009 13:00:01 +0100 Subject: [PATCH] remote_driver.c: also zero out ->saslDecodedOffset member
* src/remote/remote_driver.c (remoteIOReadMessage): ...rather than zeroing out priv->saslDecodedLength twice. --- src/remote/remote_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index affeb0f..77962fe 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7782,7 +7782,7 @@ remoteIOReadMessage(virConnectPtr conn, struct private_data *priv, priv->saslDecodedOffset += wantData; priv->bufferOffset += wantData; if (priv->saslDecodedOffset == priv->saslDecodedLength) { - priv->saslDecodedLength = priv->saslDecodedLength = 0; + priv->saslDecodedOffset = priv->saslDecodedLength = 0; priv->saslDecoded = NULL; } -- 1.6.6.rc2.275.g51e2d

On Mon, Dec 14, 2009 at 01:30:48PM +0100, Jim Meyering wrote:
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index affeb0f..77962fe 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7782,7 +7782,7 @@ remoteIOReadMessage(virConnectPtr conn, struct private_data *priv, priv->saslDecodedOffset += wantData; priv->bufferOffset += wantData; if (priv->saslDecodedOffset == priv->saslDecodedLength) { - priv->saslDecodedLength = priv->saslDecodedLength = 0; + priv->saslDecodedOffset = priv->saslDecodedLength = 0; priv->saslDecoded = NULL; }
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Daniel P. Berrange
-
Jim Meyering