From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60630) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aEXV1-0005Nu-Dj for qemu-devel@nongnu.org; Thu, 31 Dec 2015 02:18:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aEXUy-0003UA-8k for qemu-devel@nongnu.org; Thu, 31 Dec 2015 02:18:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47824) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aEXUy-0003Tu-2z for qemu-devel@nongnu.org; Thu, 31 Dec 2015 02:18:12 -0500 References: <1451537606-16030-1-git-send-email-ppandit@redhat.com> <5684BB0A.3080007@redhat.com> From: Jason Wang Message-ID: <5684D6AC.7030701@redhat.com> Date: Thu, 31 Dec 2015 15:18:04 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATVH v2] net: ne2000: fix bounds check in ioport operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: P J P Cc: Ling Liu , Prasad J Pandit , QEMU Developers On 12/31/2015 01:56 PM, P J P wrote: > +-- On Thu, 31 Dec 2015, Jason Wang wrote --+ > | > - (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { > | > + if (addr < 32 || (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) { > | > | The change is unnecessary. > > Okay. > > | > + if (addr < 32 > | > + || (addr >= NE2000_PMEM_START > | > + && addr + sizeof(uint16_t) < NE2000_MEM_SIZE)) { > | > | I think you mean '<=' instead of '<' here? (And for the other checks below). > > I think <= would lead to an off-by-one, no? The real byte we could touch is in fact addr + sizeof(uint16_t) -1 here. Consider we should allow double bytes access at NE2000_MEM_SIZE - 2, but this patch forbids this. Btw, looking at ne2000_mem_writew(), it has: addr &= ~1; at the beginning, so looks like we are really safe, Need only to care about writel? > As the last array index would be > one less than the size; Same as ne2000_mem_readb() above. > > Thank you. > -- > Prasad J Pandit / Red Hat Product Security Team > 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F