| Message ID | 20161101190505.1191.20536.stgit@taos (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
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 <gary.hook@amd.com> Patch applied. Thanks.
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 <gary.hook@amd.com> > > Patch applied. Thanks. > Thanks, Herbert. Is there a possibility of getting this pushed to 4.9, being it's a bug fix?
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 <gary.hook@amd.com> > > > >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,
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 <gary.hook@amd.com> >>> >>> 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.
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,
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.
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); }
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 <gary.hook@amd.com> --- drivers/crypto/ccp/ccp-dev-v5.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html