2016-11-01 19:05:13

by Gary R Hook

[permalink] [raw]
Subject: [PATCH] crypto: ccp - Fix handling of RSA exponent on a v5 device

The exponent size in the ccp_op structure is in bits. A v5
CCP requires the exponent size to be in bytes, so convert
the size from bits to bytes when populating the descriptor.

The current code references the exponent in memory, but
these fields have not been set since the exponent is
actually store in the LSB. Populate the descriptor with
the LSB location (address).

Signed-off-by: Gary R Hook <[email protected]>
---
drivers/crypto/ccp/ccp-dev-v5.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c
index ff7816a..e2ce819 100644
--- a/drivers/crypto/ccp/ccp-dev-v5.c
+++ b/drivers/crypto/ccp/ccp-dev-v5.c
@@ -403,7 +403,7 @@ static int ccp5_perform_rsa(struct ccp_op *op)
CCP5_CMD_PROT(&desc) = 0;

function.raw = 0;
- CCP_RSA_SIZE(&function) = op->u.rsa.mod_size;
+ CCP_RSA_SIZE(&function) = op->u.rsa.mod_size >> 3;
CCP5_CMD_FUNCTION(&desc) = function.raw;

CCP5_CMD_LEN(&desc) = op->u.rsa.input_len;
@@ -418,10 +418,10 @@ static int ccp5_perform_rsa(struct ccp_op *op)
CCP5_CMD_DST_HI(&desc) = ccp_addr_hi(&op->dst.u.dma);
CCP5_CMD_DST_MEM(&desc) = CCP_MEMTYPE_SYSTEM;

- /* Key (Exponent) is in external memory */
- CCP5_CMD_KEY_LO(&desc) = ccp_addr_lo(&op->exp.u.dma);
- CCP5_CMD_KEY_HI(&desc) = ccp_addr_hi(&op->exp.u.dma);
- CCP5_CMD_KEY_MEM(&desc) = CCP_MEMTYPE_SYSTEM;
+ /* Exponent is in LSB memory */
+ CCP5_CMD_KEY_LO(&desc) = op->sb_key * LSB_ITEM_SIZE;
+ CCP5_CMD_KEY_HI(&desc) = 0;
+ CCP5_CMD_KEY_MEM(&desc) = CCP_MEMTYPE_SB;

return ccp5_do_cmd(&desc, op->cmd_q);
}


2016-11-13 09:50:22

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: ccp - Fix handling of RSA exponent on a v5 device

On Tue, Nov 01, 2016 at 02:05:05PM -0500, Gary R Hook wrote:
> The exponent size in the ccp_op structure is in bits. A v5
> CCP requires the exponent size to be in bytes, so convert
> the size from bits to bytes when populating the descriptor.
>
> The current code references the exponent in memory, but
> these fields have not been set since the exponent is
> actually store in the LSB. Populate the descriptor with
> the LSB location (address).
>
> Signed-off-by: Gary R Hook <[email protected]>

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-11-15 21:56:36

by Gary R Hook

[permalink] [raw]
Subject: Re: [PATCH] crypto: ccp - Fix handling of RSA exponent on a v5 device

On 11/13/2016 03:49 AM, Herbert Xu wrote:
> On Tue, Nov 01, 2016 at 02:05:05PM -0500, Gary R Hook wrote:
>> The exponent size in the ccp_op structure is in bits. A v5
>> CCP requires the exponent size to be in bytes, so convert
>> the size from bits to bytes when populating the descriptor.
>>
>> The current code references the exponent in memory, but
>> these fields have not been set since the exponent is
>> actually store in the LSB. Populate the descriptor with
>> the LSB location (address).
>>
>> Signed-off-by: Gary R Hook <[email protected]>
>
> Patch applied. Thanks.
>

Thanks, Herbert.

Is there a possibility of getting this pushed to 4.9, being
it's a bug fix?

--
This is my day job. Follow me at:
IG/Twitter/Facebook: @grhookphoto
IG/Twitter/Facebook: @grhphotographer

2016-11-16 09:01:37

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: ccp - Fix handling of RSA exponent on a v5 device

