Messages in this thread | | | Subject | Re: [PATCH 2/7] tpm: reorganize sysfs files - Updated patch | From | Kylene Jo Hall <> | Date | Tue, 11 Apr 2006 09:24:54 -0500 |
| |
On Mon, 2006-04-10 at 14:46 -0700, Andrew Morton wrote: > 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?
No this is not ok because in several of these cases the response to the command is longer than tpm_cap thus the reason for the hardcoded size. I can put in a max function though that compares the size of the response and the tpm_cap. The read functions will make sure the response does not overflow the buffer should that length ever change in the future.
Thanks, Kylie
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |