All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] omap-rng: Change RNG_CONFIG_REG to RNG_CONTROL_REG when, checking and disabling TRNG
@ 2015-02-20  4:37 ` Andre Wolokita
  0 siblings, 0 replies; 5+ messages in thread
From: Andre Wolokita @ 2015-02-20  4:37 UTC (permalink / raw)
  To: lokeshvutla, dsaxena; +Cc: linux-omap, linux-crypto

In omap4_rng_init(), a check of bit 10 of the RNG_CONFIG_REG is done to determine
whether the RNG is running. This is suspicious firstly due to the use of
RNG_CONTROL_ENABLE_TRNG_MASK and secondly because the same mask is written to
RNG_CONTROL_REG after configuration of the FROs. Similar suspicious logic is
repeated in omap4_rng_cleanup() when RNG_CONTROL_REG masked with
RNG_CONTROL_ENABLE_TRNG_MASK is read, the same mask bit is cleared, and then
written to RNG_CONFIG_REG. Unless the TRNG is enabled with one bit in RNG_CONTROL
and disabled with another in RNG_CONFIG and these bits are mirrored in some way,
I believe that the TRNG is not really shutting off, leaving the FROs running
indefinitely which, in an extreme case, could lead to degredation of the
hardware and potentially reduce the level of entropy generated.

Apart from the strange logic, I have reason to suspect that the OMAP4 related
code in this driver is driving an Inside Secure IP hardware RNG and strongly
suspect that bit 10 of RNG_CONFIG_REG is one of the bits configuring the
sampling rate of the FROs. This option is by default set to 0 and is not being
set anywhere in omap-rng.c. Reading this bit during omap4_rng_init() will
always return 0. It will remain 0 because ~(value of TRNG_MASK in control) will
always be 0, because the TRNG is never shut off. This is of course presuming
that the OMAP4 features the Inside Secure IP, which I strongly suspect.

I'm interested in knowing what the guys at TI think about this, as only they
can confirm or deny the detailed structure of these registers.

Signed-off-by: Andre Wolokita <Andre.Wolokita@analog.com>
---
 drivers/char/hw_random/omap-rng.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index d14dcf7..67151cb 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -236,7 +236,7 @@ static int omap4_rng_init(struct omap_rng_dev *priv)
        u32 val;

        /* Return if RNG is already running. */
-       if (omap_rng_read(priv, RNG_CONFIG_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
+       if (omap_rng_read(priv, RNG_CONTROL_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
                return 0;

        val = RNG_CONFIG_MIN_REFIL_CYCLES << RNG_CONFIG_MIN_REFIL_CYCLES_SHIFT;
@@ -262,7 +262,7 @@ static void omap4_rng_cleanup(struct omap_rng_dev *priv)

        val = omap_rng_read(priv, RNG_CONTROL_REG);
        val &= ~RNG_CONTROL_ENABLE_TRNG_MASK;
-       omap_rng_write(priv, RNG_CONFIG_REG, val);
+       omap_rng_write(priv, RNG_CONTROL_REG, val);
 }

 static irqreturn_t omap4_rng_irq(int irq, void *dev_id)
-- 
1.7.9.5

-- 
Andre Wolokita (andre.wolokita@analog.com)
Design Engineer, Analog Devices Australia Pty Ltd
Unit 3, 97 Lewis Road, Wantirna, Victoria, 3152, AUSTRALIA
Direct: +61 3 9881 9933   Main: +61 3 9881 9999
Fax: +61 3 9881 9988      Web: www.analog.com/au

Embedded DSP software for multimedia & telecommunications.

This communication is proprietary and confidential. 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3] omap-rng: Change RNG_CONFIG_REG to RNG_CONTROL_REG when, checking and disabling TRNG
@ 2015-02-20  4:37 ` Andre Wolokita
  0 siblings, 0 replies; 5+ messages in thread
From: Andre Wolokita @ 2015-02-20  4:37 UTC (permalink / raw)
  To: lokeshvutla, dsaxena; +Cc: linux-omap, linux-crypto

In omap4_rng_init(), a check of bit 10 of the RNG_CONFIG_REG is done to determine
whether the RNG is running. This is suspicious firstly due to the use of
RNG_CONTROL_ENABLE_TRNG_MASK and secondly because the same mask is written to
RNG_CONTROL_REG after configuration of the FROs. Similar suspicious logic is
repeated in omap4_rng_cleanup() when RNG_CONTROL_REG masked with
RNG_CONTROL_ENABLE_TRNG_MASK is read, the same mask bit is cleared, and then
written to RNG_CONFIG_REG. Unless the TRNG is enabled with one bit in RNG_CONTROL
and disabled with another in RNG_CONFIG and these bits are mirrored in some way,
I believe that the TRNG is not really shutting off, leaving the FROs running
indefinitely which, in an extreme case, could lead to degredation of the
hardware and potentially reduce the level of entropy generated.

Apart from the strange logic, I have reason to suspect that the OMAP4 related
code in this driver is driving an Inside Secure IP hardware RNG and strongly
suspect that bit 10 of RNG_CONFIG_REG is one of the bits configuring the
sampling rate of the FROs. This option is by default set to 0 and is not being
set anywhere in omap-rng.c. Reading this bit during omap4_rng_init() will
always return 0. It will remain 0 because ~(value of TRNG_MASK in control) will
always be 0, because the TRNG is never shut off. This is of course presuming
that the OMAP4 features the Inside Secure IP, which I strongly suspect.

I'm interested in knowing what the guys at TI think about this, as only they
can confirm or deny the detailed structure of these registers.

Signed-off-by: Andre Wolokita <Andre.Wolokita@analog.com>
---
 drivers/char/hw_random/omap-rng.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index d14dcf7..67151cb 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -236,7 +236,7 @@ static int omap4_rng_init(struct omap_rng_dev *priv)
        u32 val;

        /* Return if RNG is already running. */
-       if (omap_rng_read(priv, RNG_CONFIG_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
+       if (omap_rng_read(priv, RNG_CONTROL_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
                return 0;

        val = RNG_CONFIG_MIN_REFIL_CYCLES << RNG_CONFIG_MIN_REFIL_CYCLES_SHIFT;
@@ -262,7 +262,7 @@ static void omap4_rng_cleanup(struct omap_rng_dev *priv)

        val = omap_rng_read(priv, RNG_CONTROL_REG);
        val &= ~RNG_CONTROL_ENABLE_TRNG_MASK;
-       omap_rng_write(priv, RNG_CONFIG_REG, val);
+       omap_rng_write(priv, RNG_CONTROL_REG, val);
 }

 static irqreturn_t omap4_rng_irq(int irq, void *dev_id)
-- 
1.7.9.5

-- 
Andre Wolokita (andre.wolokita@analog.com)
Design Engineer, Analog Devices Australia Pty Ltd
Unit 3, 97 Lewis Road, Wantirna, Victoria, 3152, AUSTRALIA
Direct: +61 3 9881 9933   Main: +61 3 9881 9999
Fax: +61 3 9881 9988      Web: www.analog.com/au

Embedded DSP software for multimedia & telecommunications.

This communication is proprietary and confidential. 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [v3] hwrng: omap - Change RNG_CONFIG_REG to RNG_CONTROL_REG when, checking and disabling TRNG
  2015-02-20  4:37 ` Andre Wolokita
  (?)
@ 2015-03-01 10:02 ` Herbert Xu
  -1 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2015-03-01 10:02 UTC (permalink / raw)
  To: Andre Wolokita; +Cc: lokeshvutla, dsaxena, linux-omap, linux-crypto

On Fri, Feb 20, 2015 at 03:37:21PM +1100, Andre Wolokita wrote:
> In omap4_rng_init(), a check of bit 10 of the RNG_CONFIG_REG is done to determine
> whether the RNG is running. This is suspicious firstly due to the use of
> RNG_CONTROL_ENABLE_TRNG_MASK and secondly because the same mask is written to
> RNG_CONTROL_REG after configuration of the FROs. Similar suspicious logic is
> repeated in omap4_rng_cleanup() when RNG_CONTROL_REG masked with
> RNG_CONTROL_ENABLE_TRNG_MASK is read, the same mask bit is cleared, and then
> written to RNG_CONFIG_REG. Unless the TRNG is enabled with one bit in RNG_CONTROL
> and disabled with another in RNG_CONFIG and these bits are mirrored in some way,
> I believe that the TRNG is not really shutting off, leaving the FROs running
> indefinitely which, in an extreme case, could lead to degredation of the
> hardware and potentially reduce the level of entropy generated.
> 
> Apart from the strange logic, I have reason to suspect that the OMAP4 related
> code in this driver is driving an Inside Secure IP hardware RNG and strongly
> suspect that bit 10 of RNG_CONFIG_REG is one of the bits configuring the
> sampling rate of the FROs. This option is by default set to 0 and is not being
> set anywhere in omap-rng.c. Reading this bit during omap4_rng_init() will
> always return 0. It will remain 0 because ~(value of TRNG_MASK in control) will
> always be 0, because the TRNG is never shut off. This is of course presuming
> that the OMAP4 features the Inside Secure IP, which I strongly suspect.
> 
> I'm interested in knowing what the guys at TI think about this, as only they
> can confirm or deny the detailed structure of these registers.
> 
> Signed-off-by: Andre Wolokita <Andre.Wolokita@analog.com>

Your patch is white-space damaged.  Please fix and resubmit.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] omap-rng: Change RNG_CONFIG_REG to RNG_CONTROL_REG when, checking and disabling TRNG
  2015-02-20  4:37 ` Andre Wolokita
@ 2015-03-23  5:12   ` Lokesh Vutla
  -1 siblings, 0 replies; 5+ messages in thread
From: Lokesh Vutla @ 2015-03-23  5:12 UTC (permalink / raw)
  To: Andre Wolokita, dsaxena; +Cc: linux-omap, linux-crypto

Hi Andre,
On Friday 20 February 2015 10:07 AM, Andre Wolokita wrote:
> In omap4_rng_init(), a check of bit 10 of the RNG_CONFIG_REG is done to determine
> whether the RNG is running. This is suspicious firstly due to the use of
> RNG_CONTROL_ENABLE_TRNG_MASK and secondly because the same mask is written to
> RNG_CONTROL_REG after configuration of the FROs. Similar suspicious logic is
> repeated in omap4_rng_cleanup() when RNG_CONTROL_REG masked with
> RNG_CONTROL_ENABLE_TRNG_MASK is read, the same mask bit is cleared, and then
> written to RNG_CONFIG_REG. Unless the TRNG is enabled with one bit in RNG_CONTROL
> and disabled with another in RNG_CONFIG and these bits are mirrored in some way,
> I believe that the TRNG is not really shutting off, leaving the FROs running
> indefinitely which, in an extreme case, could lead to degredation of the
> hardware and potentially reduce the level of entropy generated.
Yes you are correct. Patch looks good to me.
It should be RNG_CONTROL_REG

Acked-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh

> 
> Apart from the strange logic, I have reason to suspect that the OMAP4 related
> code in this driver is driving an Inside Secure IP hardware RNG and strongly
> suspect that bit 10 of RNG_CONFIG_REG is one of the bits configuring the
> sampling rate of the FROs. This option is by default set to 0 and is not being
> set anywhere in omap-rng.c. Reading this bit during omap4_rng_init() will
> always return 0. It will remain 0 because ~(value of TRNG_MASK in control) will
> always be 0, because the TRNG is never shut off. This is of course presuming
> that the OMAP4 features the Inside Secure IP, which I strongly suspect.
> 
> I'm interested in knowing what the guys at TI think about this, as only they
> can confirm or deny the detailed structure of these registers.
> 
> Signed-off-by: Andre Wolokita <Andre.Wolokita@analog.com>
> ---
>  drivers/char/hw_random/omap-rng.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
> index d14dcf7..67151cb 100644
> --- a/drivers/char/hw_random/omap-rng.c
> +++ b/drivers/char/hw_random/omap-rng.c
> @@ -236,7 +236,7 @@ static int omap4_rng_init(struct omap_rng_dev *priv)
>         u32 val;
> 
>         /* Return if RNG is already running. */
> -       if (omap_rng_read(priv, RNG_CONFIG_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
> +       if (omap_rng_read(priv, RNG_CONTROL_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
>                 return 0;
> 
>         val = RNG_CONFIG_MIN_REFIL_CYCLES << RNG_CONFIG_MIN_REFIL_CYCLES_SHIFT;
> @@ -262,7 +262,7 @@ static void omap4_rng_cleanup(struct omap_rng_dev *priv)
> 
>         val = omap_rng_read(priv, RNG_CONTROL_REG);
>         val &= ~RNG_CONTROL_ENABLE_TRNG_MASK;
> -       omap_rng_write(priv, RNG_CONFIG_REG, val);
> +       omap_rng_write(priv, RNG_CONTROL_REG, val);
>  }
> 
>  static irqreturn_t omap4_rng_irq(int irq, void *dev_id)
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] omap-rng: Change RNG_CONFIG_REG to RNG_CONTROL_REG when, checking and disabling TRNG
@ 2015-03-23  5:12   ` Lokesh Vutla
  0 siblings, 0 replies; 5+ messages in thread
From: Lokesh Vutla @ 2015-03-23  5:12 UTC (permalink / raw)
  To: Andre Wolokita, dsaxena; +Cc: linux-omap, linux-crypto

Hi Andre,
On Friday 20 February 2015 10:07 AM, Andre Wolokita wrote:
> In omap4_rng_init(), a check of bit 10 of the RNG_CONFIG_REG is done to determine
> whether the RNG is running. This is suspicious firstly due to the use of
> RNG_CONTROL_ENABLE_TRNG_MASK and secondly because the same mask is written to
> RNG_CONTROL_REG after configuration of the FROs. Similar suspicious logic is
> repeated in omap4_rng_cleanup() when RNG_CONTROL_REG masked with
> RNG_CONTROL_ENABLE_TRNG_MASK is read, the same mask bit is cleared, and then
> written to RNG_CONFIG_REG. Unless the TRNG is enabled with one bit in RNG_CONTROL
> and disabled with another in RNG_CONFIG and these bits are mirrored in some way,
> I believe that the TRNG is not really shutting off, leaving the FROs running
> indefinitely which, in an extreme case, could lead to degredation of the
> hardware and potentially reduce the level of entropy generated.
Yes you are correct. Patch looks good to me.
It should be RNG_CONTROL_REG

Acked-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh

> 
> Apart from the strange logic, I have reason to suspect that the OMAP4 related
> code in this driver is driving an Inside Secure IP hardware RNG and strongly
> suspect that bit 10 of RNG_CONFIG_REG is one of the bits configuring the
> sampling rate of the FROs. This option is by default set to 0 and is not being
> set anywhere in omap-rng.c. Reading this bit during omap4_rng_init() will
> always return 0. It will remain 0 because ~(value of TRNG_MASK in control) will
> always be 0, because the TRNG is never shut off. This is of course presuming
> that the OMAP4 features the Inside Secure IP, which I strongly suspect.
> 
> I'm interested in knowing what the guys at TI think about this, as only they
> can confirm or deny the detailed structure of these registers.
> 
> Signed-off-by: Andre Wolokita <Andre.Wolokita@analog.com>
> ---
>  drivers/char/hw_random/omap-rng.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
> index d14dcf7..67151cb 100644
> --- a/drivers/char/hw_random/omap-rng.c
> +++ b/drivers/char/hw_random/omap-rng.c
> @@ -236,7 +236,7 @@ static int omap4_rng_init(struct omap_rng_dev *priv)
>         u32 val;
> 
>         /* Return if RNG is already running. */
> -       if (omap_rng_read(priv, RNG_CONFIG_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
> +       if (omap_rng_read(priv, RNG_CONTROL_REG) & RNG_CONTROL_ENABLE_TRNG_MASK)
>                 return 0;
> 
>         val = RNG_CONFIG_MIN_REFIL_CYCLES << RNG_CONFIG_MIN_REFIL_CYCLES_SHIFT;
> @@ -262,7 +262,7 @@ static void omap4_rng_cleanup(struct omap_rng_dev *priv)
> 
>         val = omap_rng_read(priv, RNG_CONTROL_REG);
>         val &= ~RNG_CONTROL_ENABLE_TRNG_MASK;
> -       omap_rng_write(priv, RNG_CONFIG_REG, val);
> +       omap_rng_write(priv, RNG_CONTROL_REG, val);
>  }
> 
>  static irqreturn_t omap4_rng_irq(int irq, void *dev_id)
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-03-23  5:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-20  4:37 [PATCH v3] omap-rng: Change RNG_CONFIG_REG to RNG_CONTROL_REG when, checking and disabling TRNG Andre Wolokita
2015-02-20  4:37 ` Andre Wolokita
2015-03-01 10:02 ` [v3] hwrng: omap - " Herbert Xu
2015-03-23  5:12 ` [PATCH v3] omap-rng: " Lokesh Vutla
2015-03-23  5:12   ` Lokesh Vutla

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.