From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NCWP8-0000Q2-Uz for qemu-devel@nongnu.org; Mon, 23 Nov 2009 05:44:23 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NCWP3-0000Mu-8X for qemu-devel@nongnu.org; Mon, 23 Nov 2009 05:44:21 -0500 Received: from [199.232.76.173] (port=49062 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NCWP2-0000Mi-GD for qemu-devel@nongnu.org; Mon, 23 Nov 2009 05:44:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17480) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NCWP1-0004Ah-Q8 for qemu-devel@nongnu.org; Mon, 23 Nov 2009 05:44:16 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nANAiDTi011463 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 23 Nov 2009 05:44:13 -0500 Date: Mon, 23 Nov 2009 10:44:11 +0000 From: "Daniel P. Berrange" Subject: Re: [Qemu-devel] Fwd: qemu code review Message-ID: <20091123104411.GA22000@redhat.com> References: <4B03DD07.7090300@redhat.com> <4B0445B1.1080207@mail.berlios.de> <4B050B5C.4080908@redhat.com> <200911191311.56137.sgrubb@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200911191311.56137.sgrubb@redhat.com> Reply-To: "Daniel P. Berrange" List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Steve Grubb Cc: Kevin Wolf , "qemu-devel@nongnu.org" On Thu, Nov 19, 2009 at 01:11:56PM -0500, Steve Grubb wrote: > On Thursday 19 November 2009 04:09:48 am Kevin Wolf wrote: > > >> ... > > >> In hw/e1000.c at line 89, vlan is declared to be 4 bytes. At line 382 is > > >> an attempt to do a memmove over it with a size of 12. > > > > > > Obviously this was intentional. Would replacing > > > memmove(tp->vlan, tp->data, 12); > > > by > > > memmove(tp->data - 4, tp->data, 12); > > > be better and satisfy the analysis tool? > > No. Its likely point out a negative index. > > > > Or even better (hopefully the compiler will combine both statements) > > > memmove(tp->vlan, tp->data, 4); > > > memmove(tp->data, tp->data + 4, 8); > > This would make it happier. But if a comment was made that its intentionally > overrunning the vlan array, it would cause less concern. Even if it was delibrate, it should be changed because when you build with FORTIFY_SOURCE, glibc tries to add check for overruns in this type of scenario and will abort the process. We're already being hit by aborts() on these delibrate overruns in the vvfat block driver https://bugzilla.redhat.com/show_bug.cgi?id=538047 Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|