From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47298)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1VDiWa-00024Q-Nr
	for qemu-devel@nongnu.org; Sun, 25 Aug 2013 18:11:14 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1VDiWU-0008UT-O6
	for qemu-devel@nongnu.org; Sun, 25 Aug 2013 18:11:08 -0400
Message-ID: <1377468637.3819.27.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Mon, 26 Aug 2013 08:10:37 +1000
In-Reply-To: <24C2B209-2082-4AF8-A8FB-1FF8A8B7751B@suse.de>
References: <1377249737-12570-1-git-send-email-aik@ozlabs.ru>
	<24C2B209-2082-4AF8-A8FB-1FF8A8B7751B@suse.de>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities
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: Alexander Graf <agraf@suse.de>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>, Paolo Bonzini <pbonzini@redhat.com>, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

On Sun, 2013-08-25 at 17:41 +0100, Alexander Graf wrote:
> > +    vcap = &req->iu.mad.capabilities;
> > +    rc = spapr_vio_dma_read(&s->vdev, be64_to_cpu(vcap->buffer),
> > +                            &cap,
> be16_to_cpu(vcap->common.length));
> 
> While I don't think any harm could happen from it, this could lead to
> a potential timing attack where we read and write from different
> locations in memory if the guest swizzles the request while we're
> processing it.

BTW. While I disagree with your initial comment ... is there any bound
checking here ? That looks like potential stack corruption unless I
miss something if the guest passes a too big length...

So at least the length should be read once, bound-checked, then the read
done with the result (don't bound check and read again, that would be
indeed racy).

Cheers,
Ben.