>> + /* err can be positive if it came from the TPM itself, >> + * so return a negative value here instead. */ >> + err = -EFAULT; > > -EFAULT is definitely wrong (that means a bad pointer was passed), you > can use -EIO instead. > > However, I would suggest: > > err = total ? total : -EIO;
This is fine w/ me... > ... so you report the number of bytes successfully received if we got > any. However, since you *also* do that on the retry line, > >> + 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; > > Should we return something other than 0 if we run out of retries here, too? Ugh, I was hoping to avoid this kind of complexity. > Perhaps we should just do the same "err = total ? total : -EIO;" here > and the above statement can just turn into a break;. Yeah, this seems like the right thing to do. >> - ret = my_get_random(hash, SHA1_DIGEST_SIZE); >> + ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE); >> if (ret < 0) >> return ret; > > You are still not checking the return values correctly! Dave, can you weigh in on these individual cases? In some cases like capping a pcr I think using uninitialized stack data could be better than failing... Kent > This needs to be something like: > > ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE); > if (ret != SHA1_DIGEST_SIZE) > return -EIO; /* Or whatever is appropriate here */ > > > -hpa > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- IBM LTC Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
