From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sasha Levin <levinsasha928@gmail.com>
Subject: Re: [PATCH 2/3] kvm tools: check negative value of num_pages
Date: Thu, 11 Aug 2011 08:37:01 +0300
Message-ID: <1313041021.10059.9.camel@lappy>
References: <1313039267-25951-1-git-send-email-walimisdev@gmail.com>
	 <1313039267-25951-2-git-send-email-walimisdev@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Cc: Pekka Enberg <penberg@kernel.org>, Ingo Molnar <mingo@elte.hu>,
	Asias He <asias.hejun@gmail.com>, kvm@vger.kernel.org
To: Liming Wang <walimisdev@gmail.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-wy0-f174.google.com ([74.125.82.174]:60794 "EHLO
	mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751170Ab1HKFiA (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 11 Aug 2011 01:38:00 -0400
Received: by wyg24 with SMTP id 24so1174660wyg.19
        for <kvm@vger.kernel.org>; Wed, 10 Aug 2011 22:37:58 -0700 (PDT)
In-Reply-To: <1313039267-25951-2-git-send-email-walimisdev@gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, 2011-08-11 at 13:07 +0800, Liming Wang wrote:
> If num_pages is negative, balloon will make kernel crash with
> "out of memory". So we check this value to avoid it to be negative.
> 
> Signed-off-by: Liming Wang <walimisdev@gmail.com>
> ---
>  tools/kvm/virtio/balloon.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/kvm/virtio/balloon.c b/tools/kvm/virtio/balloon.c
> index 854d04b..0223ee4 100644
> --- a/tools/kvm/virtio/balloon.c
> +++ b/tools/kvm/virtio/balloon.c
> @@ -222,8 +222,13 @@ static void handle_sigmem(int sig)
>  {
>  	if (sig == SIGKVMADDMEM)
>  		bdev.config.num_pages += 256;
> -	else
> +	else {
>  		bdev.config.num_pages -= 256;
> +		if ((s32)bdev.config.num_pages < 0){

imo it's worth doing this check before the decrement instead of casting
to signed here.

you also need to wrap the 'if ()' with parenthesis if you add them to
the 'else' case.

> +			bdev.config.num_pages = 0;
> +			return;
> +		}
> +	}
>  
>  	/* Notify that the configuration space has changed */
>  	bdev.isr = VIRTIO_PCI_ISR_CONFIG;

-- 

Sasha.