Subject: [PATCH] crypto: exynoes-rng: Set cra_ctxsize to 0

As cra_ctxsize is set but the allocated space is not used, set it 0.

Signed-off-by: PrasannaKumar Muralidharan <[email protected]>
---
drivers/crypto/exynos-rng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c
index 451620b..d009df6 100644
--- a/drivers/crypto/exynos-rng.c
+++ b/drivers/crypto/exynos-rng.c
@@ -260,7 +260,7 @@ static int exynos_rng_kcapi_init(struct crypto_tfm *tfm)
.cra_name = "stdrng",
.cra_driver_name = "exynos_rng",
.cra_priority = 100,
- .cra_ctxsize = sizeof(struct exynos_rng_ctx),
+ .cra_ctxsize = 0,
.cra_module = THIS_MODULE,
.cra_init = exynos_rng_kcapi_init,
}
--
1.8.5.6


2017-05-21 06:26:11

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] crypto: exynoes-rng: Set cra_ctxsize to 0

On Sun, May 21, 2017 at 8:09 AM, PrasannaKumar Muralidharan
<[email protected]> wrote:
> As cra_ctxsize is set but the allocated space is not used, set it 0.

Why do you think it is not used? Did you test our change on hardware?

Best regards,
Krzysztof

>
> Signed-off-by: PrasannaKumar Muralidharan <[email protected]>
> ---
> drivers/crypto/exynos-rng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c
> index 451620b..d009df6 100644
> --- a/drivers/crypto/exynos-rng.c
> +++ b/drivers/crypto/exynos-rng.c
> @@ -260,7 +260,7 @@ static int exynos_rng_kcapi_init(struct crypto_tfm *tfm)
> .cra_name = "stdrng",
> .cra_driver_name = "exynos_rng",
> .cra_priority = 100,
> - .cra_ctxsize = sizeof(struct exynos_rng_ctx),
> + .cra_ctxsize = 0,
> .cra_module = THIS_MODULE,
> .cra_init = exynos_rng_kcapi_init,
> }
> --
> 1.8.5.6
>

Subject: Re: [PATCH] crypto: exynoes-rng: Set cra_ctxsize to 0

Hi Krzysztof

On 21 May 2017 at 11:56, Krzysztof Kozlowski <[email protected]> wrote:
> On Sun, May 21, 2017 at 8:09 AM, PrasannaKumar Muralidharan
> <[email protected]> wrote:
>> As cra_ctxsize is set but the allocated space is not used, set it 0.
>
> Why do you think it is not used? Did you test our change on hardware?

Had a look at the crypto rng code. I think the additional size is used
to store driver private data. But this driver does not store any
private data in the crypto_tfm structure so I think the 'cra_ctxsize'
can be safely set to 0.

I do not have access to the hardware, did not test the change. Sorry I
forgot to mention that.

> Best regards,
> Krzysztof
>
>>
>> Signed-off-by: PrasannaKumar Muralidharan <[email protected]>
>> ---
>> drivers/crypto/exynos-rng.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c
>> index 451620b..d009df6 100644
>> --- a/drivers/crypto/exynos-rng.c
>> +++ b/drivers/crypto/exynos-rng.c
>> @@ -260,7 +260,7 @@ static int exynos_rng_kcapi_init(struct crypto_tfm *tfm)
>> .cra_name = "stdrng",
>> .cra_driver_name = "exynos_rng",
>> .cra_priority = 100,
>> - .cra_ctxsize = sizeof(struct exynos_rng_ctx),
>> + .cra_ctxsize = 0,
>> .cra_module = THIS_MODULE,
>> .cra_init = exynos_rng_kcapi_init,
>> }
>> --
>> 1.8.5.6
>>

Regards,
PrasannaKumar

2017-05-21 07:14:14

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] crypto: exynoes-rng: Set cra_ctxsize to 0

On Sun, May 21, 2017 at 9:11 AM, PrasannaKumar Muralidharan
<[email protected]> wrote:
> Hi Krzysztof
>
> On 21 May 2017 at 11:56, Krzysztof Kozlowski <[email protected]> wrote:
>> On Sun, May 21, 2017 at 8:09 AM, PrasannaKumar Muralidharan
>> <[email protected]> wrote:
>>> As cra_ctxsize is set but the allocated space is not used, set it 0.
>>
>> Why do you think it is not used? Did you test our change on hardware?
>
> Had a look at the crypto rng code. I think the additional size is used
> to store driver private data. But this driver does not store any
> private data in the crypto_tfm structure so I think the 'cra_ctxsize'
> can be safely set to 0.

Then from where does crypto_tfm_ctx() get its memory?

> I do not have access to the hardware, did not test the change. Sorry I
> forgot to mention that.

That is quite important... By default everything must be tested so if
you are skipping this step then please mark the patch respectively so
others will provide testing.

Best regards,
Krzysztof

Subject: Re: [PATCH] crypto: exynoes-rng: Set cra_ctxsize to 0

On 21 May 2017 at 12:44, Krzysztof Kozlowski <[email protected]> wrote:
> On Sun, May 21, 2017 at 9:11 AM, PrasannaKumar Muralidharan
> <[email protected]> wrote:
>> Hi Krzysztof
>>
>> On 21 May 2017 at 11:56, Krzysztof Kozlowski <[email protected]> wrote:
>>> On Sun, May 21, 2017 at 8:09 AM, PrasannaKumar Muralidharan
>>> <[email protected]> wrote:
>>>> As cra_ctxsize is set but the allocated space is not used, set it 0.
>>>
>>> Why do you think it is not used? Did you test our change on hardware?
>>
>> Had a look at the crypto rng code. I think the additional size is used
>> to store driver private data. But this driver does not store any
>> private data in the crypto_tfm structure so I think the 'cra_ctxsize'
>> can be safely set to 0.
>
> Then from where does crypto_tfm_ctx() get its memory?

Ah, yes. Thanks for pointing out. I overlooked this. My patch is
completely wrong.

>> I do not have access to the hardware, did not test the change. Sorry I
>> forgot to mention that.
>
> That is quite important... By default everything must be tested so if
> you are skipping this step then please mark the patch respectively so
> others will provide testing.

Sure. Will keep that in mind. Instead of marking it as a [PATCH]
should I use something else for this? Will it make things easier?

Thanks,
PrasannaKumar

2017-05-22 05:56:11

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] crypto: exynoes-rng: Set cra_ctxsize to 0

On Mon, May 22, 2017 at 5:44 AM, PrasannaKumar Muralidharan
<[email protected]> wrote:
>>> I do not have access to the hardware, did not test the change. Sorry I
>>> forgot to mention that.
>>
>> That is quite important... By default everything must be tested so if
>> you are skipping this step then please mark the patch respectively so
>> others will provide testing.
>
> Sure. Will keep that in mind. Instead of marking it as a [PATCH]
> should I use something else for this? Will it make things easier?

If sending untested patch is really necessary, then mark it either as
RFT instead of PATCH or add a comment about it after --- separator.

Best regards,
Krzysztof