From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34403) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WV02o-000723-0l for qemu-devel@nongnu.org; Tue, 01 Apr 2014 10:52:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WV02i-00011L-UI for qemu-devel@nongnu.org; Tue, 01 Apr 2014 10:52:05 -0400 Date: Tue, 1 Apr 2014 17:52:19 +0300 From: "Michael S. Tsirkin" Message-ID: <20140401145219.GA9237@redhat.com> References: <1396275242-10810-1-git-send-email-mst@redhat.com> <1396275242-10810-30-git-send-email-mst@redhat.com> <20140331154837.GD11125@work-vm> <734C319D-BB55-4819-815E-55E56895F0B1@daynix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <734C319D-BB55-4819-815E-55E56895F0B1@daynix.com> Subject: Re: [Qemu-devel] [PATCH v4 29/30] vmxnet3: validate queues configuration coming from quest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dmitry Fleytman Cc: Anthony Liguori , qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, qemu-stable@nongnu.org, Stefan Hajnoczi , Paolo Bonzini , "Dr. David Alan Gilbert" On Tue, Apr 01, 2014 at 01:04:12PM +0300, Dmitry Fleytman wrote: > > On Mar 31, 2014, at 18:48 PM, Dr. David Alan Gilbert > wrote: > > > * Michael S. Tsirkin (mst@redhat.com) wrote: > > From: Dmitry Fleytman > > CVE-2013-4544 > > Signed-off-by: Dmitry Fleytman > Reported-by: Michael S. Tsirkin > Signed-off-by: Michael S. Tsirkin > --- > hw/net/vmxnet3.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index 8c6df05..0fa54e7 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -1336,6 +1336,17 @@ static void vmxnet3_validate_interrupts > (VMXNET3State *s) > } > } > > +static void vmxnet3_validate_queues(VMXNET3State *s) > +{ > + if (s->txq_num > VMXNET3_DEVICE_MAX_TX_QUEUES) { > + hw_error("Bad TX queues number: %d\n", s->txq_num); > + } > + > + if (s->rxq_num > VMXNET3_DEVICE_MAX_RX_QUEUES) { > + hw_error("Bad RX queues number: %d\n", s->rxq_num); > + } > > > Why isn't that >= ? > (I agree it matches the original assert). > > Vmxnet3RxqDescr rxq_descr[VMXNET3_DEVICE_MAX_RX_QUEUES]; > Vmxnet3TxqDescr txq_descr[VMXNET3_DEVICE_MAX_TX_QUEUES]; > > static void vmxnet3_fill_stats(VMXNET3State *s) > { > int i; > for (i = 0; i < s->txq_num; i++) { > cpu_physical_memory_write(s->txq_descr[i].tx_stats_pa, > &s->txq_descr[i].txq_stats, > sizeof(s->txq_descr[i].txq_stats)); > } > > so that looks like it's 0 indexed. > > Dave > > > HI Dave, thanks for the review. > > The verification is ok because s->txq_num and s->rxq_num are total number of > queues, not a queue index. > > Dmitry. A code comment here might be helpful. > > > > > +} > + > static void vmxnet3_activate_device(VMXNET3State *s) > { > int i; > @@ -1382,7 +1393,7 @@ static void vmxnet3_activate_device(VMXNET3State > *s) > VMXNET3_READ_DRV_SHARED8(s->drv_shmem, > devRead.misc.numRxQueues); > > VMW_CFPRN("Number of TX/RX queues %u/%u", s->txq_num, s->rxq_num); > - assert(s->txq_num <= VMXNET3_DEVICE_MAX_TX_QUEUES); > + vmxnet3_validate_queues(s); > > qdescr_table_pa = > VMXNET3_READ_DRV_SHARED64(s->drv_shmem, > devRead.misc.queueDescPA); > -- > MST > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > >