Re: [RFC][PATCH] RANDOM: ATH9K RNG delivers zero bits of entropy

[Jason Cooper]

Hi Stephan,

On Sat, Aug 06, 2016 at 10:03:58PM +0200, Stephan Mueller wrote:
> Am Samstag, 6. August 2016, 19:45:51 CEST schrieb Jason Cooper:
> > On Fri, Aug 05, 2016 at 05:08:14PM +0200, Stephan Mueller wrote:
...
> > > diff --git a/drivers/net/wireless/ath/ath9k/rng.c
> > > b/drivers/net/wireless/ath/ath9k/rng.c index d38e50f..d63dc48 100644
> > > --- a/drivers/net/wireless/ath/ath9k/rng.c
> > > +++ b/drivers/net/wireless/ath/ath9k/rng.c
> > > @@ -92,8 +92,7 @@ static int ath9k_rng_kthread(void *data)
> > > 
> > >   fail_stats = 0;
> > >   
> > >   /* sleep until entropy bits under write_wakeup_threshold */
> > > 
> > > - add_hwgenerator_randomness((void *)rng_buf, bytes_read,
> > > -ATH9K_RNG_ENTROPY(bytes_read));
> > 
> > This is the only use of this macro.  I'd remove the #define on line 25
> > as well.
> 
> My idea for leaving it was that folks who would bring the RNG into the 
> hwrandom framework could reuse the ideas from the original authors.
> 
> What about commenting it out with #if 0 ?

#if 0 is frowned upon.  If that calculation is documented somewhere,
then it can be redone from the spec.  If it isn't, then I'd be curious
to know where it came from.

Perhaps one of the ath9k devs can point to a document containing the
formula?  We could put the reference in a comment.

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH] RANDOM: ATH9K RNG delivers zero bits of entropy

[Stephan Mueller]

Am Samstag, 6. August 2016, 19:45:51 CEST schrieb Jason Cooper:

Hi Jason,

> Hi Stephan,
> 
> On Fri, Aug 05, 2016 at 05:08:14PM +0200, Stephan Mueller wrote:
> > Hi Ted, Herbert,
> > 
> > I sent a question to the ATH9K RNG some time ago to the developers.
> > See
> > https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg19115.html
> > 
> > I have not yet received a word and I think this issue should be resolved.
> > 
> > Thanks
> > Stephan
> > 
> > ---8<---
> 
> If the above text is placed below the three dashes, "---", below ...
> 
> > The ATH9K driver implements an RNG which is completely bypassing the
> > standard Linux HW generator logic.
> > 
> > The RNG may or may not deliver entropy. Considering the conservative
> > approach in treating entropy with respect to non-auditable sources, this
> > patch changes the delivered entropy value to zero. The RNG still feeds
> > data into the input_pool but it is assumed to have no entropy.
> > 
> > When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
> > the entropy estimation considering that a user can change that value at
> > boot and runtime.
> > 
> > Signed-off-by: Stephan Mueller 
> > ---
> 
> here, then the mail can be applied directly without editing.

Thank you for the hint. I will resend the patch that can be applied.
> 
> >  drivers/net/wireless/ath/ath9k/rng.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/ath9k/rng.c
> > b/drivers/net/wireless/ath/ath9k/rng.c index d38e50f..d63dc48 100644
> > --- a/drivers/net/wireless/ath/ath9k/rng.c
> > +++ b/drivers/net/wireless/ath/ath9k/rng.c
> > @@ -92,8 +92,7 @@ static int ath9k_rng_kthread(void *data)
> > 
> > fail_stats = 0;
> > 
> > /* sleep until entropy bits under write_wakeup_threshold */
> > 
> > -   add_hwgenerator_randomness((void *)rng_buf, bytes_read,
> > -  ATH9K_RNG_ENTROPY(bytes_read));
> 
> This is the only use of this macro.  I'd remove the #define on line 25
> as well.

My idea for leaving it was that folks who would bring the RNG into the 
hwrandom framework could reuse the ideas from the original authors.

What about commenting it out with #if 0 ?
> 
> > +   add_hwgenerator_randomness((void *)rng_buf, bytes_read, 0);
> > 
> > }
> > 
> > kfree(rng_buf);
> 
> Other than that,
> 
> Reviewed-by: Jason Cooper 

Thank you.
> 
> thx,
> 
> Jason.



-- 
Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH] RANDOM: ATH9K RNG delivers zero bits of entropy

[Jason Cooper]

Hi Stephan,

On Fri, Aug 05, 2016 at 05:08:14PM +0200, Stephan Mueller wrote:
> Hi Ted, Herbert,
> 
> I sent a question to the ATH9K RNG some time ago to the developers.
> See https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg19115.html
> 
> I have not yet received a word and I think this issue should be resolved.
> 
> Thanks
> Stephan
> 
> ---8<---

If the above text is placed below the three dashes, "---", below ...

> The ATH9K driver implements an RNG which is completely bypassing the
> standard Linux HW generator logic.
> 
> The RNG may or may not deliver entropy. Considering the conservative
> approach in treating entropy with respect to non-auditable sources, this
> patch changes the delivered entropy value to zero. The RNG still feeds
> data into the input_pool but it is assumed to have no entropy.
> 
> When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
> the entropy estimation considering that a user can change that value at
> boot and runtime.
> 
> Signed-off-by: Stephan Mueller 
> ---

here, then the mail can be applied directly without editing.

>  drivers/net/wireless/ath/ath9k/rng.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/rng.c 
> b/drivers/net/wireless/ath/ath9k/rng.c
> index d38e50f..d63dc48 100644
> --- a/drivers/net/wireless/ath/ath9k/rng.c
> +++ b/drivers/net/wireless/ath/ath9k/rng.c
> @@ -92,8 +92,7 @@ static int ath9k_rng_kthread(void *data)
>   fail_stats = 0;
>  
>   /* sleep until entropy bits under write_wakeup_threshold */
> - add_hwgenerator_randomness((void *)rng_buf, bytes_read,
> -ATH9K_RNG_ENTROPY(bytes_read));

This is the only use of this macro.  I'd remove the #define on line 25
as well.

> + add_hwgenerator_randomness((void *)rng_buf, bytes_read, 0);
>   }
>  
>   kfree(rng_buf);

Other than that,

Reviewed-by: Jason Cooper 

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html