On Tue, Nov 15, 2016 at 03:41:25PM -0600, Gary R Hook wrote:
> On 11/13/2016 03:49 AM, Herbert Xu wrote:
> >On Tue, Nov 01, 2016 at 02:05:05PM -0500, Gary R Hook wrote:
> >>The exponent size in the ccp_op structure is in bits. A v5
> >>CCP requires the exponent size to be in bytes, so convert
> >>the size from bits to bytes when populating the descriptor.
> >>
> >>The current code references the exponent in memory, but
> >>these fields have not been set since the exponent is
> >>actually store in the LSB. Populate the descriptor with
> >>the LSB location (address).
> >>
> >>Signed-off-by: Gary R Hook <[email protected]>
> >
> >Patch applied. Thanks.
> >
>
> Thanks, Herbert.
>
> Is there a possibility of getting this pushed to 4.9, being
> it's a bug fix?

I thought ccp doesn't support RSA yet or is there another entry
path into this code?

Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-11-16 17:25:35

by Gary R Hook

[permalink] [raw]
Subject: Re: [PATCH] crypto: ccp - Fix handling of RSA exponent on a v5 device

On 11/16/2016 03:01 AM, Herbert Xu wrote:
> On Tue, Nov 15, 2016 at 03:41:25PM -0600, Gary R Hook wrote:
>> On 11/13/2016 03:49 AM, Herbert Xu wrote:
>>> On Tue, Nov 01, 2016 at 02:05:05PM -0500, Gary R Hook wrote:
>>>> The exponent size in the ccp_op structure is in bits. A v5
>>>> CCP requires the exponent size to be in bytes, so convert
>>>> the size from bits to bytes when populating the descriptor.
>>>>
>>>> The current code references the exponent in memory, but
>>>> these fields have not been set since the exponent is
>>>> actually store in the LSB. Populate the descriptor with
>>>> the LSB location (address).
>>>>
>>>> Signed-off-by: Gary R Hook <[email protected]>
>>>
>>> Patch applied. Thanks.
>>>
>>
>> Thanks, Herbert.
>>
>> Is there a possibility of getting this pushed to 4.9, being
>> it's a bug fix?
>
> I thought ccp doesn't support RSA yet or is there another entry
> path into this code?

The kernel crypto layer does not yet support RSA, true. However, we
designed the ccp.ko layer to be available to anyone that wants to use
it. The underlying module currently has differing behavior/results
between the v3 and v5 implementations of the RSA command function.
This patch fixes the borked v5 code.


--
This is my day job. Follow me at:
IG/Twitter/Facebook: @grhookphoto
IG/Twitter/Facebook: @grhphotographer

2016-11-17 16:57:24

by Gary R Hook

[permalink] [raw]
Subject: Re: [PATCH] crypto: ccp - Fix handling of RSA exponent on a v5 device

On 11/17/2016 07:14 AM, Herbert Xu wrote:
> On Wed, Nov 16, 2016 at 11:25:19AM -0600, Gary R Hook wrote:
>>
>> The kernel crypto layer does not yet support RSA, true. However, we
>> designed the ccp.ko layer to be available to anyone that wants to use
>> it. The underlying module currently has differing behavior/results
>> between the v3 and v5 implementations of the RSA command function.
>> This patch fixes the borked v5 code.
>
> Do you mean that an out-of-tree module could enter the buggy
> code path?

I mean that anything that can call ccp_run_cmd() (in ccp.ko) can run
into a problem, yes. Is this likely? We don't know, as we don't know
if anyone actually uses this layer. But it _is_ possible to find the
problem.

--
This is my day job. Follow me at:
IG/Twitter/Facebook: @grhookphoto
IG/Twitter/Facebook: @grhphotographer

2016-11-17 17:19:31

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: ccp - Fix handling of RSA exponent on a v5 device

On Wed, Nov 16, 2016 at 11:25:19AM -0600, Gary R Hook wrote:
>
> The kernel crypto layer does not yet support RSA, true. However, we
> designed the ccp.ko layer to be available to anyone that wants to use
> it. The underlying module currently has differing behavior/results
> between the v3 and v5 implementations of the RSA command function.
> This patch fixes the borked v5 code.

Do you mean that an out-of-tree module could enter the buggy
code path?

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt