Messages in this thread | | | Date | Fri, 27 Jul 2012 11:24:57 -0700 | From | "H. Peter Anvin" <> | Subject | Re: [GIT PULL] New TPM driver, hwrng driver and fixes |
| |
On 07/27/2012 11:14 AM, Kent Yoder wrote: > Hi James, > > Please pull from this new branch and ignore the 7-25-12 branch. This > new branch includes fixes for comments by hpa. I've also included one > additional patch from [1] to close a race and prevent possibly sensitive > data from being free'd before being zeroed. I'm attaching this entire > diff here since my fixes for hpa's comments aren't public yet. > > Thanks, > Kent
> + > +/** > + * tpm_get_random() - Get random bytes from the tpm's RNG > + * @chip_num: A specific chip number for the request or TPM_ANY_NUM > + * @out: destination buffer for the random bytes > + * @max: on input, the max number of bytes to write to @out, on output > + * this is set to the actual number of bytes written to @out > + * > + * Note that @max will be capped at TPM_MAX_RNG_DATA bytes. > + */ > +int tpm_get_random(u32 chip_num, u8 *out, size_t *max) > +{ > + struct tpm_chip *chip; > + struct tpm_cmd_t tpm_cmd; > + u32 recd, total = 0, num_bytes = min_t(u32, *max, TPM_MAX_RNG_DATA); > + int err, retries = 5; > + u8 *dest = out; > + > + chip = tpm_chip_find_get(chip_num); > + if (chip == NULL) > + return -ENODEV; > + > + if (!out || !num_bytes || *max > TPM_MAX_RN > + return -EINVAL; > + > + do { > + tpm_cmd.header.in = tpm_getrandom_header; > + tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes); > + > + err = transmit_cmd(chip, &tpm_cmd, > + TPM_GETRANDOM_RESULT_SIZE + num_bytes, > + "attempting get random"); > + if (err) > + goto out_err; > + > + recd = be32_to_cpu(tpm_cmd.params.getrandom_out.rng_data_len); > + memcpy(dest, tpm_cmd.params.getrandom_out.rng_data, recd); > + > + dest += recd; > + total += recd; > + num_bytes -= recd; > + } while (retries-- && total < *max); > + > + err = ((total < *max) ? -EAGAIN : 0); > +out_err: > + return err; > +} > +EXPORT_SYMBOL_GPL(tpm_get_random); > +
Since you no longer modify *max anywhere in this function, why leave it a pointer? Making it pass by value seems more logical at that point (and cleaner).
The only consumer which can make use of partial result is tpm_rng_read(), but that will now return zero unless the buffer is filled.
My suggestion would be to drop the pointer and instead return a positive result (number of bytes) if you read anything and a negative result (-errno) on error.
That way, a caller which can only use an exact number of bytes should do:
rv = tpm_get_random(chip, buf, size); if (rv != size) /* error! */
... and tpm_rng_read() can do something smarter.
-hpa
| |