Re: [PATCH 2/7] tpm: reorganize sysfs files - Updated patch

[Andrew Morton <akpm@osdl.org>]

Kylene Jo Hall <kjhall@us.ibm.com> wrote:
>
>  ssize_t tpm_show_pcrs(struct device *dev, struct device_attribute *attr,
>   		      char *buf)
>   {
>  -	u8 data[READ_PCR_RESULT_SIZE];
>  -	ssize_t len;
>  +	u8 data[30];
>  +	ssize_t rc;
>   	int i, j, num_pcrs;
>   	__be32 index;
>   	char *str = buf;
>  @@ -150,29 +190,24 @@ ssize_t tpm_show_pcrs(struct device *dev
>   	if (chip == NULL)
>   		return -ENODEV;
>   
>  -	memcpy(data, cap_pcr, sizeof(cap_pcr));
>  -	if ((len = tpm_transmit(chip, data, sizeof(data)))
>  -	    < CAP_PCR_RESULT_SIZE) {
>  -		dev_dbg(chip->dev, "A TPM error (%d) occurred "
>  -				"attempting to determine the number of PCRS\n",
>  -			be32_to_cpu(*((__be32 *) (data + 6))));
>  +	memcpy(data, tpm_cap, sizeof(tpm_cap));

I'd be a bit worried about potential for array overruns here.  If someone
later were to increase the size of tpm_cap[] we'll silently overrun data[].

One approach would be to do:

--- devel/drivers/char/tpm/tpm.c~tpm-reorganize-sysfs-files-fix	2006-04-10 14:43:16.000000000 -0700
+++ devel-akpm/drivers/char/tpm/tpm.c	2006-04-10 14:45:19.000000000 -0700
@@ -180,7 +180,7 @@ static const u8 pcrread[] = {
 ssize_t tpm_show_pcrs(struct device *dev, struct device_attribute *attr,
 		      char *buf)
 {
-	u8 data[30];
+	u8 data[ARRAY_SIZE(tpm_cap)];
 	ssize_t rc;
 	int i, j, num_pcrs;
 	__be32 index;
@@ -296,7 +296,7 @@ static const u8 cap_version[] = {
 ssize_t tpm_show_caps(struct device *dev, struct device_attribute *attr,
 		      char *buf)
 {
-	u8 data[30];
+	u8 data[max(ARRAY_SIZE(tpm_cap), ARRAY_SIZE(cap_version))];
 	ssize_t rc;
 	char *str = buf;
 
_


Does that look OK?