linux-kernel - Re: [PATCH v4] drivers/tpm: add xen tpmfront interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51ED4D4A.3060705@citrix.com>
Date:	Mon, 22 Jul 2013 16:18:34 +0100
From:	David Vrabel <david.vrabel@...rix.com>
To:	Daniel De Graaf <dgdegra@...ho.nsa.gov>
CC:	<konrad.wilk@...cle.com>, <tpmdd-devel@...ts.sourceforge.net>,
	<linux-kernel@...r.kernel.org>, <leosilva@...ux.vnet.ibm.com>,
	<shpedoikal@...il.com>, <xen-devel@...ts.xen.org>,
	<mail@...jiv.net>, <adlai@...ux.vnet.ibm.com>, <tpmdd@...rix.com>,
	<PeterHuewe@....de>
Subject: Re: [PATCH v4] drivers/tpm: add xen tpmfront interface

On 01/07/13 22:34, Daniel De Graaf wrote:
> This is a complete rewrite of the Xen TPM frontend driver, taking
> advantage of a simplified frontend/backend interface and adding support
> for cancellation and timeouts.  The backend for this driver is provided
> by a vTPM stub domain using the interface in Xen 4.3.
[...]
> --- /dev/null
> +++ b/Documentation/xen-tpmfront.txt

Suggest putting this in Documentation/tpm/.

> --- /dev/null
> +++ b/drivers/char/tpm/xen-tpmfront.c
[...]
> +static void backend_changed(struct xenbus_device *dev,
> +		enum xenbus_state backend_state)
> +{
> +	int val;

Hrm. I don't like how every front/back pair invents their own variation
of the state machine.

Please document the front and back state machines in
xen/include/public/io/tpmif.h (and the correspoding copy in Linux).

> +
> +	switch (backend_state) {
> +	case XenbusStateInitialised:
> +	case XenbusStateConnected:

		if (dev->state == XenbusStateConnected)
			break;

Perhaps?

> +		if (xenbus_scanf(XBT_NIL, dev->otherend,
> +				"feature-protocol-v2", "%d", &val) < 0)
> +			val = 0;
> +		if (!val) {
> +			xenbus_dev_fatal(dev, -EINVAL,
> +					"vTPM protocol 2 required");
> +			return;
> +		}
> +		xenbus_switch_state(dev, XenbusStateConnected);
> +		break;
> +
> +	case XenbusStateClosing:
> +	case XenbusStateClosed:
> +		device_unregister(&dev->dev);
> +		xenbus_frontend_closed(dev);
> +		break;
> +	default:
> +		break;
> +	}
> +}

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/