From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:56606)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ppandit@redhat.com>) id 1bfJxQ-0004dc-KQ
	for qemu-devel@nongnu.org; Thu, 01 Sep 2016 00:50:33 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ppandit@redhat.com>) id 1bfJxM-0002DC-G2
	for qemu-devel@nongnu.org; Thu, 01 Sep 2016 00:50:31 -0400
Received: from mx1.redhat.com ([209.132.183.28]:44232)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ppandit@redhat.com>) id 1bfJxM-0002D0-9m
	for qemu-devel@nongnu.org; Thu, 01 Sep 2016 00:50:28 -0400
Date: Thu, 1 Sep 2016 10:20:20 +0530 (IST)
From: P J P <ppandit@redhat.com>
In-Reply-To: <8FCD84DF-B5BC-43FC-9CFB-A64E9FA81919@daynix.com>
Message-ID: <alpine.LFD.2.20.1609011010550.4581@wniryva>
References: <1472626169-12989-1-git-send-email-ppandit@redhat.com>
	<8FCD84DF-B5BC-43FC-9CFB-A64E9FA81919@daynix.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v2] scsi: check page count while
 initialising descriptor rings
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Dmitry Fleytman <dmitry@daynix.com>
Cc: Qemu Developers <qemu-devel@nongnu.org>, Paolo Bonzini <pbonzini@redhat.com>, Tom Victor <vv474172261@gmail.com>

  Hello Dmitry,

+-- On Wed, 31 Aug 2016, Dmitry Fleytman wrote --+
| > -    if ((ri->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)
| > -        || (ri->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES))=
 {
| > -        return -1;
| > -    }
|=20
| Hello Prasad,
|=20
| Why did you decide to move this logic out of pvscsi_ring_init_data()?
| Why not just amend existing =E2=80=9Cif" as you did in v1 of this patch=
?

  'ri->reqRingNumPages' and 'ri->cmpRingNumPages' values are also used in=
=20
routine 'pvscsi_dbg_dump_tx_rings_config' before 'pvscsi_ring_init_data' =
call.=20
if they were to have arbitrary values, this loop could run longer leading=
 to=20
OOB memory access.

    for (i =3D 0; i < rc->reqRingNumPages; i++) {                        =
        =20
        trace_pvscsi_tx_rings_ppn("Request Ring", rc->reqRingPPNs[i]);   =
      =20
    }

Moving above logic to 'pvscsi_on_cmd_setup_rings' helps both functions.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F