From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752784Ab2G0S02 (ORCPT ); Fri, 27 Jul 2012 14:26:28 -0400 Received: from terminus.zytor.com ([198.137.202.10]:50905 "EHLO mail.zytor.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752426Ab2G0S01 (ORCPT ); Fri, 27 Jul 2012 14:26:27 -0400 Message-ID: <5012DCF9.7010408@zytor.com> Date: Fri, 27 Jul 2012 11:24:57 -0700 From: "H. Peter Anvin" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 MIME-Version: 1.0 To: Kent Yoder CC: James Morris , linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, tpmdd-devel@lists.sourceforge.net, Peter Huewe , Bryan Freed Subject: Re: [GIT PULL] New TPM driver, hwrng driver and fixes References: <20120727181436.GA6271@linux.vnet.ibm.com> In-Reply-To: <20120727181436.GA6271@linux.vnet.ibm.com> X-Enigmail-Version: 1.4.3 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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