Re: [libvirt] [PATCH v2] rpc: Correct the wrong payload size checking

On Tue, Oct 15, 2013 at 11:15:32PM +0800, Osier Yang wrote: > <...> > /* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX > * and VIR_NET_MESSAGE_INITIAL. > */ > const VIR_NET_MESSAGE_LEN_MAX = 4; > </...> > > However, msg->bufferLength includes the length word. The wrong checking > was introduced by commit e914dcfd. > > * src/rpc/virnetmessage.c: > - Correct the checking in virNetMessageEncodePayloadRaw > - Use a new variable to track the new payload length in > virNetMessageEncodePayloadRaw > --- > src/rpc/virnetmessage.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c > index 8f4e4bc..d60366b 100644 > --- a/src/rpc/virnetmessage.c > +++ b/src/rpc/virnetmessage.c > @@ -346,15 +346,16 @@ int virNetMessageEncodePayload(virNetMessagePtr msg, > > /* Try to encode the payload. If the buffer is too small increase it. */ > while (!(*filter)(&xdr, data)) { > - if ((msg->bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4 > VIR_NET_MESSAGE_MAX) { > + unsigned int newlen = (msg->bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4; > + > + if (newlen > VIR_NET_MESSAGE_MAX) { > virReportError(VIR_ERR_RPC, "%s", _("Unable to encode message payload")); > goto error; > } > > xdr_destroy(&xdr); > > - msg->bufferLength = (msg->bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4 + > - VIR_NET_MESSAGE_LEN_MAX; > + msg->bufferLength = newlen + VIR_NET_MESSAGE_LEN_MAX; > > if (VIR_REALLOC_N(msg->buffer, msg->bufferLength) < 0) > goto error; > @@ -426,10 +427,15 @@ int virNetMessageEncodePayloadRaw(virNetMessagePtr msg, > > /* If the message buffer is too small for the payload increase it accordingly. */ > if ((msg->bufferLength - msg->bufferOffset) < len) { > - if ((msg->bufferOffset + len) > VIR_NET_MESSAGE_MAX) { > + if ((msg->bufferOffset + len) > > + (VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX)) { > virReportError(VIR_ERR_RPC, > - _("Stream data too long to send (%zu bytes needed, %zu bytes available)"), > - len, (VIR_NET_MESSAGE_MAX - msg->bufferOffset)); > + _("Stream data too long to send " > + "(%zu bytes needed, %zu bytes available)"), > + len, > + VIR_NET_MESSAGE_MAX + > + VIR_NET_MESSAGE_LEN_MAX - > + msg->bufferOffset); > return -1; > } ACK