From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1LKZxV-00084Y-4R
	for qemu-devel@nongnu.org; Wed, 07 Jan 2009 10:04:37 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1LKZxU-00084M-2g
	for qemu-devel@nongnu.org; Wed, 07 Jan 2009 10:04:36 -0500
Received: from [199.232.76.173] (port=44211 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1LKZxT-00084J-W7
	for qemu-devel@nongnu.org; Wed, 07 Jan 2009 10:04:36 -0500
Received: from mx2.redhat.com ([66.187.237.31]:51600)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <markmc@redhat.com>) id 1LKZxT-0005gy-Cm
	for qemu-devel@nongnu.org; Wed, 07 Jan 2009 10:04:35 -0500
Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26])
	by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n07F4YQL008760
	for <qemu-devel@nongnu.org>; Wed, 7 Jan 2009 10:04:34 -0500
Subject: Re: [Qemu-devel] [PATCH] mark nic as trusted
From: Mark McLoughlin <markmc@redhat.com>
In-Reply-To: <20090107142626.GE3267@redhat.com>
References: <20090107142626.GE3267@redhat.com>
Content-Type: text/plain
Date: Wed, 07 Jan 2009 15:04:31 +0000
Message-Id: <1231340671.5050.69.camel@localhost.localdomain>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Reply-To: Mark McLoughlin <markmc@redhat.com>, qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gleb Natapov <gleb@redhat.com>
Cc: qemu-devel@nongnu.org

Hi Gleb,

On Wed, 2009-01-07 at 16:26 +0200, Gleb Natapov wrote:

> This patch allows to mark specific nic as trusted by adding special
> PCI capability. "Trusted" means that it is used for communication
> between host and guest and no malicious entity can inject traffic
> to the nic.

I'm not sure I follow - is this cookie a shared secret that only the
host and guest knows, or do literally mean that the cookie will contain
the string "Trusted" as a indicator that the guest can trust the NIC?

> Signed-off-by: Gleb Natapov <gleb@redhat.com>
...
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 1f45b2d..186c6bd 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -309,6 +309,9 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
>      if (!n)
>          return NULL;
>  
> +    if (nd->secure_cookie[0])
> +        pci_add_capability(&n->vdev.pci_dev, 0x0f, 0xf0, nd->secure_cookie, 14);

How was the Capability ID 0x0f chosen? It it unallocated by the PCI SIG
allocated it or ...? I see it's not defined in the kernel sources:

#define  PCI_CAP_ID_AGP3        0x0E    /* AGP Target PCI-PCI bridge */
#define  PCI_CAP_ID_EXP         0x10    /* PCI Express */

Also, to reduce magic numbers it would be nice to define the CAP_ID
(0xf) and offset (0xf0) as a macro somewhere and use
sizeof(nd->secure_cookie) instead of 14.

> diff --git a/net.c b/net.c
> index 6af4255..000768f 100644
> --- a/net.c
> +++ b/net.c
> @@ -1474,6 +1474,11 @@ int net_client_init(const char *device, const char *p)
>          if (get_param_value(buf, sizeof(buf), "model", p)) {
>              nd->model = strdup(buf);
>          }
> +        if (get_param_value(buf, sizeof(buf), "trusted", p)) {
> +            strncpy(nd->secure_cookie, buf, sizeof(nd->secure_cookie));
> +        } else {
> +            nd->secure_cookie[0] = NULL;

NULL isn't a uint8_t, use '\0' instead I guess. Or maybe just memset()
the NICInfo struct before starting to assign to it.

Cheers,
Mark.