From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=48893 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Pv8Ij-0003mN-VX
	for qemu-devel@nongnu.org; Thu, 03 Mar 2011 08:10:43 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1Pv8Ii-0004eS-Ev
	for qemu-devel@nongnu.org; Thu, 03 Mar 2011 08:10:41 -0500
Received: from mx1.redhat.com ([209.132.183.28]:1762)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1Pv8Ih-0004eN-Ti
	for qemu-devel@nongnu.org; Thu, 03 Mar 2011 08:10:40 -0500
Message-ID: <4D6F9344.40001@redhat.com>
Date: Thu, 03 Mar 2011 14:10:28 +0100
From: Jes Sorensen <Jes.Sorensen@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH] Improve error handling in
	do_snapshot_blkdev()
References: <1299149091-13979-1-git-send-email-Jes.Sorensen@redhat.com>
	<AANLkTinXxjGvx6KnsU+_VP5j_nLMzWBAX1-g_u9mv6bF@mail.gmail.com>
In-Reply-To: <AANLkTinXxjGvx6KnsU+_VP5j_nLMzWBAX1-g_u9mv6bF@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com

On 03/03/11 14:06, Stefan Hajnoczi wrote:
> On Thu, Mar 3, 2011 at 10:44 AM,  <Jes.Sorensen@redhat.com> wrote:
>> +    char old_filename[1024];
>> +
>> +    old_filename[1023] = '\0';
>>
>>     if (!filename) {
>>         qerror_report(QERR_MISSING_PARAMETER, "snapshot_file");
>> @@ -591,6 +594,10 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>         goto out;
>>     }
>>
>> +    strncpy(old_filename, bs->filename, 1024);
> 
> strncpy does not NUL-terminate if you reach the maximum length.  The
> source buffer is 1024 chars so we should be fine unless there is a bug
> somewhere else too, but please move the old_filename[1023] = '\0'
> after the strncpy and use sizeof(old_filename) as the maximum instead
> of 1024.

Good point, I was trying to catch it but got it backwards :(

Cheers,
Jes