Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NXaDY-0007K5-O3 for qemu-devel@nongnu.org; Wed, 20 Jan 2010 08:03:28 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NXaDI-00074d-5s for qemu-devel@nongnu.org; Wed, 20 Jan 2010 08:03:16 -0500 Received: from [199.232.76.173] (port=49674 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NXaDH-00074K-Pp for qemu-devel@nongnu.org; Wed, 20 Jan 2010 08:03:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50855) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NXaDG-0004lD-6V for qemu-devel@nongnu.org; Wed, 20 Jan 2010 08:03:11 -0500 From: Markus Armbruster Subject: Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE References: <85e877202ec86dac15d392f5e88d5b5d76e3b02f.1263944807.git.quintela@redhat.com> <20100120103324.GA17856@redhat.com> <4B56ECBC.40804@redhat.com> Date: Wed, 20 Jan 2010 14:03:04 +0100 In-Reply-To: (Kirill A. Shutemov's message of "Wed, 20 Jan 2010 14:36:15 +0200") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Kirill A. Shutemov" Cc: Kevin Wolf , qemu-devel@nongnu.org, Juan Quintela "Kirill A. Shutemov" writes: > On Wed, Jan 20, 2010 at 2:15 PM, Markus Armbruster wr= ote: >> Kevin Wolf writes: >> >>> Am 20.01.2010 12:09, schrieb Kirill A. Shutemov: >>>> On Wed, Jan 20, 2010 at 12:33 PM, Daniel P. Berrange >>>> wrote: >>>>> On Wed, Jan 20, 2010 at 08:19:26AM +0200, Kirill A. Shutemov wrote: >>>>>> On Wed, Jan 20, 2010 at 1:56 AM, Juan Quintela = wrote: >> [...] >>>>>>> diff --git a/block/vvfat.c b/block/vvfat.c >>>>>>> index 063f731..df957e5 100644 >>>>>>> --- a/block/vvfat.c >>>>>>> +++ b/block/vvfat.c >>>>>>> @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, >>>>>>> =C2=A0 =C2=A0 { >>>>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0direntry_t* entry=3Darray_get_next(&(s->= directory)); >>>>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0entry->attributes=3D0x28; /* archive | v= olume label */ >>>>>>> - =C2=A0 =C2=A0 =C2=A0 snprintf((char*)entry->name,11,"QEMU VVFAT"); >>>>>>> + =C2=A0 =C2=A0 =C2=A0 memcpy(entry->name,"QEMU VVF",8); >>>>>>> + =C2=A0 =C2=A0 =C2=A0 memcpy(entry->extension,"AT ",3); >>>>>>> =C2=A0 =C2=A0 } >>>>>> >>>>>> Better to use >>>>>> >>>>>> memcpy(entry->name, "QEMU VVFAT", 11); >>>>>> >>>>>> memcpy() doesn't check bounds. >> >> No, this is evil, and may well be flagged by static analysis tools. > > If so, the tool is stupid. > >>>>> It doesn't *currently* check bounds. >>>> >>>> No. memcpy() will never check bounds. It's totaly different from strcp= y, >>>> http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00419.html >>> >>> Regardless if deliberately overflowing the buffer works or doesn't >>> making it explicit is better. Someone might reorder the struct or add >>> new fields in between (okay, unlikely in this case, but still) and >>> you'll overflow into fields you never wanted to touch. >> >> Moreover, compilers are free to put padding between members name and >> extension. > > No, compiler can't add anything between. 'char' is always byte-aligned. You got some reading to do then.