From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.33)
	id 1BbhQa-0005JR-1p
	for qemu-devel@nongnu.org; Sat, 19 Jun 2004 11:06:44 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.33)
	id 1BbhQX-0005Hy-SQ
	for qemu-devel@nongnu.org; Sat, 19 Jun 2004 11:06:43 -0400
Received: from [199.232.76.173] (helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.33) id 1BbhQX-0005HM-LB
	for qemu-devel@nongnu.org; Sat, 19 Jun 2004 11:06:41 -0400
Received: from [206.72.67.39] (helo=claudius.sentinelchicken.org)
	by monty-python.gnu.org with smtp (Exim 4.34) id 1BbhP9-00042C-US
	for qemu-devel@nongnu.org; Sat, 19 Jun 2004 11:05:16 -0400
Date: Sat, 19 Jun 2004 08:05:15 -0700
From: Tim <tim-qemu@sentinelchicken.org>
Subject: Re: [Qemu-devel] [PATCH] security_20040618
Message-ID: <20040619150514.GB1962@sentinelchicken.org>
References: <200406181841.i5IIfZQa019337@treas.simtreas.ru>
	<40D3ED57.2040506@simtreas.ru>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <40D3ED57.2040506@simtreas.ru>
Reply-To: qemu-devel@nongnu.org
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.gnu.org/pipermail/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: "Vladimir N. Oleynik" <dzo@simtreas.ru>
Cc: qemu-devel@nongnu.org

> > --- qemu-current/monitor.c	2004-06-16 20:49:59.000000000 -0700
> > +++ qemu-dev/monitor.c	2004-06-17 22:12:49.000000000 -0700
> >                  str = qemu_malloc(strlen(buf) + 1);
> > -                strcpy(str, buf);
> > +                pstrcpy(str, strlen(buf) + 1,  buf);
> 
> In my opinion, it already absolutely unnecessary correction.
> There is in this place no problem.

Yeah, you are probably right.  I looked at that one on 3 seperate
occasions before making the change, since I recognized that there are
very few conditions where it could possibly be a problem, and come to
think of it, this fix doesn't mitigate those conditions.

That chunk of code makes me uncomfortable for other reasons though (does
qemu_malloc() return NULL ever?  could buf possibly be missing a
trailing '\0' ever?) so I'll re-visit it again and see what makes the
most sense.  The pstrcpy isn't hurting anything though.  Slightly slower
copy, due to the length checking, but it isn't in a critical piece of
code (monitor.c is just for the user interface command prompt, right?),
so I also don't see a reason to remove it, esp if changes in the future
open up the possibility of an overflow.

thanks for the comment,
tim