Re: [PATCH] hwrng: do not warn when there are no devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20 June 2017 at 00:33, Mike Frysinger <vapier@xxxxxxxxxxxx> wrote:
> On Mon, Jun 19, 2017 at 2:43 AM, PrasannaKumar Muralidharan wrote:
>> On 19 June 2017 at 11:51, Herbert Xu wrote:
>>> On Sun, Jun 18, 2017 at 10:00:17PM -0700, Mike Frysinger wrote:
>>>>
>>>> in order to make tpm-rng react in the way you're implying, the TPM
>>>> subsystem would need to add a notification chain for transitions from
>>>> none<->some devices, then tpm-rng could subscribe to that, and during
>>>> those transition points, it would call hwrng_register/hwrng_unregister
>>>> to make itself visible accordingly to the hwrng subsystem.  maybe
>>>> someone on the TPM side would be interested in writing all that logic,
>>>> but it sounds excessive for this minor usage.  the current tpm-rng
>>>> driver is *extremely* simple -- it's 3 funcs, each of which are 1
>>>> line.
>>>
>>> It's simple and it's broken, as far as the way it hooks into the
>>> hwrng is concerned.
>>
>> *********************************************************************************************
>> diff --git a/drivers/char/hw_random/tpm-rng.c b/drivers/char/hw_random/tpm-rng.c
>> index d6d4482..4861b35 100644
>> --- a/drivers/char/hw_random/tpm-rng.c
>> +++ b/drivers/char/hw_random/tpm-rng.c
>> @@ -22,6 +22,10 @@
>>  #include <linux/tpm.h>
>>
>>  #define MODULE_NAME "tpm-rng"
>> +#define MAX_RETRIES 30
>> +
>> +static struct delayed_work check_tpm_work;
>> +static int retry_count;
>>
>>  static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>>  {
>> @@ -33,9 +37,27 @@ static struct hwrng tpm_rng = {
>>         .read = tpm_rng_read,
>>  };
>>
>> +static void check_tpm_presence(struct work_struct *work)
>> +{
>> +       u8 data = 0;
>> +       if (tpm_get_random(TPM_ANY_NUM, &data, 1) > 0) {
>> +               hwrng_register(&tpm_rng);
>> +       } else {
>> +               if (retry_count < MAX_RETRIES) {
>> +                       retry_count++;
>> +                       schedule_delayed_work(&check_tpm_work, HZ * 10);
>> +               } else {
>> +                       pr_err("Could not find any TPM chip, not
>> registering rng");
>> +               }
>> +       }
>> +}
>> +
>>  static int __init rng_init(void)
>>  {
>> -       return hwrng_register(&tpm_rng);
>> +       INIT_DELAYED_WORK(&check_tpm_work, check_tpm_presence);
>> +       check_tpm_presence(NULL);
>> +
>> +       return 0;
>>  }
>>  module_init(rng_init);
>> *********************************************************************************************
>>
>> Why not something like this? Patch is completely untested. If this
>> idea seems useful I can clean the code but would require help in
>> testing.
>
> first, that's not how deferred device probing works in the kernel.
> drivers shouldn't be doing their own sleeping.  but we can ignore that
> because no amount of delay/retries will work -- TPMs can come & go at
> anytime via hotplugging or module loading/unloading.  so the only way
> to pull it off would be to do something like what i described --
> extending the tpm framework so that it can signal children to come
> up/go down.

If TPM can come and go then notification or callback is the correct
way to handle this case.

> imo, standing all of that up is over-engineering and not worth the
> effort, so i'm not going to do it.  but maybe you can convince some of
> the TPM maintainers it's worthwhile.

Okay.

Thanks,
PrasannaKumar



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux