On 16/10/13 20:31, Daniel P. Berrange wrote:
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
Pushed. Thanks.
Osier