Re: [PATCH v2] X.509: unpack RSA signatureValue field from BIT STRING
On 17.04.2018 15:39, Maciej S. Szmigiero wrote: > The signatureValue field of a X.509 certificate is encoded as a BIT STRING. > For RSA signatures this BIT STRING is of so-called primitive subtype, which > contains a u8 prefix indicating a count of unused bits in the encoding. > > We have to strip this prefix from signature data, just as we already do for > key data in x509_extract_key_data() function. > > This wasn't noticed earlier because this prefix byte is zero for RSA key > sizes divisible by 8. Since BIT STRING is a big-endian encoding adding zero > prefixes has no bearing on its value. > > The signature length, however was incorrect, which is a problem for RSA > implementations that need it to be exactly correct (like AMD CCP). > > Signed-off-by: Maciej S. Szmigiero <m...@maciej.szmigiero.name> your e-mail address looks incorrect [...] -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH v2] crypto: doc - clarify hash callbacks state machine
On 20.03.2018 08:56, Horia Geantă wrote: > Add a note that it is perfectly legal to "abandon" a request object: > - call .init() and then (as many times) .update() > - _not_ call any of .final(), .finup() or .export() at any point in > future > > Link: https://lkml.kernel.org/r/20180222114741.ga27...@gondor.apana.org.au > Signed-off-by: Horia Geantă <horia.gea...@nxp.com> > --- > Documentation/crypto/devel-algos.rst | 8 > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/crypto/devel-algos.rst > b/Documentation/crypto/devel-algos.rst > index 66f50d32dcec..c45c6f400dbd 100644 > --- a/Documentation/crypto/devel-algos.rst > +++ b/Documentation/crypto/devel-algos.rst > @@ -236,6 +236,14 @@ when used from another part of the kernel. > | > '---> HASH2 > > +Note that it is perfectly legal to "abandon" a request object: > +- call .init() and then (as many times) .update() > +- _not_ call any of .final(), .finup() or .export() at any point in future > + > +In other words implementations should mind the resource allocation and > clean-up. > +No resources related to request objects should remain allocated after a call -- > +to .init() or .update(), since there might be no chance to free them. is it for crypto api users or for drivers ? the creator of request context is responsible for alloc and destroy, so why there are no chance of free ? -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH 3/4] crypto: bcm: Constify *hash_alg_name[]
On 27.02.2018 23:01, Hernán Gonzalez wrote:
> Note: This is compile only tested.
> No gain from this except some self-documenting.
>
> Signed-off-by: Hernán Gonzalez <her...@vanguardiasur.com.ar>
> ---
> drivers/crypto/bcm/spu.c | 5 +++--
> drivers/crypto/bcm/spu.h | 2 +-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/bcm/spu.c b/drivers/crypto/bcm/spu.c
> index efaf3cf..c7bb79e 100644
> --- a/drivers/crypto/bcm/spu.c
> +++ b/drivers/crypto/bcm/spu.c
> @@ -23,8 +23,9 @@
> #include "cipher.h"
>
> /* This array is based on the hash algo type supported in spu.h */
> -char *hash_alg_name[] = { "None", "md5", "sha1", "sha224", "sha256", "aes",
^^^
'aes' is not hash, so either remove 'aes' or change array name to
crypto_alg_name
Or maybe I am missing something, or is it hardcoded in silicon ?
> - "sha384", "sha512", "sha3_224", "sha3_256", "sha3_384", "sha3_512" };
> +char const * const hash_alg_name[] = { "None", "md5", "sha1", "sha224",
> + "sha256", "aes", "sha384", "sha512", "sha3_224", "sha3_256", "sha3_384",
> + "sha3_512" };
>
> /* Assumes SPU-M messages are in big endian */
> void spum_dump_msg_hdr(u8 *buf, unsigned int buf_len)
> diff --git a/drivers/crypto/bcm/spu.h b/drivers/crypto/bcm/spu.h
> index f252367..71cf6b5 100644
> --- a/drivers/crypto/bcm/spu.h
> +++ b/drivers/crypto/bcm/spu.h
> @@ -111,7 +111,7 @@ enum aead_type {
> AEAD_TYPE_LAST
> };
>
> -extern char *hash_alg_name[HASH_ALG_LAST];
> +extern const char * const hash_alg_name[HASH_ALG_LAST];
>
> struct spu_request_opts {
> bool is_inbound;
>
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
[PATCH v2] crypto: hash.h: Prevent use of req->result in ahash update
Prevent improper use of req->result field in ahash update, init, export and
import functions in drivers code. A driver should use ahash request context
if it needs to save internal state.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
version 2:
Change req->digest to req->result, as pointed out by Tom Lendacky
include/crypto/hash.h | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 2d1849dffb80..76e432cab75d 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -74,7 +74,8 @@ struct ahash_request {
* @init: **[mandatory]** Initialize the transformation context. Intended only
to initialize the
* state of the HASH transformation at the beginning. This shall fill in
* the internal structures used during the entire duration of the whole
- * transformation. No data processing happens at this point.
+ * transformation. No data processing happens at this point. Driver code
+ * implementation must not use req->result.
* @update: **[mandatory]** Push a chunk of data into the driver for
transformation. This
*function actually pushes blocks of data from upper layers into the
*driver, which then passes those to the hardware as seen fit. This
@@ -83,7 +84,8 @@ struct ahash_request {
*transformation. This function shall not modify the transformation
*context, as this function may be called in parallel with the same
*transformation object. Data processing can happen synchronously
- *[SHASH] or asynchronously [AHASH] at this point.
+ *[SHASH] or asynchronously [AHASH] at this point. Driver must not use
+ *req->result.
* @final: **[mandatory]** Retrieve result from the driver. This function
finalizes the
*transformation and retrieves the resulting hash from the driver and
*pushes it back to upper layers. No data processing happens at this
@@ -120,11 +122,12 @@ struct ahash_request {
* you want to save partial result of the transformation after
* processing certain amount of data and reload this partial result
* multiple times later on for multiple re-use. No data processing
- * happens at this point.
+ * happens at this point. Driver must not use req->result.
* @import: Import partial state of the transformation. This function loads the
* entire state of the ongoing transformation from a provided block of
* data so the transformation can continue from this point onward. No
- * data processing happens at this point.
+ * data processing happens at this point. Driver must not use
+ * req->result.
* @halg: see struct hash_alg_common
*/
struct ahash_alg {
--
2.16.2
Re: [PATCH] crypto: hash.h: Prevent use of req->digest in ahash update
On 06.03.2018 19:04, Tom Lendacky wrote:
> On 3/6/2018 5:45 AM, Kamil Konieczny wrote:
>> Prevent improper use of req->digest field in ahash update, init, export and
>
> Shouldn't that be req->result (here and below)?
Yes, it should, I will send version 2 soon,
thank you.
Best regards,
Kamil Konieczny
>> import functions in drivers code. A driver should use ahash request context
>> if it needs to save internal state.
>>
>> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
>> ---
>> include/crypto/hash.h | 11 +++
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/crypto/hash.h b/include/crypto/hash.h
>> index 2d1849dffb80..e97c2e662d6a 100644
>> --- a/include/crypto/hash.h
>> +++ b/include/crypto/hash.h
>> @@ -74,7 +74,8 @@ struct ahash_request {
>> * @init: **[mandatory]** Initialize the transformation context. Intended
>> only to initialize the
>> *state of the HASH transformation at the beginning. This shall fill in
>> *the internal structures used during the entire duration of the whole
>> - *transformation. No data processing happens at this point.
>> + *transformation. No data processing happens at this point. Driver code
>> + *implementation must not use req->digest.
>> * @update: **[mandatory]** Push a chunk of data into the driver for
>> transformation. This
>> * function actually pushes blocks of data from upper layers into the
>> * driver, which then passes those to the hardware as seen fit. This
>> @@ -83,7 +84,8 @@ struct ahash_request {
>> * transformation. This function shall not modify the transformation
>> * context, as this function may be called in parallel with the same
>> * transformation object. Data processing can happen synchronously
>> - * [SHASH] or asynchronously [AHASH] at this point.
>> + * [SHASH] or asynchronously [AHASH] at this point. Driver must not use
>> + * req->digest.
>> * @final: **[mandatory]** Retrieve result from the driver. This function
>> finalizes the
>> * transformation and retrieves the resulting hash from the driver and
>> * pushes it back to upper layers. No data processing happens at this
>> @@ -120,11 +122,12 @@ struct ahash_request {
>> * you want to save partial result of the transformation after
>> * processing certain amount of data and reload this partial result
>> * multiple times later on for multiple re-use. No data processing
>> - * happens at this point.
>> + * happens at this point. Driver must not use req->digest.
>> * @import: Import partial state of the transformation. This function loads
>> the
>> * entire state of the ongoing transformation from a provided block of
>> * data so the transformation can continue from this point onward. No
>> - * data processing happens at this point.
>> + * data processing happens at this point. Driver must not use
>> + * req->digest.
>> * @halg: see struct hash_alg_common
>> */
>> struct ahash_alg {
>>
>
>
>
[PATCH] crypto: hash.h: Prevent use of req->digest in ahash update
Prevent improper use of req->digest field in ahash update, init, export and
import functions in drivers code. A driver should use ahash request context
if it needs to save internal state.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
include/crypto/hash.h | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 2d1849dffb80..e97c2e662d6a 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -74,7 +74,8 @@ struct ahash_request {
* @init: **[mandatory]** Initialize the transformation context. Intended only
to initialize the
* state of the HASH transformation at the beginning. This shall fill in
* the internal structures used during the entire duration of the whole
- * transformation. No data processing happens at this point.
+ * transformation. No data processing happens at this point. Driver code
+ * implementation must not use req->digest.
* @update: **[mandatory]** Push a chunk of data into the driver for
transformation. This
*function actually pushes blocks of data from upper layers into the
*driver, which then passes those to the hardware as seen fit. This
@@ -83,7 +84,8 @@ struct ahash_request {
*transformation. This function shall not modify the transformation
*context, as this function may be called in parallel with the same
*transformation object. Data processing can happen synchronously
- *[SHASH] or asynchronously [AHASH] at this point.
+ *[SHASH] or asynchronously [AHASH] at this point. Driver must not use
+ *req->digest.
* @final: **[mandatory]** Retrieve result from the driver. This function
finalizes the
*transformation and retrieves the resulting hash from the driver and
*pushes it back to upper layers. No data processing happens at this
@@ -120,11 +122,12 @@ struct ahash_request {
* you want to save partial result of the transformation after
* processing certain amount of data and reload this partial result
* multiple times later on for multiple re-use. No data processing
- * happens at this point.
+ * happens at this point. Driver must not use req->digest.
* @import: Import partial state of the transformation. This function loads the
* entire state of the ongoing transformation from a provided block of
* data so the transformation can continue from this point onward. No
- * data processing happens at this point.
+ * data processing happens at this point. Driver must not use
+ * req->digest.
* @halg: see struct hash_alg_common
*/
struct ahash_alg {
--
2.16.2
Re: Why are we testing an intermediate result in ahash?
On 05.03.2018 18:47, Gary R Hook wrote: > On 03/05/2018 03:57 AM, Kamil Konieczny wrote: >> >> >> On 02.03.2018 22:11, Gary R Hook wrote: >>> Commit 466d7b9f6 (cryptodev-2.6) added code to testmgr to populate, for >>> async hash operations, >>> the result buffer with a known value and to test the buffer against that >>> value at intermediate >>> steps. If the result buffer changes the operation is failed. >>> >>> My question is: why? >>> >>> What problem does this solve? Has this requirement existed all along, or is >>> it new? >>> >>> I'm now seeing complaints for AES/CMAC and SHA in my driver. I have no >>> problem updating the driver, >>> of course, but I'd like to better understand the precipitating issue for >>> the commit. >>> >>> Mar 2 12:30:56 sosxen2 kernel: [ 60.919198] alg: No test for cfb(aes) >>> (cfb-aes-ccp) >>> Mar 2 12:30:56 sosxen2 kernel: [ 60.924787] 367: alg: hash: update >>> failed on test 3 for cmac-aes-ccp: used req->result >>> Mar 2 12:30:56 sosxen2 kernel: [ 60.946571] 367: alg: hash: update >>> failed on test 4 for sha1-ccp: used req->result >>> Mar 2 12:30:56 sosxen2 kernel: [ 60.956461] 367: alg: hash: update >>> failed on test 1 for hmac-sha1-ccp: used req->result >>> Mar 2 12:30:56 sosxen2 kernel: [ 60.966117] 367: alg: hash: update >>> failed on test 4 for sha224-ccp: used req->result >> >> ahash req->result can be used in digit, final and finup hash operations. >> It should not be used in init and update (nor in export and import). > > Where is this documented, please? I'm not seeing it in Documentation/crypto. > Of course, I could be looking for the wrong thing. It was recent addition, and you are right, the doc needs update. >> There were some bugs in past, when drivers try to use req->result >> as theirs temporary storage. >> The bug comes up in some scenarios when caller reused ahash request >> and leaves in req->result undefined value, it can be NULL or >> container_of(NULL) >> or whatever was on stack >> > > As I mention in my other post, our driver vets the pointer before > dereference. Problem will not happen with NULL, but only because there is code like (pseudocode) in ahash_update: 1: if (req->result == NULL) 2: // use as temp memory ahash request context 3: else 4: // use as temp memory req->result The point is - if we need temporary storage for keeping some state between updates, we should use ahash request context, and get rid of the code lines 1,3,4 The line 4 can bite us when req->result will be neither NULL nor valid memory pointer. The second argument is, why we bother to check with 1: when we can should already do 2: only > And I don't have a problem with a clear definition of what should and should > not happen to > buffers offered by a caller. I simply want to know where this behavior is > defined, and is it a change from the past? This come up after some code review. -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: Why are we testing an intermediate result in ahash?
On 02.03.2018 22:11, Gary R Hook wrote: > Commit 466d7b9f6 (cryptodev-2.6) added code to testmgr to populate, for async > hash operations, > the result buffer with a known value and to test the buffer against that > value at intermediate > steps. If the result buffer changes the operation is failed. > > My question is: why? > > What problem does this solve? Has this requirement existed all along, or is > it new? > > I'm now seeing complaints for AES/CMAC and SHA in my driver. I have no > problem updating the driver, > of course, but I'd like to better understand the precipitating issue for the > commit. > > Mar 2 12:30:56 sosxen2 kernel: [ 60.919198] alg: No test for cfb(aes) > (cfb-aes-ccp) > Mar 2 12:30:56 sosxen2 kernel: [ 60.924787] 367: alg: hash: update failed > on test 3 for cmac-aes-ccp: used req->result > Mar 2 12:30:56 sosxen2 kernel: [ 60.946571] 367: alg: hash: update failed > on test 4 for sha1-ccp: used req->result > Mar 2 12:30:56 sosxen2 kernel: [ 60.956461] 367: alg: hash: update failed > on test 1 for hmac-sha1-ccp: used req->result > Mar 2 12:30:56 sosxen2 kernel: [ 60.966117] 367: alg: hash: update failed > on test 4 for sha224-ccp: used req->result ahash req->result can be used in digit, final and finup hash operations. It should not be used in init and update (nor in export and import). There were some bugs in past, when drivers try to use req->result as theirs temporary storage. The bug comes up in some scenarios when caller reused ahash request and leaves in req->result undefined value, it can be NULL or container_of(NULL) or whatever was on stack -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH] Update Boris Brezillon email address
On 16.02.2018 15:54, Boris Brezillon wrote: > Adding back all the people that were Cc-ed on the initial email. > > On Fri, 16 Feb 2018 15:18:21 +0100 > Kamil Konieczny <k.koniec...@partner.samsung.com> wrote: > >> On 16.02.2018 15:00, Boris Brezillon wrote: >>> On Fri, 16 Feb 2018 12:21:53 +0100 >>> Kamil Konieczny <k.koniec...@partner.samsung.com> wrote: >>> >>>> On 16.02.2018 12:16, Boris Brezillon wrote: >>>>> On Fri, 16 Feb 2018 12:07:42 +0100 >>>>> Kamil Konieczny <k.koniec...@partner.samsung.com> wrote: >>>>> >>>>>> On 16.02.2018 11:44, Boris Brezillon wrote: >>>>>>> Free Electrons is now Bootlin. >>>>>>> >>>>>>> Signed-off-by: Boris Brezillon <boris.brezil...@bootlin.com> >>>>>>> --- >>>>>>> Note that I'm planning to take this patch through the MTD tree. >>>>>>> --- >>>>>>> .mailmap| 7 --- >>>>>>> MAINTAINERS | 10 +- >>>>>>> 2 files changed, 9 insertions(+), 8 deletions(-) >>>>>>> >>>>>>> diff --git a/.mailmap b/.mailmap >>>>>>> index e18cab73e209..50c1d6bf36b2 100644 >>>>>>> --- a/.mailmap >>>>>>> +++ b/.mailmap >>>>>>> @@ -33,9 +33,10 @@ Axel Lin <axel@gmail.com> >>>>>>> Ben Gardner <bgard...@wabtec.com> >>>>>>> Ben M Cahill <ben.m.cah...@intel.com> >>>>>>> Björn Steinbrink <b.steinbr...@gmx.de> >>>>>>> -Boris Brezillon <boris.brezil...@free-electrons.com> >>>>>>> -Boris Brezillon <boris.brezil...@free-electrons.com> >>>>>>> <b.brezillon@gmail.com> >>>>>>> -Boris Brezillon <boris.brezil...@free-electrons.com> >>>>>>> <b.brezil...@overkiz.com> >>>>>>> +Boris Brezillon <boris.brezil...@bootlin.com> >>>>>>> +Boris Brezillon <boris.brezil...@bootlin.com> >>>>>>> <boris.brezil...@free-electrons.com> >>>>>> >>>>>> -- >>>>>> ?? >>>>> >>>>> Is there a problem with this line? >>>>> >>>> >>>> I think you want to delete all 'free-electrons', yet here you add this >>> >>> I'm clearly not a git expert, but according to man git-shortlog [1], the >>> format I'm using here allows mailmap to replace the old email by the >>> new one, which is exactly what I want. >>> >>> [1]https://git-scm.com/docs/git-shortlog >>> >> >> Ah, I see, but why do you send patch for .mailmap ? >> it is your private file, not global one, I do not want mine .mailmap >> to be replaced by yours :) > > No, it's not private, it's part of the git tree. > >> But beside this, you can use sed script >> >> sed -i -e 's/free-electrons/bootlin/g' .mailmap >> >> Please just resend this with patch for MAINTAINER's only >> > > .mailmap is versioned for a good reason, and I don't see why I couldn't > update entries for my emails, especially since those entries already > existed before this patch. I am newbie, it looks you are right, sorry for questions. -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH v3 0/5] crypto: ahash.c: Require export/import in ahash
On 15.02.2018 19:32, Marek Vasut wrote: > On 02/15/2018 07:06 PM, Kamil Konieczny wrote: >> >> >> On 15.02.2018 18:06, Marek Vasut wrote: >>> On 02/15/2018 06:00 PM, Kamil Konieczny wrote: >>>> >>>> >>>> On 15.02.2018 17:27, Marek Vasut wrote: >>>>> On 02/15/2018 04:41 PM, Herbert Xu wrote: >>>>>> On Thu, Jan 18, 2018 at 07:33:59PM +0100, Kamil Konieczny wrote: >>>>>>> First four patches add empty hash export and import functions to each >>>>>>> driver, >>>>>>> with the same behaviour as in crypto framework. The last one drops them >>>>>>> from >>>>>>> crypto framework. Last one for ahash.c depends on all previous. >>>>>>> >>>>>>> Changes in v3: >>>>>>> added change for bfin_crc.c >>>>>>> make this a patchset, instead of unreleated patches >>>>>>> make commit message more descriptive >>>>>>> >>>>>>> Kamil Konieczny (5): >>>>>>> crypto: mxs-dcp: Add empty hash export and import >>>>>>> crypto: n2_core: Add empty hash export and import >>>>>>> crypto: ux500/hash: Add empty export and import >>>>>>> crypto: bfin_crc: Add empty hash export and import >>>>>>> crypto: ahash.c: Require export/import in ahash >>>>>>> >>>>>>> crypto/ahash.c| 18 ++ >>>>>>> drivers/crypto/bfin_crc.c | 12 >>>>>>> drivers/crypto/mxs-dcp.c | 14 ++ >>>>>>> drivers/crypto/n2_core.c | 12 >>>>>>> drivers/crypto/ux500/hash/hash_core.c | 18 ++ >>>>>>> 5 files changed, 58 insertions(+), 16 deletions(-) >>>>>> >>>>>> All applied. Thanks. >>>>> >>>>> This makes no sense, cfr my comment on 5/5 >>>>> >>>>> Seems like if the driver doesn't implement those, the core can easily >>>>> detect that and perform the necessary action. Moving the checks out of >>>>> core seems like the wrong thing to do, rather you should enhance the >>>>> checks in core if they're insufficient in my opinion. >>>> >>>> The bug can only be in driver which will not implement those two functions, >>>> but we already had all drivers with those due to patches 1..4 >>>> All other drivers do have them. >>> >>> The core can very well check if these functions are not populated and >>> return ENOSYS >>> >>>> Additionally, with crypto we want minimize code and run as fast as >>>> possible. >>> >>> So you remove all NULL pointer checks ? Esp. in security-sensitive code? >>> What is the impact of this non-critical path code on performance? >>> >>> Come on ... >>> >> >> Why you want checks for something that not exist ? >> >> Those without them will not work and will do Oops in crypto testmgr, >> so such drivers should not be used nor accepted in drivers/crypto >> >> Ask yourself why crypto do not check for NULL in ahash digest or other >> required ahash functions. > > Are you suggesting that the kernel code should NOT perform NULL pointer > checks ? > > Are you suggesting each driver should implement every single callback > available and if it is not implemented, return -ENOSYS ? This looks like > a MASSIVE code duplication. It is source code duplication. One do not load all crypto drivers at once, simple because one board has only one crypto HW (or few closely related), and if one even try, almost none of them will initialize on given hardware. E.g. on Exynos board only exynos drivers will load, on board with omap crypto only omap crypto will load. >>>> Moving checks out of core will impose on driver author need for implement >>>> those functions, or declare them empty, but in case of empty ones >>>> crypto will not work properly with such driver. >>> >>> You can very well impose that in the core, except you don't duplicate >>> the code. >> >> Now size of crypto core is reduced. > > You implemented the same code thrice, it surely is not reduced. As I said above, it reduces binary size at cost of more source code in few drivers. -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH v3 0/5] crypto: ahash.c: Require export/import in ahash
On 15.02.2018 18:06, Marek Vasut wrote: > On 02/15/2018 06:00 PM, Kamil Konieczny wrote: >> >> >> On 15.02.2018 17:27, Marek Vasut wrote: >>> On 02/15/2018 04:41 PM, Herbert Xu wrote: >>>> On Thu, Jan 18, 2018 at 07:33:59PM +0100, Kamil Konieczny wrote: >>>>> First four patches add empty hash export and import functions to each >>>>> driver, >>>>> with the same behaviour as in crypto framework. The last one drops them >>>>> from >>>>> crypto framework. Last one for ahash.c depends on all previous. >>>>> >>>>> Changes in v3: >>>>> added change for bfin_crc.c >>>>> make this a patchset, instead of unreleated patches >>>>> make commit message more descriptive >>>>> >>>>> Kamil Konieczny (5): >>>>> crypto: mxs-dcp: Add empty hash export and import >>>>> crypto: n2_core: Add empty hash export and import >>>>> crypto: ux500/hash: Add empty export and import >>>>> crypto: bfin_crc: Add empty hash export and import >>>>> crypto: ahash.c: Require export/import in ahash >>>>> >>>>> crypto/ahash.c| 18 ++ >>>>> drivers/crypto/bfin_crc.c | 12 >>>>> drivers/crypto/mxs-dcp.c | 14 ++ >>>>> drivers/crypto/n2_core.c | 12 >>>>> drivers/crypto/ux500/hash/hash_core.c | 18 ++ >>>>> 5 files changed, 58 insertions(+), 16 deletions(-) >>>> >>>> All applied. Thanks. >>> >>> This makes no sense, cfr my comment on 5/5 >>> >>> Seems like if the driver doesn't implement those, the core can easily >>> detect that and perform the necessary action. Moving the checks out of >>> core seems like the wrong thing to do, rather you should enhance the >>> checks in core if they're insufficient in my opinion. >> >> The bug can only be in driver which will not implement those two functions, >> but we already had all drivers with those due to patches 1..4 >> All other drivers do have them. > > The core can very well check if these functions are not populated and > return ENOSYS > >> Additionally, with crypto we want minimize code and run as fast as possible. > > So you remove all NULL pointer checks ? Esp. in security-sensitive code? > What is the impact of this non-critical path code on performance? > > Come on ... > Why you want checks for something that not exist ? Those without them will not work and will do Oops in crypto testmgr, so such drivers should not be used nor accepted in drivers/crypto Ask yourself why crypto do not check for NULL in ahash digest or other required ahash functions. >> Moving checks out of core will impose on driver author need for implement >> those functions, or declare them empty, but in case of empty ones >> crypto will not work properly with such driver. > > You can very well impose that in the core, except you don't duplicate > the code. Now size of crypto core is reduced. -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH v3 0/5] crypto: ahash.c: Require export/import in ahash
On 15.02.2018 17:27, Marek Vasut wrote: > On 02/15/2018 04:41 PM, Herbert Xu wrote: >> On Thu, Jan 18, 2018 at 07:33:59PM +0100, Kamil Konieczny wrote: >>> First four patches add empty hash export and import functions to each >>> driver, >>> with the same behaviour as in crypto framework. The last one drops them from >>> crypto framework. Last one for ahash.c depends on all previous. >>> >>> Changes in v3: >>> added change for bfin_crc.c >>> make this a patchset, instead of unreleated patches >>> make commit message more descriptive >>> >>> Kamil Konieczny (5): >>> crypto: mxs-dcp: Add empty hash export and import >>> crypto: n2_core: Add empty hash export and import >>> crypto: ux500/hash: Add empty export and import >>> crypto: bfin_crc: Add empty hash export and import >>> crypto: ahash.c: Require export/import in ahash >>> >>> crypto/ahash.c| 18 ++ >>> drivers/crypto/bfin_crc.c | 12 >>> drivers/crypto/mxs-dcp.c | 14 ++ >>> drivers/crypto/n2_core.c | 12 >>> drivers/crypto/ux500/hash/hash_core.c | 18 ++ >>> 5 files changed, 58 insertions(+), 16 deletions(-) >> >> All applied. Thanks. > > This makes no sense, cfr my comment on 5/5 > > Seems like if the driver doesn't implement those, the core can easily > detect that and perform the necessary action. Moving the checks out of > core seems like the wrong thing to do, rather you should enhance the > checks in core if they're insufficient in my opinion. The bug can only be in driver which will not implement those two functions, but we already had all drivers with those due to patches 1..4 All other drivers do have them. Additionally, with crypto we want minimize code and run as fast as possible. Moving checks out of core will impose on driver author need for implement those functions, or declare them empty, but in case of empty ones crypto will not work properly with such driver. -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH v3] crypto: s5p-sss.c: Fix kernel Oops in AES-ECB mode
In AES-ECB mode crypt is done with key only, so any use of IV
can cause kernel Oops. Use IV only in AES-CBC and AES-CTR.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
Reported-by: Anand Moon <linux.am...@gmail.com>
Reviewed-by: Krzysztof Kozlowski <k...@kernel.org>
Tested-by: Anand Moon <linux.am...@gmail.com>
Cc: sta...@vger.kernel.org # can be applied after commit 8f9702aad138
---
version 3:
Change commit message: drop 'Fixes: 8f9702aad138' as the error was
introduced earlier.
version 2:
Change commit message.
Tested on Odroid XU4/HC1, kernel 4.15 with following command:
fallocate -l 128MiB /tmp/test.bin
dd if=/dev/urandom of=/tmp/testkey.key bs=128 count=1
sync
cryptsetup luksFormat --debug -q -d /tmp/testkey.key \
--cipher aes-cbc-essiv:sha256 -h sha256 -s 128 /tmp/test.bin
The original report by Anand Moon:
https://www.spinics.net/lists/linux-crypto/msg31180.html
Oops reproduced with cryptsetup 2.0.0, kernel 4.15,
in .config in crypto API ECB support was turned off, and s5p-sss AES driver on.
cryptsetup is using aes-ecb and has req->info in aes_ctx set to 0x10,
which caused Oops:
[ 2078.683779] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
[ 2078.689148] Modules linked in: algif_skcipher af_alg sd_mod sg
evdev uas usb_storage scsi_mod gpio_keys fbtft(C) spidev spi_s3c64xx
ipv6
[ 2078.701377] CPU: 1 PID: 15 Comm: ksoftirqd/1 Tainted: G C
4.15.0-rc9-xu4krck #1
[ 2078.709861] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 2078.715932] PC is at memcpy+0x80/0x330
[ 2078.719652] LR is at s5p_tasklet_cb+0x19c/0x328
drivers/crypto/s5p-sss.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 142c6020cec7..5c0496d1ed41 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -1926,15 +1926,21 @@ static void s5p_aes_crypt_start(struct s5p_aes_dev
*dev, unsigned long mode)
uint32_t aes_control;
unsigned long flags;
int err;
+ u8 *iv;
aes_control = SSS_AES_KEY_CHANGE_MODE;
if (mode & FLAGS_AES_DECRYPT)
aes_control |= SSS_AES_MODE_DECRYPT;
- if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CBC)
+ if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CBC) {
aes_control |= SSS_AES_CHAIN_MODE_CBC;
- else if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CTR)
+ iv = req->info;
+ } else if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CTR) {
aes_control |= SSS_AES_CHAIN_MODE_CTR;
+ iv = req->info;
+ } else {
+ iv = NULL; /* AES_ECB */
+ }
if (dev->ctx->keylen == AES_KEYSIZE_192)
aes_control |= SSS_AES_KEY_SIZE_192;
@@ -1965,7 +1971,7 @@ static void s5p_aes_crypt_start(struct s5p_aes_dev *dev,
unsigned long mode)
goto outdata_error;
SSS_AES_WRITE(dev, AES_CONTROL, aes_control);
- s5p_set_aes(dev, dev->ctx->aes_key, req->info, dev->ctx->keylen);
+ s5p_set_aes(dev, dev->ctx->aes_key, iv, dev->ctx->keylen);
s5p_set_dma_indata(dev, dev->sg_src);
s5p_set_dma_outdata(dev, dev->sg_dst);
--
2.16.0
[PATCH v2] crypto: s5p-sss.c: Fix kernel Oops in AES-ECB mode
In AES-ECB mode crypt is done with key only, so any use of IV
can cause kernel Oops. Use IV only in AES-CBC and AES-CTR.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
Reported-by: Anand Moon <linux.am...@gmail.com>
Reviewed-by: Krzysztof Kozlowski <k...@kernel.org>
Tested-by: Anand Moon <linux.am...@gmail.com>
Cc: sta...@vger.kernel.org
Fixes: 8f9702aad138 ("crypto: s5p-sss - validate iv before memcpy")
---
version 2:
Change commit message.
Tested on Odroid XU4/HC1, kernel 4.15 with following command:
fallocate -l 128MiB /tmp/test.bin
dd if=/dev/urandom of=/tmp/testkey.key bs=128 count=1
sync
cryptsetup luksFormat --debug -q -d /tmp/testkey.key \
--cipher aes-cbc-essiv:sha256 -h sha256 -s 128 /tmp/test.bin
The original report by Anand Moon:
https://www.spinics.net/lists/linux-crypto/msg31180.html
Oops reproduced with cryptsetup 2.0.0, kernel 4.15,
in .config in crypto API ECB support was turned off, and s5p-sss AES driver on.
cryptsetup is using aes-ecb and has req->info in aes_ctx set to 0x10,
which caused Oops:
[ 2078.683779] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
[ 2078.689148] Modules linked in: algif_skcipher af_alg sd_mod sg
evdev uas usb_storage scsi_mod gpio_keys fbtft(C) spidev spi_s3c64xx
ipv6
[ 2078.701377] CPU: 1 PID: 15 Comm: ksoftirqd/1 Tainted: G C
4.15.0-rc9-xu4krck #1
[ 2078.709861] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 2078.715932] PC is at memcpy+0x80/0x330
[ 2078.719652] LR is at s5p_tasklet_cb+0x19c/0x328
drivers/crypto/s5p-sss.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 142c6020cec7..5c0496d1ed41 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -1926,15 +1926,21 @@ static void s5p_aes_crypt_start(struct s5p_aes_dev
*dev, unsigned long mode)
uint32_t aes_control;
unsigned long flags;
int err;
+ u8 *iv;
aes_control = SSS_AES_KEY_CHANGE_MODE;
if (mode & FLAGS_AES_DECRYPT)
aes_control |= SSS_AES_MODE_DECRYPT;
- if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CBC)
+ if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CBC) {
aes_control |= SSS_AES_CHAIN_MODE_CBC;
- else if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CTR)
+ iv = req->info;
+ } else if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CTR) {
aes_control |= SSS_AES_CHAIN_MODE_CTR;
+ iv = req->info;
+ } else {
+ iv = NULL; /* AES_ECB */
+ }
if (dev->ctx->keylen == AES_KEYSIZE_192)
aes_control |= SSS_AES_KEY_SIZE_192;
@@ -1965,7 +1971,7 @@ static void s5p_aes_crypt_start(struct s5p_aes_dev *dev,
unsigned long mode)
goto outdata_error;
SSS_AES_WRITE(dev, AES_CONTROL, aes_control);
- s5p_set_aes(dev, dev->ctx->aes_key, req->info, dev->ctx->keylen);
+ s5p_set_aes(dev, dev->ctx->aes_key, iv, dev->ctx->keylen);
s5p_set_dma_indata(dev, dev->sg_src);
s5p_set_dma_outdata(dev, dev->sg_dst);
--
2.16.0
Re: [PATCH] crypto: s5p-sss.c: Fix kernel Oops in AES-ECB mode
On 06.02.2018 17:48, Anand Moon wrote: > Hi Kamil, > > Thanks for providing the fix to this issue. > > On 5 February 2018 at 23:10, Kamil Konieczny > <k.koniec...@partner.samsung.com> wrote: >> >> In AES-ECB mode crypt is done with key only, so any use of IV >> can cause kernel Oops, as reported by Anand Moon. > > If possible could you avoid the name in commit message. This is added after '---' line, so it will not appear in commit message. >> Fixed it by using IV only in AES-CBC and AES-CTR. >> >> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> >> Reported-by: Anand Moon <linux.am...@gmail.com> > > [snip] > > Please add my. Tested on Odroid HC2 > > Tested-by: Anand Moon <linux.am...@gmail.com> This will add you name in commit message, additionally with 'Reported-by:' line. > Below are the result at my end. > > aes-cbc-essiv:sha256 (128 bit key) > WRITE: > 100+0 records in > 100+0 records out > 838860800 bytes (839 MB, 800 MiB) copied, 11.7225 s, 71.6 MB/s > [...] is it from 'cryptsetup benchmark' ? benchmark did not cause oops. Please test with luksFormat, ie. use cryptsetup luksFormat --debug -q -d /tmp/testkey.key \ --cipher aes-cbc-essiv:sha256 -h sha256 -s 128 /tmp/test.bin -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH] crypto: s5p-sss.c: Fix kernel Oops in AES-ECB mode
In AES-ECB mode crypt is done with key only, so any use of IV
can cause kernel Oops, as reported by Anand Moon.
Fixed it by using IV only in AES-CBC and AES-CTR.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
Reported-by: Anand Moon <linux.am...@gmail.com>
---
Tested on Odroid XU4/HC1, kernel 4.15 with following command:
fallocate -l 128MiB /tmp/test.bin
dd if=/dev/urandom of=/tmp/testkey.key bs=128 count=1
sync
cryptsetup luksFormat --debug -q -d /tmp/testkey.key \
--cipher aes-cbc-essiv:sha256 -h sha256 -s 128 /tmp/test.bin
The original report by Anand Moon:
https://www.spinics.net/lists/linux-crypto/msg31180.html
Oops reproduced with cryptsetup 2.0.0, kernel 4.15,
in .config in crypto API ECB support was turned off, and s5p-sss AES driver on.
cryptsetup is using aes-ecb and has req->info in aes_ctx set to 0x10,
which caused Oops:
[ 2078.683779] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
[ 2078.689148] Modules linked in: algif_skcipher af_alg sd_mod sg
evdev uas usb_storage scsi_mod gpio_keys fbtft(C) spidev spi_s3c64xx
ipv6
[ 2078.701377] CPU: 1 PID: 15 Comm: ksoftirqd/1 Tainted: G C
4.15.0-rc9-xu4krck #1
[ 2078.709861] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 2078.715932] PC is at memcpy+0x80/0x330
[ 2078.719652] LR is at s5p_tasklet_cb+0x19c/0x328
drivers/crypto/s5p-sss.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 142c6020cec7..5c0496d1ed41 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -1926,15 +1926,21 @@ static void s5p_aes_crypt_start(struct s5p_aes_dev
*dev, unsigned long mode)
uint32_t aes_control;
unsigned long flags;
int err;
+ u8 *iv;
aes_control = SSS_AES_KEY_CHANGE_MODE;
if (mode & FLAGS_AES_DECRYPT)
aes_control |= SSS_AES_MODE_DECRYPT;
- if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CBC)
+ if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CBC) {
aes_control |= SSS_AES_CHAIN_MODE_CBC;
- else if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CTR)
+ iv = req->info;
+ } else if ((mode & FLAGS_AES_MODE_MASK) == FLAGS_AES_CTR) {
aes_control |= SSS_AES_CHAIN_MODE_CTR;
+ iv = req->info;
+ } else {
+ iv = NULL; /* AES_ECB */
+ }
if (dev->ctx->keylen == AES_KEYSIZE_192)
aes_control |= SSS_AES_KEY_SIZE_192;
@@ -1965,7 +1971,7 @@ static void s5p_aes_crypt_start(struct s5p_aes_dev *dev,
unsigned long mode)
goto outdata_error;
SSS_AES_WRITE(dev, AES_CONTROL, aes_control);
- s5p_set_aes(dev, dev->ctx->aes_key, req->info, dev->ctx->keylen);
+ s5p_set_aes(dev, dev->ctx->aes_key, iv, dev->ctx->keylen);
s5p_set_dma_indata(dev, dev->sg_src);
s5p_set_dma_outdata(dev, dev->sg_dst);
--
2.16.0
Re: Odroid HC1 cryptsetup:encrypt sata driver
On 24.01.2018 15:01, Krzysztof Kozlowski wrote: > On Wed, Jan 24, 2018 at 2:04 PM, Anand Moon <linux.am...@gmail.com> wrote: >> Hi Kamil Konieczny, >> >> I am looking in setup of encrypted sata hard-disk on Odroid XU4/HC1 device. >> using following encryption method. >> >> aes-cbc-essiv:sha256 128 >> aes-cbc-essiv:sha256 256 >> >> Here is my defconfig I am using. https://pastebin.com/gF5T2stp >> >> Following crypt benchmark we use to test : https://pastebin.com/WiexsJA2 > > No problems on my side with a 128 MB file (not a device): > # cryptsetup -v luksFormat /tmp/testcrypt /dev/urandom > --keyfile-size=32 --cipher aes-cbc-essiv:sha256 > # Command successful. > # cryptsetup -v luksFormat ~/testcrypt /dev/urandom --keyfile-size=32 > --cipher aes-cbc-essiv:sha256 > # Command successful. What version is cryptsetup ? > > Linux 4.15.0-rc9-00023-g1f07476ec143. > > Some time ago you were building from not usual source code and your > kernel version from WARN is not unambiguous. > > What is necessary to reproduce it? >>[...] I get OOPS with cryptsetup 2.0.0, kernel 4.15 If you have older cryptsetup, please keep it for reference. For now, I found that crypto-api or dm-crypt try to use aes-ecb instead of aes-cbc, and sets req->info (pointer used for passing IV) to 0x10 -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: Odroid HC1 cryptsetup:encrypt sata driver
On 31.01.2018 06:51, Anand Moon wrote: > Hi Kamil, > > On 30 January 2018 at 21:02, Kamil Konieczny > <k.koniec...@partner.samsung.com> wrote: >> Hi Anand, >> >> On 24.01.2018 14:04, Anand Moon wrote: >>> Hi Kamil Konieczny, >>> >>> I am looking in setup of encrypted sata hard-disk on Odroid XU4/HC1 device. >>> using following encryption method. >>> >>> aes-cbc-essiv:sha256 128 >>> aes-cbc-essiv:sha256 256 >>> >>> Here is my defconfig I am using. https://pastebin.com/gF5T2stp >>> >>> Following crypt benchmark we use to test : https://pastebin.com/WiexsJA2 >>> >>> When I am trying to format the the hard drive I am getting kernel panic. >>> I have tired different option like below. >>> >>> *Please guide me in how to fix this bug* >>> [...] >> >> Sorry for late response, I was on holidays. Try latest kernel 4.15 >> and turn off option: >> [...] > > Thanks for your input, but I tried your suggestion on latest kernel. > but the result is the same. > > How can I help trace this bug. please guide me. I was able to reproduce this bug with latest kernel 4.15 on Odroid HC1, I am debugging it now. Thank you for reporting it. -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: Odroid HC1 cryptsetup:encrypt sata driver
Hi Anand, On 24.01.2018 14:04, Anand Moon wrote: > Hi Kamil Konieczny, > > I am looking in setup of encrypted sata hard-disk on Odroid XU4/HC1 device. > using following encryption method. > > aes-cbc-essiv:sha256 128 > aes-cbc-essiv:sha256 256 > > Here is my defconfig I am using. https://pastebin.com/gF5T2stp > > Following crypt benchmark we use to test : https://pastebin.com/WiexsJA2 > > When I am trying to format the the hard drive I am getting kernel panic. > I have tired different option like below. > > *Please guide me in how to fix this bug* > [...] Sorry for late response, I was on holidays. Try latest kernel 4.15 and turn off option: CONFIG_CRYPTO_DEV_EXYNOS_HASH=n in Cryptographic API: Hardware crypto devices: Support for Samsung Exynos HASH accelerator --> turn off You should also turn on option for software SHA256 (and maybe SHA1) This is last change in this driver, so maybe there is some problem with concurrent access to hardware by AES and HASH driver parts. -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH 5/5] crypto: ahash.c: Require export/import in ahash
On 19.01.2018 11:08, Marek Vasut wrote:
> On 01/19/2018 10:53 AM, Kamil Konieczny wrote:
>> On 18.01.2018 22:31, Marek Vasut wrote:
>>> On 01/18/2018 07:34 PM, Kamil Konieczny wrote:
>>>> Export and import are mandatory in async hash. As drivers were
>>>> rewritten, drop empty wrappers and correct init of ahash transformation.
>>>
>>> Are you moving checks from the core subsystem to drivers ? This looks
>>> really nonsensical and the commit message doesn't explain the rationale
>>> for that at all.
>>
>> I am removing checks from core. Export and import were optional in beginnig
>> of crypto framework, but as time goes on they become mandatory.
>
> Seems like if the driver doesn't implement those, the core can easily
> detect that and perform the necessary action. Moving the checks out of
> core seems like the wrong thing to do, rather you should enhance the
> checks in core if they're insufficient in my opinion.
I removed all checks. No checks in driver and no checks in crypto framework.
If you would like any check, I think the place to add them is in ahash alg
registraction, in function ahash_prepare_alg add something like:
if ((alg->init == NULL) ||
(alg->digest == NULL) ||
(alg->final == NULL) ||
(alg->update == NULL) ||
(alg->export == NULL) ||
(alg->import == NULL))
return -EINVAL;
The only downsize is this will be usefull in backport (to prevent NULL
dereference),
as any new driver will have all those pointers sets.
>>>> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
>>>> ---
>>>> crypto/ahash.c | 18 ++
>>>> 1 file changed, 2 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/crypto/ahash.c b/crypto/ahash.c
>>>> index 3a35d67de7d9..c3cce508c1d4 100644
>>>> --- a/crypto/ahash.c
>>>> +++ b/crypto/ahash.c
>>>> @@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
>>>>return ahash_def_finup_finish1(req, err);
>>>> }
>>>>
>>>> -static int ahash_no_export(struct ahash_request *req, void *out)
>>>> -{
>>>> - return -ENOSYS;
>>>> -}
>>>> -
>>>> -static int ahash_no_import(struct ahash_request *req, const void *in)
>>>> -{
>>>> - return -ENOSYS;
>>>> -}
>>>> -
>>>> static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>>>> {
>>>>struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
>>>> @@ -451,8 +441,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm
>>>> *tfm)
>>>>
>>>>hash->setkey = ahash_nosetkey;
>>>>hash->has_setkey = false;
>>>> - hash->export = ahash_no_export;
>>>> - hash->import = ahash_no_import;
>>>>
>>>>if (tfm->__crt_alg->cra_type != _ahash_type)
>>>>return crypto_init_shash_ops_async(tfm);
>>>> @@ -462,15 +450,13 @@ static int crypto_ahash_init_tfm(struct crypto_tfm
>>>> *tfm)
>>>>hash->final = alg->final;
>>>>hash->finup = alg->finup ?: ahash_def_finup;
>>>>hash->digest = alg->digest;
>>>> + hash->export = alg->export;
>>>> + hash->import = alg->import;
>>>>
>>>>if (alg->setkey) {
>>>>hash->setkey = alg->setkey;
>>>>hash->has_setkey = true;
>>>>}
>>>> - if (alg->export)
>>>> - hash->export = alg->export;
>>>> - if (alg->import)
>>>> - hash->import = alg->import;
>>>>
>>>>return 0;
>>>> }
>>>>
>>>
>>>
>>
>
>
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH 5/5] crypto: ahash.c: Require export/import in ahash
On 18.01.2018 22:31, Marek Vasut wrote:
> On 01/18/2018 07:34 PM, Kamil Konieczny wrote:
>> Export and import are mandatory in async hash. As drivers were
>> rewritten, drop empty wrappers and correct init of ahash transformation.
>
> Are you moving checks from the core subsystem to drivers ? This looks
> really nonsensical and the commit message doesn't explain the rationale
> for that at all.
I am removing checks from core. Export and import were optional in beginnig
of crypto framework, but as time goes on they become mandatory.
>
>> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
>> ---
>> crypto/ahash.c | 18 ++
>> 1 file changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/crypto/ahash.c b/crypto/ahash.c
>> index 3a35d67de7d9..c3cce508c1d4 100644
>> --- a/crypto/ahash.c
>> +++ b/crypto/ahash.c
>> @@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
>> return ahash_def_finup_finish1(req, err);
>> }
>>
>> -static int ahash_no_export(struct ahash_request *req, void *out)
>> -{
>> -return -ENOSYS;
>> -}
>> -
>> -static int ahash_no_import(struct ahash_request *req, const void *in)
>> -{
>> -return -ENOSYS;
>> -}
>> -
>> static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>> {
>> struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
>> @@ -451,8 +441,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>>
>> hash->setkey = ahash_nosetkey;
>> hash->has_setkey = false;
>> -hash->export = ahash_no_export;
>> -hash->import = ahash_no_import;
>>
>> if (tfm->__crt_alg->cra_type != _ahash_type)
>> return crypto_init_shash_ops_async(tfm);
>> @@ -462,15 +450,13 @@ static int crypto_ahash_init_tfm(struct crypto_tfm
>> *tfm)
>> hash->final = alg->final;
>> hash->finup = alg->finup ?: ahash_def_finup;
>> hash->digest = alg->digest;
>> +hash->export = alg->export;
>> +hash->import = alg->import;
>>
>> if (alg->setkey) {
>> hash->setkey = alg->setkey;
>> hash->has_setkey = true;
>> }
>> -if (alg->export)
>> -hash->export = alg->export;
>> -if (alg->import)
>> -hash->import = alg->import;
>>
>> return 0;
>> }
>>
>
>
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
[PATCH 3/5] crypto: ux500/hash: Add empty export and import
Crypto framework requires export/import in async hash. If driver do not
implement them, wrapper functions in framework will be used, and it will
cause error during ahash alg registration (unless one disables crypto
internal tests). To make change in framework and expose this requirement,
I will remove wrappers from crypto/ahash.c , but this can broke code which
depends on them. Add empty hash export and import, with the same behaviour
as in framework and expose this directly in driver. This can also prevent
OOPS when config option in Cryptographic API 'Disable run-time self tests'
will be enabled.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
Acked-by: Linus Walleij <linus.wall...@linaro.org>
---
drivers/crypto/ux500/hash/hash_core.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/crypto/ux500/hash/hash_core.c
b/drivers/crypto/ux500/hash/hash_core.c
index 9acccad26928..2d0a677bcc76 100644
--- a/drivers/crypto/ux500/hash/hash_core.c
+++ b/drivers/crypto/ux500/hash/hash_core.c
@@ -1403,6 +1403,16 @@ static int ahash_sha256_digest(struct ahash_request *req)
return ret1 ? ret1 : ret2;
}
+static int ahash_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int ahash_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int hmac_sha1_init(struct ahash_request *req)
{
struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
@@ -1507,6 +1517,8 @@ static struct hash_algo_template hash_algs[] = {
.update = ahash_update,
.final = ahash_final,
.digest = ahash_sha1_digest,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA1_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1529,6 +1541,8 @@ static struct hash_algo_template hash_algs[] = {
.update = ahash_update,
.final = ahash_final,
.digest = ahash_sha256_digest,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA256_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1553,6 +1567,8 @@ static struct hash_algo_template hash_algs[] = {
.final = ahash_final,
.digest = hmac_sha1_digest,
.setkey = hmac_sha1_setkey,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA1_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1577,6 +1593,8 @@ static struct hash_algo_template hash_algs[] = {
.final = ahash_final,
.digest = hmac_sha256_digest,
.setkey = hmac_sha256_setkey,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA256_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
--
2.15.0
[PATCH 4/5] crypto: bfin_crc: Add empty hash export and import
Crypto framework requires export/import in async hash. If driver do not
implement them, wrapper functions in framework will be used, and it will
cause error during ahash alg registration (unless one disables crypto
internal tests). To make change in framework and expose this requirement,
I will remove wrappers from crypto/ahash.c , but this can broke code which
depends on them.
Add empty hash export and import, with the same behaviour as in framework
and expose this directly in driver. This can also prevent OOPS when config
option in Cryptographic API 'Disable run-time self tests' will be enabled.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
drivers/crypto/bfin_crc.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/crypto/bfin_crc.c b/drivers/crypto/bfin_crc.c
index a118b9bed669..65a8e07835e8 100644
--- a/drivers/crypto/bfin_crc.c
+++ b/drivers/crypto/bfin_crc.c
@@ -450,6 +450,16 @@ static int bfin_crypto_crc_digest(struct ahash_request
*req)
return bfin_crypto_crc_finup(req);
}
+static int bfin_crypto_crc_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int bfin_crypto_crc_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int bfin_crypto_crc_setkey(struct crypto_ahash *tfm, const u8 *key,
unsigned int keylen)
{
@@ -487,6 +497,8 @@ static struct ahash_alg algs = {
.final = bfin_crypto_crc_final,
.finup = bfin_crypto_crc_finup,
.digest = bfin_crypto_crc_digest,
+ .export = bfin_crypto_crc_noexport,
+ .import = bfin_crypto_crc_noimport,
.setkey = bfin_crypto_crc_setkey,
.halg.digestsize= CHKSUM_DIGEST_SIZE,
.halg.base = {
--
2.15.0
[PATCH 5/5] crypto: ahash.c: Require export/import in ahash
Export and import are mandatory in async hash. As drivers were
rewritten, drop empty wrappers and correct init of ahash transformation.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
crypto/ahash.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 3a35d67de7d9..c3cce508c1d4 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
return ahash_def_finup_finish1(req, err);
}
-static int ahash_no_export(struct ahash_request *req, void *out)
-{
- return -ENOSYS;
-}
-
-static int ahash_no_import(struct ahash_request *req, const void *in)
-{
- return -ENOSYS;
-}
-
static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
{
struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
@@ -451,8 +441,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = ahash_nosetkey;
hash->has_setkey = false;
- hash->export = ahash_no_export;
- hash->import = ahash_no_import;
if (tfm->__crt_alg->cra_type != _ahash_type)
return crypto_init_shash_ops_async(tfm);
@@ -462,15 +450,13 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->final = alg->final;
hash->finup = alg->finup ?: ahash_def_finup;
hash->digest = alg->digest;
+ hash->export = alg->export;
+ hash->import = alg->import;
if (alg->setkey) {
hash->setkey = alg->setkey;
hash->has_setkey = true;
}
- if (alg->export)
- hash->export = alg->export;
- if (alg->import)
- hash->import = alg->import;
return 0;
}
--
2.15.0
[PATCH v3 1/5] crypto: mxs-dcp: Add empty hash export and import
Crypto framework requires export/import in async hash. If driver do not
implement them, wrapper functions in framework will be used, and it will
cause error during ahash alg registration (unless one disables crypto
internal tests). To make change in framework and expose this requirement,
I will remove wrappers from crypto/ahash.c , but this can broke code which
depends on them. Add empty hash export and import, with the same behaviour
as in framework and expose this directly in driver. This can also prevent
OOPS when config option in Cryptographic API 'Disable run-time self tests'
will be enabled.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
drivers/crypto/mxs-dcp.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 764be3e6933c..a10c418d4e5c 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -759,6 +759,16 @@ static int dcp_sha_digest(struct ahash_request *req)
return dcp_sha_finup(req);
}
+static int dcp_sha_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int dcp_sha_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int dcp_sha_cra_init(struct crypto_tfm *tfm)
{
crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
@@ -829,6 +839,8 @@ static struct ahash_alg dcp_sha1_alg = {
.final = dcp_sha_final,
.finup = dcp_sha_finup,
.digest = dcp_sha_digest,
+ .import = dcp_sha_noimport,
+ .export = dcp_sha_noexport,
.halg = {
.digestsize = SHA1_DIGEST_SIZE,
.base = {
@@ -853,6 +865,8 @@ static struct ahash_alg dcp_sha256_alg = {
.final = dcp_sha_final,
.finup = dcp_sha_finup,
.digest = dcp_sha_digest,
+ .import = dcp_sha_noimport,
+ .export = dcp_sha_noexport,
.halg = {
.digestsize = SHA256_DIGEST_SIZE,
.base = {
--
2.15.0
[PATCH 2/5] crypto: n2_core: Add empty hash export and import
Crypto framework requires export/import in async hash. If driver do not
implement them, wrapper functions in framework will be used, and it will
cause error during ahash alg registration (unless one disables crypto
internal tests). To make change in framework and expose this requirement,
I will remove wrappers from crypto/ahash.c , but this can broke code which
depends on them. Add empty hash export and import, with the same behaviour
as in framework and expose this directly in driver. This can also prevent
OOPS when config option in Cryptographic API 'Disable run-time self tests'
will be enabled.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
drivers/crypto/n2_core.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
index 662e709812cc..80e9c842aad4 100644
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -359,6 +359,16 @@ static int n2_hash_async_finup(struct ahash_request *req)
return crypto_ahash_finup(>fallback_req);
}
+static int n2_hash_async_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int n2_hash_async_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int n2_hash_cra_init(struct crypto_tfm *tfm)
{
const char *fallback_driver_name = crypto_tfm_alg_name(tfm);
@@ -1467,6 +1477,8 @@ static int __n2_register_one_ahash(const struct
n2_hash_tmpl *tmpl)
ahash->final = n2_hash_async_final;
ahash->finup = n2_hash_async_finup;
ahash->digest = n2_hash_async_digest;
+ ahash->export = n2_hash_async_noexport;
+ ahash->import = n2_hash_async_noimport;
halg = >halg;
halg->digestsize = tmpl->digest_size;
--
2.15.0
[PATCH v3 0/5] crypto: ahash.c: Require export/import in ahash
First four patches add empty hash export and import functions to each driver, with the same behaviour as in crypto framework. The last one drops them from crypto framework. Last one for ahash.c depends on all previous. Changes in v3: added change for bfin_crc.c make this a patchset, instead of unreleated patches make commit message more descriptive Kamil Konieczny (5): crypto: mxs-dcp: Add empty hash export and import crypto: n2_core: Add empty hash export and import crypto: ux500/hash: Add empty export and import crypto: bfin_crc: Add empty hash export and import crypto: ahash.c: Require export/import in ahash crypto/ahash.c| 18 ++ drivers/crypto/bfin_crc.c | 12 drivers/crypto/mxs-dcp.c | 14 ++ drivers/crypto/n2_core.c | 12 drivers/crypto/ux500/hash/hash_core.c | 18 ++ 5 files changed, 58 insertions(+), 16 deletions(-) -- 2.15.0
Re: [PATCH] crypto: ux500/hash: Add empty export and import
Please drop this as I will resend it as part of patchset.
On 16.01.2018 17:32, Kamil Konieczny wrote:
> Crypto framework will require async hash export/import, so add empty
> functions to prevent OOPS.
>
> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
> ---
> drivers/crypto/ux500/hash/hash_core.c | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/crypto/ux500/hash/hash_core.c
> b/drivers/crypto/ux500/hash/hash_core.c
> index 9acccad26928..2d0a677bcc76 100644
> --- a/drivers/crypto/ux500/hash/hash_core.c
> +++ b/drivers/crypto/ux500/hash/hash_core.c
> @@ -1403,6 +1403,16 @@ static int ahash_sha256_digest(struct ahash_request
> *req)
> return ret1 ? ret1 : ret2;
> }
>
> +static int ahash_noimport(struct ahash_request *req, const void *in)
> +{
> + return -ENOSYS;
> +}
> +
> +static int ahash_noexport(struct ahash_request *req, void *out)
> +{
> + return -ENOSYS;
> +}
> +
> static int hmac_sha1_init(struct ahash_request *req)
> {
> struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> @@ -1507,6 +1517,8 @@ static struct hash_algo_template hash_algs[] = {
> .update = ahash_update,
> .final = ahash_final,
> .digest = ahash_sha1_digest,
> + .export = ahash_noexport,
> + .import = ahash_noimport,
> .halg.digestsize = SHA1_DIGEST_SIZE,
> .halg.statesize = sizeof(struct hash_ctx),
> .halg.base = {
> @@ -1529,6 +1541,8 @@ static struct hash_algo_template hash_algs[] = {
> .update = ahash_update,
> .final = ahash_final,
> .digest = ahash_sha256_digest,
> + .export = ahash_noexport,
> + .import = ahash_noimport,
> .halg.digestsize = SHA256_DIGEST_SIZE,
> .halg.statesize = sizeof(struct hash_ctx),
> .halg.base = {
> @@ -1553,6 +1567,8 @@ static struct hash_algo_template hash_algs[] = {
> .final = ahash_final,
> .digest = hmac_sha1_digest,
> .setkey = hmac_sha1_setkey,
> + .export = ahash_noexport,
> + .import = ahash_noimport,
> .halg.digestsize = SHA1_DIGEST_SIZE,
> .halg.statesize = sizeof(struct hash_ctx),
> .halg.base = {
> @@ -1577,6 +1593,8 @@ static struct hash_algo_template hash_algs[] = {
> .final = ahash_final,
> .digest = hmac_sha256_digest,
> .setkey = hmac_sha256_setkey,
> + .export = ahash_noexport,
> + .import = ahash_noimport,
> .halg.digestsize = SHA256_DIGEST_SIZE,
> .halg.statesize = sizeof(struct hash_ctx),
> .halg.base = {
>
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH] crypto: n2_core: Add empty hash export and import
Please drop this as I will resend it as part of patchset.
On 16.01.2018 17:18, Kamil Konieczny wrote:
> Crypto framework will require async hash export/import, so add empty
> functions to prevent OOPS.
>
> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
> ---
> drivers/crypto/n2_core.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
> index 662e709812cc..80e9c842aad4 100644
> --- a/drivers/crypto/n2_core.c
> +++ b/drivers/crypto/n2_core.c
> @@ -359,6 +359,16 @@ static int n2_hash_async_finup(struct ahash_request *req)
> return crypto_ahash_finup(>fallback_req);
> }
>
> +static int n2_hash_async_noimport(struct ahash_request *req, const void *in)
> +{
> + return -ENOSYS;
> +}
> +
> +static int n2_hash_async_noexport(struct ahash_request *req, void *out)
> +{
> + return -ENOSYS;
> +}
> +
> static int n2_hash_cra_init(struct crypto_tfm *tfm)
> {
> const char *fallback_driver_name = crypto_tfm_alg_name(tfm);
> @@ -1467,6 +1477,8 @@ static int __n2_register_one_ahash(const struct
> n2_hash_tmpl *tmpl)
> ahash->final = n2_hash_async_final;
> ahash->finup = n2_hash_async_finup;
> ahash->digest = n2_hash_async_digest;
> + ahash->export = n2_hash_async_noexport;
> + ahash->import = n2_hash_async_noimport;
>
> halg = >halg;
> halg->digestsize = tmpl->digest_size;
>
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH] crypto: mxs-dcp: Add empty hash export and import
Please drop this as I will resend it as part of patchset.
On 16.01.2018 17:16, Kamil Konieczny wrote:
> Crypto framework will require async hash export/import, so add empty
> functions to prevent OOPS.
>
> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
> ---
> drivers/crypto/mxs-dcp.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> index 764be3e6933c..a10c418d4e5c 100644
> --- a/drivers/crypto/mxs-dcp.c
> +++ b/drivers/crypto/mxs-dcp.c
> @@ -759,6 +759,16 @@ static int dcp_sha_digest(struct ahash_request *req)
> return dcp_sha_finup(req);
> }
>
> +static int dcp_sha_noimport(struct ahash_request *req, const void *in)
> +{
> + return -ENOSYS;
> +}
> +
> +static int dcp_sha_noexport(struct ahash_request *req, void *out)
> +{
> + return -ENOSYS;
> +}
> +
> static int dcp_sha_cra_init(struct crypto_tfm *tfm)
> {
> crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
> @@ -829,6 +839,8 @@ static struct ahash_alg dcp_sha1_alg = {
> .final = dcp_sha_final,
> .finup = dcp_sha_finup,
> .digest = dcp_sha_digest,
> + .import = dcp_sha_noimport,
> + .export = dcp_sha_noexport,
> .halg = {
> .digestsize = SHA1_DIGEST_SIZE,
> .base = {
> @@ -853,6 +865,8 @@ static struct ahash_alg dcp_sha256_alg = {
> .final = dcp_sha_final,
> .finup = dcp_sha_finup,
> .digest = dcp_sha_digest,
> + .import = dcp_sha_noimport,
> + .export = dcp_sha_noexport,
> .halg = {
> .digestsize = SHA256_DIGEST_SIZE,
> .base = {
>
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH] crypto: ux500/hash: Add empty export and import
On 18.01.2018 11:06, Linus Walleij wrote: > On Tue, Jan 16, 2018 at 5:32 PM, Kamil Konieczny > <k.koniec...@partner.samsung.com> wrote: > >> Crypto framework will require async hash export/import, so add empty >> functions to prevent OOPS. >> >> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> > > Acked-by: Linus Walleij <linus.wall...@linaro.org> > > But why isn't the framework code just checking the vtable for NULL? > > if (foo->fp) > foo->fp(bar); This will be inefficient, it should be checked once at ahash alg register, or with the old method by using wrapper -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH v2] crypto/ahash: Require export/import in ahash
On 16.01.2018 19:38, Kamil Konieczny wrote:
> Export and import were optional in async hash. As most drivers were
> rewritten, they become mandatory now, so correct init of ahash
> transformation.
>
> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
Please drop this patch, as there is one more driver needed for convert,
namely bfin_crc.c
I will also prepare this as patch series, to be sure that this patch is last
> ---
> This is resend of previous patch. As Bartlomiej Zolnierkiewicz pointed out,
> there are still three crypto drivers that didn't have export/import
> implemented:
>
> drivers/crypto/mxs-dcp.c
> drivers/crypto/n2_core.c
> drivers/crypto/ux500/hash/hash_core.c
>
> I have no documentation for them, so I sended patches with the behaviour taken
> from crypto framework, but maybe that hardware is capable of import/export,
> so proper implementation is possible. Unfortunatly, there is no maintainer
> for any of these files.
>
> Please take this patch after these remainig drivers will be patched.
>
> crypto/ahash.c | 18 ++
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/crypto/ahash.c b/crypto/ahash.c
> index 3a35d67de7d9..7a8906d5af53 100644
> --- a/crypto/ahash.c
> +++ b/crypto/ahash.c
> @@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
> return ahash_def_finup_finish1(req, err);
> }
>
> -static int ahash_no_export(struct ahash_request *req, void *out)
> -{
> - return -ENOSYS;
> -}
> -
> -static int ahash_no_import(struct ahash_request *req, const void *in)
> -{
> - return -ENOSYS;
> -}
> -
> static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
> {
> struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
> @@ -451,8 +441,8 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>
> hash->setkey = ahash_nosetkey;
> hash->has_setkey = false;
> - hash->export = ahash_no_export;
> - hash->import = ahash_no_import;
> + hash->export = alg->export;
> + hash->import = alg->import;
>
> if (tfm->__crt_alg->cra_type != _ahash_type)
> return crypto_init_shash_ops_async(tfm);
> @@ -467,10 +457,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
> hash->setkey = alg->setkey;
> hash->has_setkey = true;
> }
> - if (alg->export)
> - hash->export = alg->export;
> - if (alg->import)
> - hash->import = alg->import;
>
> return 0;
> }
>
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
[PATCH v2] crypto/ahash: Require export/import in ahash
Export and import were optional in async hash. As most drivers were
rewritten, they become mandatory now, so correct init of ahash
transformation.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
This is resend of previous patch. As Bartlomiej Zolnierkiewicz pointed out,
there are still three crypto drivers that didn't have export/import implemented:
drivers/crypto/mxs-dcp.c
drivers/crypto/n2_core.c
drivers/crypto/ux500/hash/hash_core.c
I have no documentation for them, so I sended patches with the behaviour taken
from crypto framework, but maybe that hardware is capable of import/export,
so proper implementation is possible. Unfortunatly, there is no maintainer
for any of these files.
Please take this patch after these remainig drivers will be patched.
crypto/ahash.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 3a35d67de7d9..7a8906d5af53 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
return ahash_def_finup_finish1(req, err);
}
-static int ahash_no_export(struct ahash_request *req, void *out)
-{
- return -ENOSYS;
-}
-
-static int ahash_no_import(struct ahash_request *req, const void *in)
-{
- return -ENOSYS;
-}
-
static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
{
struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
@@ -451,8 +441,8 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = ahash_nosetkey;
hash->has_setkey = false;
- hash->export = ahash_no_export;
- hash->import = ahash_no_import;
+ hash->export = alg->export;
+ hash->import = alg->import;
if (tfm->__crt_alg->cra_type != _ahash_type)
return crypto_init_shash_ops_async(tfm);
@@ -467,10 +457,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = alg->setkey;
hash->has_setkey = true;
}
- if (alg->export)
- hash->export = alg->export;
- if (alg->import)
- hash->import = alg->import;
return 0;
}
--
2.15.0
Re: [PATCH] crypto: mxs-dcp: Add empty hash export and import
On 16.01.2018 18:28, Fabio Estevam wrote: > Hi Kamil, > > On Tue, Jan 16, 2018 at 2:16 PM, Kamil Konieczny > <k.koniec...@partner.samsung.com> wrote: >> Crypto framework will require async hash export/import, so add empty >> functions to prevent OOPS. > > Which Oops exactly are you getting? None now, it is for preparation for patch removing export/import wrappers. > > Just booted 4.14.13 and the mxs-dcp driver does not even probe successfully: > > [2.455404] mxs-dcp 80028000.dcp: Failed to register sha1 hash! > [2.464042] mxs-dcp: probe of 80028000.dcp failed with error -22 > With this option turned on in config: crypto: Disable run-time self tests driver should load. Can you verify ? Btw, there is no maintainer for this file. -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH] crypto: mxs-dcp: Add empty hash export and import
On 16.01.2018 17:56, Marek Vasut wrote: > On 01/16/2018 05:16 PM, Kamil Konieczny wrote: >> Crypto framework will require async hash export/import, so add empty >> functions to prevent OOPS. > > Shouldn't this be handled on the subsystem level with some > > if (foo->bar) > foo->bar(); > > instead? I am sorry, I should write more elaborate description for patch. It is handled by subsystem. Most drivers have them, and testmgr is testing for export/import and drivers without them fail internal crypto tests, so I prepared patch which removed these two wrappers from crypto framework. In summary: export/import are now required, so crypto framework can work properly. -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH] crypto: ux500/hash: Add empty export and import
Crypto framework will require async hash export/import, so add empty
functions to prevent OOPS.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
drivers/crypto/ux500/hash/hash_core.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/crypto/ux500/hash/hash_core.c
b/drivers/crypto/ux500/hash/hash_core.c
index 9acccad26928..2d0a677bcc76 100644
--- a/drivers/crypto/ux500/hash/hash_core.c
+++ b/drivers/crypto/ux500/hash/hash_core.c
@@ -1403,6 +1403,16 @@ static int ahash_sha256_digest(struct ahash_request *req)
return ret1 ? ret1 : ret2;
}
+static int ahash_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int ahash_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int hmac_sha1_init(struct ahash_request *req)
{
struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
@@ -1507,6 +1517,8 @@ static struct hash_algo_template hash_algs[] = {
.update = ahash_update,
.final = ahash_final,
.digest = ahash_sha1_digest,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA1_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1529,6 +1541,8 @@ static struct hash_algo_template hash_algs[] = {
.update = ahash_update,
.final = ahash_final,
.digest = ahash_sha256_digest,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA256_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1553,6 +1567,8 @@ static struct hash_algo_template hash_algs[] = {
.final = ahash_final,
.digest = hmac_sha1_digest,
.setkey = hmac_sha1_setkey,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA1_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
@@ -1577,6 +1593,8 @@ static struct hash_algo_template hash_algs[] = {
.final = ahash_final,
.digest = hmac_sha256_digest,
.setkey = hmac_sha256_setkey,
+ .export = ahash_noexport,
+ .import = ahash_noimport,
.halg.digestsize = SHA256_DIGEST_SIZE,
.halg.statesize = sizeof(struct hash_ctx),
.halg.base = {
--
2.15.0
[PATCH] crypto: n2_core: Add empty hash export and import
Crypto framework will require async hash export/import, so add empty
functions to prevent OOPS.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
drivers/crypto/n2_core.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
index 662e709812cc..80e9c842aad4 100644
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -359,6 +359,16 @@ static int n2_hash_async_finup(struct ahash_request *req)
return crypto_ahash_finup(>fallback_req);
}
+static int n2_hash_async_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int n2_hash_async_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int n2_hash_cra_init(struct crypto_tfm *tfm)
{
const char *fallback_driver_name = crypto_tfm_alg_name(tfm);
@@ -1467,6 +1477,8 @@ static int __n2_register_one_ahash(const struct
n2_hash_tmpl *tmpl)
ahash->final = n2_hash_async_final;
ahash->finup = n2_hash_async_finup;
ahash->digest = n2_hash_async_digest;
+ ahash->export = n2_hash_async_noexport;
+ ahash->import = n2_hash_async_noimport;
halg = >halg;
halg->digestsize = tmpl->digest_size;
--
2.15.0
[PATCH] crypto: mxs-dcp: Add empty hash export and import
Crypto framework will require async hash export/import, so add empty
functions to prevent OOPS.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
drivers/crypto/mxs-dcp.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index 764be3e6933c..a10c418d4e5c 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -759,6 +759,16 @@ static int dcp_sha_digest(struct ahash_request *req)
return dcp_sha_finup(req);
}
+static int dcp_sha_noimport(struct ahash_request *req, const void *in)
+{
+ return -ENOSYS;
+}
+
+static int dcp_sha_noexport(struct ahash_request *req, void *out)
+{
+ return -ENOSYS;
+}
+
static int dcp_sha_cra_init(struct crypto_tfm *tfm)
{
crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
@@ -829,6 +839,8 @@ static struct ahash_alg dcp_sha1_alg = {
.final = dcp_sha_final,
.finup = dcp_sha_finup,
.digest = dcp_sha_digest,
+ .import = dcp_sha_noimport,
+ .export = dcp_sha_noexport,
.halg = {
.digestsize = SHA1_DIGEST_SIZE,
.base = {
@@ -853,6 +865,8 @@ static struct ahash_alg dcp_sha256_alg = {
.final = dcp_sha_final,
.finup = dcp_sha_finup,
.digest = dcp_sha_digest,
+ .import = dcp_sha_noimport,
+ .export = dcp_sha_noexport,
.halg = {
.digestsize = SHA256_DIGEST_SIZE,
.base = {
--
2.15.0
[PATCH] crypto: testmgr.c: test misuse of result in ahash
Async hash operations can use result pointer in final/finup/digest,
but not in init/update/export/import, so test it for misuse.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
Tested with crypto run-time self test on Odroid-U3 with Exynos 4412 CPU
with insmod s5p-sss.ko
crypto/testmgr.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 44a85d4b3561..d5e23a142a04 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -177,6 +177,18 @@ static void testmgr_free_buf(char *buf[XBUFSIZE])
free_page((unsigned long)buf[i]);
}
+static int ahash_guard_result(char *result, char c, int size)
+{
+ int i;
+
+ for (i = 0; i < size; i++) {
+ if (result[i] != c)
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int ahash_partial_update(struct ahash_request **preq,
struct crypto_ahash *tfm, const struct hash_testvec *template,
void *hash_buff, int k, int temp, struct scatterlist *sg,
@@ -186,6 +198,7 @@ static int ahash_partial_update(struct ahash_request **preq,
struct ahash_request *req;
int statesize, ret = -EINVAL;
static const unsigned char guard[] = { 0x00, 0xba, 0xad, 0x00 };
+ int digestsize = crypto_ahash_digestsize(tfm);
req = *preq;
statesize = crypto_ahash_statesize(
@@ -196,12 +209,19 @@ static int ahash_partial_update(struct ahash_request
**preq,
goto out_nostate;
}
memcpy(state + statesize, guard, sizeof(guard));
+ memset(result, 1, digestsize);
ret = crypto_ahash_export(req, state);
WARN_ON(memcmp(state + statesize, guard, sizeof(guard)));
if (ret) {
pr_err("alg: hash: Failed to export() for %s\n", algo);
goto out;
}
+ ret = ahash_guard_result(result, 1, digestsize);
+ if (ret) {
+ pr_err("alg: hash: Failed, export used req->result for %s\n",
+ algo);
+ goto out;
+ }
ahash_request_free(req);
req = ahash_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -221,6 +241,12 @@ static int ahash_partial_update(struct ahash_request
**preq,
pr_err("alg: hash: Failed to import() for %s\n", algo);
goto out;
}
+ ret = ahash_guard_result(result, 1, digestsize);
+ if (ret) {
+ pr_err("alg: hash: Failed, import used req->result for %s\n",
+ algo);
+ goto out;
+ }
ret = crypto_wait_req(crypto_ahash_update(req), wait);
if (ret)
goto out;
@@ -316,18 +342,31 @@ static int __test_hash(struct crypto_ahash *tfm,
goto out;
}
} else {
+ memset(result, 1, digest_size);
ret = crypto_wait_req(crypto_ahash_init(req), );
if (ret) {
pr_err("alg: hash: init failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
+ ret = ahash_guard_result(result, 1, digest_size);
+ if (ret) {
+ pr_err("alg: hash: init failed on test %d "
+ "for %s: used req->result\n", j, algo);
+ goto out;
+ }
ret = crypto_wait_req(crypto_ahash_update(req), );
if (ret) {
pr_err("alg: hash: update failed on test %d "
"for %s: ret=%d\n", j, algo, -ret);
goto out;
}
+ ret = ahash_guard_result(result, 1, digest_size);
+ if (ret) {
+ pr_err("alg: hash: update failed on test %d "
+ "for %s: used req->result\n", j, algo);
+ goto out;
+ }
ret = crypto_wait_req(crypto_ahash_final(req), );
if (ret) {
pr_err("alg: hash: final failed on test %d "
--
2.15.0
[RFT PATCH] crypto: ahash.c: Require export/import in ahash
Export and import were optional in async hash. As drivers were rewritten,
they become mandatory now, so correct init of ahash transformation.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
Tested with crypto run-time self test on Odroid-U3 with Exynos 4412 CPU,
with insmod s5p-sss.ko
Please test with other crypto hash drivers.
crypto/ahash.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 3a35d67de7d9..7a8906d5af53 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
return ahash_def_finup_finish1(req, err);
}
-static int ahash_no_export(struct ahash_request *req, void *out)
-{
- return -ENOSYS;
-}
-
-static int ahash_no_import(struct ahash_request *req, const void *in)
-{
- return -ENOSYS;
-}
-
static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
{
struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
@@ -451,8 +441,8 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = ahash_nosetkey;
hash->has_setkey = false;
- hash->export = ahash_no_export;
- hash->import = ahash_no_import;
+ hash->export = alg->export;
+ hash->import = alg->import;
if (tfm->__crt_alg->cra_type != _ahash_type)
return crypto_init_shash_ops_async(tfm);
@@ -467,10 +457,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
hash->setkey = alg->setkey;
hash->has_setkey = true;
}
- if (alg->export)
- hash->export = alg->export;
- if (alg->import)
- hash->import = alg->import;
return 0;
}
--
2.15.0
Re: [PATCH] crypto: clear htmldocs build warnings for crypto/hash
On 08.01.2018 16:56, Herbert Xu wrote: > On Mon, Jan 08, 2018 at 02:11:21PM +0100, Kamil Konieczny wrote: >> >> >> On 07.01.2018 00:01, Tobin C. Harding wrote: >>> SPHINX build emits multiple warnings of kind: >>> >>> warning: duplicate section name 'Note' >>> >>> (when building kernel via make target 'htmldocs') >>> >>> This is caused by repeated use of comments of form: >>> >>> * Note: soau soaeusoa uoe >>> >>> We can change the format without loss of clarity and clear the build >>> warnings. >>> >>> Add '**[mandatory]**' or '**[optional]**' as kernel-doc field element >>> description prefix >>> >>> This renders in HTML as (prefixes in bold) >>> >>> final >>> [mandatory] Retrieve result from the driver. This function finalizes the >>> transformation and retrieves the resulting hash from the driver and >>> pushes it back to upper layers. No data processing happens at this >>> point unless hardware requires it to finish the transformation (then >>> the data buffered by the device driver is processed). >>> >>> Signed-off-by: Tobin C. Harding <m...@tobin.cc> >>> --- >>> >>> This patch begs the question why the other members of struct ahash_alg >>> are not marked? Some are marked 'optional' some 'mandatory'. It would >>> seem that if the marking were necessary for some members it is necessary >>> for all to eliminate ambiguity? >>> >>> thanks >> >> import, export are optional > > No import/export must be implemented for all hashes. Is it mandatory for both async hash and shash ? in crypto/ahash.c in function static int crypto_ahash_init_tfm(struct crypto_tfm *tfm) there is: hash->export = ahash_no_export; hash->import = ahash_no_import; and later in the same function: if (alg->export) hash->export = alg->export; if (alg->import) hash->import = alg->import; -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH v2] crypto: Add myself as co-maintainer for s5p-sss.c
Add myself as co-maintainer for Samsung Security SubSystem driver. I have added major functionality to the driver [hash acceleration], I have access to documentation and to hardware for testing, I can also dedicate some of my paid time for reviewing and verifying changes to the driver. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- Changes: v2: clarify reasons for co-maintenance. MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index aa71ab52fd76..3f6cadf2e087 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11982,6 +11982,7 @@ F: drivers/media/i2c/s5k5baf.c SAMSUNG S5P Security SubSystem (SSS) DRIVER M: Krzysztof Kozlowski <k...@kernel.org> M: Vladimir Zapolskiy <v...@mleia.com> +M: Kamil Konieczny <k.koniec...@partner.samsung.com> L: linux-crypto@vger.kernel.org L: linux-samsung-...@vger.kernel.org S: Maintained -- 2.15.0
Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided
Hi Herbert, On 01.12.2017 11:36, Herbert Xu wrote: > On Fri, Dec 01, 2017 at 11:18:30AM +0100, Kamil Konieczny wrote: >> >> Herbert, is it possible for every init/update that areq->result can be NULL, >> and only for final/update/digit user set it to actual memory ? >> testmgr.c can check if hash update writes into areq->result and if yes, >> then test fails ? > > Yes we should modify testmgr to check that. I can write patch for it. Should it WARN_ON or treat it as error ? -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided
Hi Antoine,
On 01.12.2017 11:24, Antoine Tenart wrote:
> Hi Kamil,
>
> On Fri, Dec 01, 2017 at 11:18:30AM +0100, Kamil Konieczny wrote:
>> On 01.12.2017 09:11, Antoine Tenart wrote:
>>> - Other drivers are doing this check (grep "if (!req->result)" or
>>> "if (req->result)" to see some of them).
>>> - I see at least one commit fixing the exact same issue I'm facing here,
>>> 393897c5156a415533ff85aa381458840417b032:
>>>
>>> crypto: ccp - Check for caller result area before using it
>>>
>>> For a hash operation, the caller doesn't have to supply a result
>>> area on every call so don't use it / update it if it hasn't
>>> been supplied.
>>
>> Do you set last_req true for digest/finup/final ? If yes,
>> then you need to copy result only when it is true,
>>
>> if (sreq->last_req) {
>> result_sz = crypto_ahash_digestsize(ahash);
>> memcpy(sreq->state, areq->result, result_sz);
>> }
>
> Yes the last_req flag is set for the last request, so when
> digest/finup/final are called. But no we can't copy the result into the
> state based on this as an user might want to perform multiple updates,
> then export the context, to import it again before sending more updates.
IMHO set them to false in hash update and init, set finish false and last_req
true
for hash final, and set both true for hash finup and digest.
As Herbert tells in https://www.spinics.net/lists/linux-crypto/msg28658.html
you should support scenario export + update/finup, so basically export is
reading
state but it do not halt your hash driver.
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided
Hi All,
On 01.12.2017 09:11, Antoine Tenart wrote:
> Hi Herbert,
>
> On Fri, Dec 01, 2017 at 11:31:09AM +1100, Herbert Xu wrote:
>> On Thu, Nov 30, 2017 at 10:19:26AM +0100, Kamil Konieczny wrote:
>>>
>>> can the driver get request for final/finup/digest with null req->result ?
>>> If yes (?), such checks can be done before any hardware processing, saving
>>> time,
>>> for example:
>>
>> This should not be possible through any user-space facing API.
>>
>> If a kernel API user did this then they're just shooting themselves
>> in the foot.
>>
>> So unless there is a valida call path that leads to this then I
>> would say that there is nothing to fix.
>
> I agree this should not be the case.
>
> But:
> - Other drivers are doing this check (grep "if (!req->result)" or
> "if (req->result)" to see some of them).
> - I see at least one commit fixing the exact same issue I'm facing here,
> 393897c5156a415533ff85aa381458840417b032:
>
> crypto: ccp - Check for caller result area before using it
>
> For a hash operation, the caller doesn't have to supply a result
> area on every call so don't use it / update it if it hasn't
> been supplied.
Herbert, is it possible for every init/update that areq->result can be NULL,
and only for final/update/digit user set it to actual memory ?
testmgr.c can check if hash update writes into areq->result and if yes,
then test fails ?
As I understand this, when crypto api user allocates ahash_request,
crypto allocates memory for itself _plus_ for driver's context. This allocated
ahash_request is "handle" for all subsequent updates/export/import,
and for last final/finup, so I do not need to copy hash state into areq->result,
but keep it whole time in context, in your code in sreq:
struct safexcel_ahash_req *sreq = ahash_request_ctx(areq);
so here sreq is async hash request context.
Do you set last_req true for digest/finup/final ? If yes,
then you need to copy result only when it is true,
if (sreq->last_req) {
result_sz = crypto_ahash_digestsize(ahash);
memcpy(sreq->state, areq->result, result_sz);
}
I do not read all your code though, so I can be wrong here.
> I'm not entirely sure what was the code path that leads to this, I'll
> reproduce the issue and try to understand what is going on (I clearly
> recall having this crash though).
>
> The crypto API does not enforce this somehow, and this should probably
> be fixed. That might break some users. But it was seen as a valid use
> for some, so we should probably fix this in previous versions of the
> driver anyway.
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided
Hi Antoine,
On 30.11.2017 10:29, Antoine Tenart wrote:
> Hi Kamil,
>
> On Thu, Nov 30, 2017 at 10:19:26AM +0100, Kamil Konieczny wrote:
>> On 28.11.2017 16:42, Antoine Tenart wrote:
>>> The patch fixes the ahash support by only updating the result buffer
>>> when provided. Otherwise the driver could crash with NULL pointer
>>> exceptions, because the ahash caller isn't required to supply a result
>>> buffer on all calls.
>>
>> Can you point to bug crush report ?
>
> Do you want the crash dump? (It'll only be a "normal" NULL pointer
> dereference).
Ah I see, in this case not.
>>> Fixes: 1b44c5a60c13 ("crypto: inside-secure - add SafeXcel EIP197 crypto
>>> engine driver")
>>> Signed-off-by: Antoine Tenart <antoine.ten...@free-electrons.com>
>>> ---
>>> drivers/crypto/inside-secure/safexcel_hash.c | 7 ++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/crypto/inside-secure/safexcel_hash.c
>>> b/drivers/crypto/inside-secure/safexcel_hash.c
>>> index 6135c9f5742c..424f4c5d4d25 100644
>>> --- a/drivers/crypto/inside-secure/safexcel_hash.c
>>> +++ b/drivers/crypto/inside-secure/safexcel_hash.c
>>> @@ -150,7 +150,12 @@ static int safexcel_handle_req_result(struct
>>> safexcel_crypto_priv *priv, int rin
>>>
>>> if (sreq->finish)
>>> result_sz = crypto_ahash_digestsize(ahash);
>>> - memcpy(sreq->state, areq->result, result_sz);
>> [...]
>> can the driver get request for final/finup/digest with null req->result ?
>
> I don't think that can happen. But having an update called without
> req->result provided is a valid call (though it's not well documented).
so maybe:
if (sreq->finish) {
result_sz = crypto_ahash_digestsize(ahash);
memcpy(sreq->state, areq->result, result_sz);
}
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH v2 3/4] crypto: inside-secure - only update the result buffer when provided
Hi Antoine,
On 28.11.2017 16:42, Antoine Tenart wrote:
> The patch fixes the ahash support by only updating the result buffer
> when provided. Otherwise the driver could crash with NULL pointer
> exceptions, because the ahash caller isn't required to supply a result
> buffer on all calls.
Can you point to bug crush report ?
> Fixes: 1b44c5a60c13 ("crypto: inside-secure - add SafeXcel EIP197 crypto
> engine driver")
> Signed-off-by: Antoine Tenart <antoine.ten...@free-electrons.com>
> ---
> drivers/crypto/inside-secure/safexcel_hash.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/inside-secure/safexcel_hash.c
> b/drivers/crypto/inside-secure/safexcel_hash.c
> index 6135c9f5742c..424f4c5d4d25 100644
> --- a/drivers/crypto/inside-secure/safexcel_hash.c
> +++ b/drivers/crypto/inside-secure/safexcel_hash.c
> @@ -150,7 +150,12 @@ static int safexcel_handle_req_result(struct
> safexcel_crypto_priv *priv, int rin
>
> if (sreq->finish)
> result_sz = crypto_ahash_digestsize(ahash);
> - memcpy(sreq->state, areq->result, result_sz);
> +
> + /* The called isn't required to supply a result buffer. Updated it only
> + * when provided.
> + */
> + if (areq->result)
> + memcpy(sreq->state, areq->result, result_sz);
>
> dma_unmap_sg(priv->dev, areq->src,
>sg_nents_for_len(areq->src, areq->nbytes), DMA_TO_DEVICE);
>
can the driver get request for final/finup/digest with null req->result ?
If yes (?), such checks can be done before any hardware processing, saving time,
for example:
int hash_final(struct ahash_request *areq)
{
if (!areq->result)
return -EINVAL;
/* normal processing here */
}
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
[PATCH] crypto: Add myself as co-maintainer for s5p-sss.c
Add myself as co-maintainer for Samsung Security SubSystem driver. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index aa71ab52fd76..3f6cadf2e087 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11982,6 +11982,7 @@ F: drivers/media/i2c/s5k5baf.c SAMSUNG S5P Security SubSystem (SSS) DRIVER M: Krzysztof Kozlowski <k...@kernel.org> M: Vladimir Zapolskiy <v...@mleia.com> +M: Kamil Konieczny <k.koniec...@partner.samsung.com> L: linux-crypto@vger.kernel.org L: linux-samsung-...@vger.kernel.org S: Maintained -- 2.15.0
Re: Question about ahash export and import
On 27.09.2017 11:08, Herbert Xu wrote: > On Tue, Sep 26, 2017 at 01:09:05PM +0200, Kamil Konieczny wrote: >> >> Can import() be called without _any_ init(), for example >> after reboot of machine ? Is following scenario valid: > > Of course it can. import must restore the state of the request > to that at the time when export was called. > > import does not need to be called after init. init simply resets > the hash state for new update/final calls. > [...] I have more questions, this time about HMAC export/import. In doc, there is stated that key can be copied into context transformation. Should .export copy request context _and_ key used ? Or not ? When processing requests, can I assume the key in transformation is const until .final ? Is is possible to have okey derived from different key then ikey ? I think not. -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH v8 2/2] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are needed for fallback. Acked-by: Vladimir Zapolskiy <v...@mleia.com> Reviewed-by: Krzysztof Kozlowski <k...@kernel.org> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1406 +- 2 files changed, 1410 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 4b75084fabad..dea4d33d9c7f 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index dfae1865c384..142c6020cec7 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,14 +1,16 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include @@ -30,28 +32,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSET BIT(3) #define SSS_FCINTENSET_BTDMAINTENSET BIT(2) #define SSS_FCINTENSET_HRDMAINTENSET BIT(1) #define SSS_FCINTENSET_PKDMAINTENSET BIT(0) #define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7) +#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5) #define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) #define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) #define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) #define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) #define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_HPARTINTPBIT(7) +#define SSS_FCINTPEND_HDONEINTPBIT(5) #define SSS_FCINTPEND_BRDMAINTPBIT(3) #define SSS_FCINTPEND_BTDMAINTPBIT(2) #define SSS_FCINTPEND_HRDMAINTPBIT(1) @@ -72,6 +87,7 @@ #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) #define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) #define SSS_HASHIN_CIPHER_OUTPUT _SBF(0, 0x02) +#define SSS_HASHIN_MASK_SBF(0, 0x03) #define SSS_REG_FCBRDMAS 0x0020 #define SSS_REG_FCBRDMAL 0x0024 @@ -146,9 +162,80 @@ #define AES_KEY_LEN16 #define CRYPTO_QUEUE_LEN 1 +/* HASH registers */ +#define SSS_REG_HASH_CTRL
[PATCH v8 1/2] crypto: s5p-sss: Change spaces to tabs
Change #define lines to use tabs consistently. Acked-by: Vladimir Zapolskiy <v...@mleia.com> Reviewed-by: Krzysztof Kozlowski <k...@kernel.org> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- drivers/crypto/s5p-sss.c | 190 +++ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..dfae1865c384 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -30,98 +30,98 @@ #include #include -#define _SBF(s, v) ((v) << (s)) +#define _SBF(s, v) ((v) << (s)) /* Feed control registers */ -#define SSS_REG_FCINTSTAT 0x -#define SSS_FCINTSTAT_BRDMAINT BIT(3) -#define SSS_FCINTSTAT_BTDMAINT BIT(2) -#define SSS_FCINTSTAT_HRDMAINT BIT(1) -#define SSS_FCINTSTAT_PKDMAINT BIT(0) - -#define SSS_REG_FCINTENSET 0x0004 -#define SSS_FCINTENSET_BRDMAINTENSETBIT(3) -#define SSS_FCINTENSET_BTDMAINTENSETBIT(2) -#define SSS_FCINTENSET_HRDMAINTENSETBIT(1) -#define SSS_FCINTENSET_PKDMAINTENSETBIT(0) - -#define SSS_REG_FCINTENCLR 0x0008 -#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3) -#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2) -#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1) -#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0) - -#define SSS_REG_FCINTPEND 0x000C -#define SSS_FCINTPEND_BRDMAINTP BIT(3) -#define SSS_FCINTPEND_BTDMAINTP BIT(2) -#define SSS_FCINTPEND_HRDMAINTP BIT(1) -#define SSS_FCINTPEND_PKDMAINTP BIT(0) - -#define SSS_REG_FCFIFOSTAT 0x0010 -#define SSS_FCFIFOSTAT_BRFIFOFULBIT(7) -#define SSS_FCFIFOSTAT_BRFIFOEMPBIT(6) -#define SSS_FCFIFOSTAT_BTFIFOFULBIT(5) -#define SSS_FCFIFOSTAT_BTFIFOEMPBIT(4) -#define SSS_FCFIFOSTAT_HRFIFOFULBIT(3) -#define SSS_FCFIFOSTAT_HRFIFOEMPBIT(2) -#define SSS_FCFIFOSTAT_PKFIFOFULBIT(1) -#define SSS_FCFIFOSTAT_PKFIFOEMPBIT(0) - -#define SSS_REG_FCFIFOCTRL 0x0014 -#define SSS_FCFIFOCTRL_DESSEL BIT(2) -#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) -#define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01) -#define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02) - -#define SSS_REG_FCBRDMAS0x0020 -#define SSS_REG_FCBRDMAL0x0024 -#define SSS_REG_FCBRDMAC0x0028 -#define SSS_FCBRDMAC_BYTESWAP BIT(1) -#define SSS_FCBRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCBTDMAS0x0030 -#define SSS_REG_FCBTDMAL0x0034 -#define SSS_REG_FCBTDMAC0x0038 -#define SSS_FCBTDMAC_BYTESWAP BIT(1) -#define SSS_FCBTDMAC_FLUSH BIT(0) - -#define SSS_REG_FCHRDMAS0x0040 -#define SSS_REG_FCHRDMAL0x0044 -#define SSS_REG_FCHRDMAC0x0048 -#define SSS_FCHRDMAC_BYTESWAP BIT(1) -#define SSS_FCHRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAS0x0050 -#define SSS_REG_FCPKDMAL0x0054 -#define SSS_REG_FCPKDMAC0x0058 -#define SSS_FCPKDMAC_BYTESWAP BIT(3) -#define SSS_FCPKDMAC_DESCENDBIT(2) -#define SSS_FCPKDMAC_TRANSMIT BIT(1) -#define SSS_FCPKDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAO0x005C +#define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_BRDMAINT BIT(3) +#define SSS_FCINTSTAT_BTDMAINT BIT(2) +#define SSS_FCINTSTAT_HRDMAINT BIT(1) +#define SSS_FCINTSTAT_PKDMAINT BIT(0) + +#define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_BRDMAINTENSET BIT(3) +#define SSS_FCINTENSET_BTDMAINTENSET BIT(2) +#define SSS_FCINTENSET_HRDMAINTENSET BIT(1) +#define SSS_FCINTENSET_PKDMAINTENSET BIT(0) + +#define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) +#define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) +#define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) +#define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) + +#define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_BRDMAINTPBIT(3) +#define SSS_FCINTPEND_BTDMAINTPBIT(2) +#define SSS_FCINTPEND_HRDMAINTPBIT(1) +#define SSS_FCINTPEND_PKDMAINTPBIT(0) + +#define SSS_REG_FCFIFOSTAT 0x0010 +#define SSS_FCFIFOSTAT_BRFIFOFUL BIT(7) +#define SSS_FCFIFOSTAT_BRFIFOEMP BIT(6) +#define SSS_FCFIFOSTAT_BTFIFOFUL BIT(5) +#define SSS_FCFIFOSTAT_BTFIFOEMP BIT(4) +#define SSS_FCFIFOSTAT_HRFIFOFUL BIT(3) +#define SSS_FCFIFOSTAT_HRFIFOEMP BIT(2) +#define SSS_FCFIFOSTAT_PKFIFOFUL BIT(1) +#define SSS_FCFIFOSTAT_PKFIFOEMP BIT(0) + +#define SSS_REG_FCFIFOCTRL 0x0014 +#define SSS_FCFIFOCTRL_D
[PATCH v8 0/2] crypto: s5p-sss: Add HASH support for Exynos
First patch change spaces to tabs, second adds HASH support for Exynos. Changes: version 8: - fixes suggested by Vladimir Zapolskiy: drop first condition check in s5p_hash_import, delete unused include delay.h, fix typo in commit message, fix descriptions of struct s5p_hash_reqctx and function s5p_hash_final() version 7: - fix ifdef into if(IS_ENABLED()) as suggested by Krzysztof Kozlowski version 6: - fixes suggested by Vladimir Zapolskiy: change HASH_OP enum into bool, fix comments, change int into unsigned int in several functions, change some functions to return void, remove unnecessary parentheses in s5p_hash_import, replace rctx with ctx for request context, drop some dd vars and use tctx->dd instead, simplify s5p_hash_rx, s5p_hash_copy_result and s5p_hash_set_flow, change int final into bool final, reoder some declarations, split patch into two - rewrite and fix while loop in s5p_hash_copy_sg_lists - rewrite while loop in s5p_hash_prepare_sgs version 5: - fix suggested by Krzysztof Kozlowski: change defines HASH_OP into enum, fix comments version 4: - fixes suggested by Krzysztof Kozlowski: reformat comments, convert context flags into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in place of ifdef, remove sss_hash_algs_info and simplify register and deregister HASH algs version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments Kamil Konieczny (2): crypto: s5p-sss: Change spaces to tabs crypto: s5p-sss: Add HASH support for Exynos drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1596 +++--- 2 files changed, 1505 insertions(+), 105 deletions(-) -- 2.14.1.536.g6867272d5b56
Re: [PATCH v7 2/2] crypto: s5p-sss: Add HASH support for Exynos
Hi Vladimir,
Thank you for review, I will apply almost all of your remarks,
see answers below.
On 22.10.2017 12:18, Vladimir Zapolskiy wrote:
> Hi Kamil,
>
> thank you for updates, I have just a few more comments.
>
> On 10/17/2017 02:28 PM, Kamil Konieczny wrote:
>> [...]
>> - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH
>> as they are nedded for fallback.
>
> Typo, s/nedded/needed/
ok, Thank you for spotting this.
> [snip]
>
>>
>> #include
>> #include
>> +#include
>
> I can not find which interfaces from linux/delay.h are needed.
> I suppose it should not be added.
>
> [snip]
Yes, you are right, I will delete this 'include delay.h'
>> -static struct s5p_aes_dev *s5p_dev;
>> +/**
>> + * struct s5p_hash_reqctx - HASH request context
>> + * @dev:Associated device
>
> dev or dd?
dd
>> + * @op_update: Current request operation (OP_UPDATE or OP_FINAL)
>> + * @digcnt: Number of bytes processed by HW (without buffer[] ones)
>> + * @digest: Digest message or IV for partial result
>> + * @nregs: Number of HW registers for digest or IV read/write
>> + * @engine: Bits for selecting type of HASH in SSS block
>> + * @sg: sg for DMA transfer
>> + * @sg_len: Length of sg for DMA transfer
>> + * @sgl[]: sg for joining buffer and req->src scatterlist
>> + * @skip: Skip offset in req->src for current op
>> + * @total: Total number of bytes for current request
>> + * @finup: Keep state for finup or final.
>> + * @error: Keep track of error.
>> + * @bufcnt: Number of bytes holded in buffer[]
>> + * @buffer[]: For byte(s) from end of req->src in UPDATE op
>> + */
>> +struct s5p_hash_reqctx {
>> +struct s5p_aes_dev *dd;
>> +boolop_update;
>> +
>
> [snip]
>
>> +
>> +/**
>> + * s5p_hash_shash_digest() - calculate shash digest
>> + * @tfm:crypto transformation
>> + * @flags: tfm flags
>> + * @data: input data
>> + * @len:length of data
>> + * @out:output buffer
>> + */
>> +static int s5p_hash_shash_digest(struct crypto_shash *tfm, u32 flags,
>> + const u8 *data, unsigned int len, u8 *out)
>> +{
>> +SHASH_DESC_ON_STACK(shash, tfm);
>
> Just for information, this line produces a spatch warning:
>
> drivers/crypto/s5p-sss.c:1534:9: warning: Variable length array is used.
>
> I think it can be ignored.
I also think it can be ignored, it uses 104 bytes on stack in case of SHA256 ,
sizeof struct sha256_state for SW generic implementation, found in
include/crypto/sha.h
>> +
>> +shash->tfm = tfm;
>> +shash->flags = flags & ~CRYPTO_TFM_REQ_MAY_SLEEP;
>> +
>> +return crypto_shash_digest(shash, data, len, out);
>> +}
>> +
>
> [snip]
>
>> +/**
>> + * s5p_hash_final() - close up hash and calculate digest
>> + * @req:AHASH request
>> + *
>> + * Note: in final req->src do not have any data, and req->nbytes can be
>> + * non-zero.
>> + *
>> + * If there were no input data processed yet and the buffered hash data is
>> + * less than BUFLEN (64) then calculate the final hash immediately by using
>> + * SW algorithm fallback.
>> + *
>> + * Otherwise enqueues the current AHASH request with OP_FINAL operation op
>> + * and finalize hash message in HW. Note that if digcnt!=0 then there were
>> + * previous update op, so there are always some buffered bytes in
>> ctx->buffer,
>> + * which means that ctx->bufcnt!=0
>> + *
>> + * Returns:
>> + * 0 if the request has been processed immediately,
>> + * -EINPROGRESS if the operation has been queued for later execution or is
>> set
>> + * to processing by HW,
>> + * -EBUSY if queue is full and request should be resubmitted later, other
>> + * negative values on error.
>
> Do you want to add -EINVAL into the list also?
Here I made bad formatting, it should read:
* -EBUSY if queue is full and request should be resubmitted later,
* other negative values on error.
I do not want to specify explicitly all error negative codes here, as it also
uses
s5p_hash_enqueue and these return codes are referenced from other places. I
intended
to listing success values, 0, -EINPROGRESS, then explaining -EBUSY, then adding
all
other negative as error. The other values can be -EINVAL, -ENOMEM or maybe
other, when
this module gets extended to HMAC implementation.
>> + */
>> +static int s5p_hash_final(struct ahash_request *req)
>> +{
>> +str
Re: [PATCH v7 1/2] crypto: s5p-sss: change spaces into tabs in defines
Hi Vladimir, Thank you for review. On 22.10.2017 12:18, Vladimir Zapolskiy wrote: > Hi Kamil, > > On 10/17/2017 02:28 PM, Kamil Konieczny wrote: >> change spaces into tabs in defines > > Here a grammatically correct sentence in English is welcome. What about: "Change spaces to tabs" ? >> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> >> --- > > Please feel free to add a tag > > Acked-by: Vladimir Zapolskiy <v...@mleia.com> -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH v7 2/2] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are nedded for fallback. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1407 +- 2 files changed, 1411 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 4b75084fabad..dea4d33d9c7f 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index dfae1865c384..c9feccf21efe 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,18 +1,21 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include #include +#include #include #include #include @@ -30,28 +33,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSET BIT(3) #define SSS_FCINTENSET_BTDMAINTENSET BIT(2) #define SSS_FCINTENSET_HRDMAINTENSET BIT(1) #define SSS_FCINTENSET_PKDMAINTENSET BIT(0) #define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7) +#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5) #define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) #define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) #define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) #define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) #define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_HPARTINTPBIT(7) +#define SSS_FCINTPEND_HDONEINTPBIT(5) #define SSS_FCINTPEND_BRDMAINTPBIT(3) #define SSS_FCINTPEND_BTDMAINTPBIT(2) #define SSS_FCINTPEND_HRDMAINTPBIT(1) @@ -72,6 +88,7 @@ #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) #define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) #define SSS_HASHIN_CIPHER_OUTPUT _SBF(0, 0x02) +#define SSS_HASHIN_MASK_SBF(0, 0x03) #define SSS_REG_FCBRDMAS 0x0020 #define SSS_REG_FCBRDMAL 0x0024 @@ -146,9 +163,80 @@ #define AES_KEY_LEN16 #define CRYPTO_QUEUE_LEN 1 +/* HASH registers */ +#define SSS_REG_HASH_CTRL 0x00 + +#define SSS_HASH_USER_IV_EN
[PATCH v7 0/2] crypto: s5p-sss: Add HASH support for Exynos
First patch cleans up spaces in defines, second adds HASH support for Exynos. Changes: version 7: - fix ifdef into if(IS_ENABLED()) as suggested by Krzysztof Kozlowski version 6: - fixes suggested by Vladimir Zapolskiy: change HASH_OP enum into bool, fix comments, change int into unsigned int in several functions, change some functions to return void, remove unnecessary parentheses in s5p_hash_import, replace rctx with ctx for request context, drop some dd vars and use tctx->dd instead, simplify s5p_hash_rx, s5p_hash_copy_result and s5p_hash_set_flow, change int final into bool final, reoder some declarations, split patch into two - rewrite and fix while loop in s5p_hash_copy_sg_lists - rewrite while loop in s5p_hash_prepare_sgs version 5: - fix suggested by Krzysztof Kozlowski: change defines HASH_OP into enum, fix comments version 4: - fixes suggested by Krzysztof Kozlowski: reformat comments, convert context flags into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in place of ifdef, remove sss_hash_algs_info and simplify register and deregister HASH algs version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments Kamil Konieczny (2): change spaces into tabs in defines Add HASH support for Exynos drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1597 +++--- 2 files changed, 1506 insertions(+), 105 deletions(-) -- 2.14.1.536.g6867272d5b56
[PATCH v7 1/2] crypto: s5p-sss: change spaces into tabs in defines
change spaces into tabs in defines Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- drivers/crypto/s5p-sss.c | 190 +++ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..dfae1865c384 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -30,98 +30,98 @@ #include #include -#define _SBF(s, v) ((v) << (s)) +#define _SBF(s, v) ((v) << (s)) /* Feed control registers */ -#define SSS_REG_FCINTSTAT 0x -#define SSS_FCINTSTAT_BRDMAINT BIT(3) -#define SSS_FCINTSTAT_BTDMAINT BIT(2) -#define SSS_FCINTSTAT_HRDMAINT BIT(1) -#define SSS_FCINTSTAT_PKDMAINT BIT(0) - -#define SSS_REG_FCINTENSET 0x0004 -#define SSS_FCINTENSET_BRDMAINTENSETBIT(3) -#define SSS_FCINTENSET_BTDMAINTENSETBIT(2) -#define SSS_FCINTENSET_HRDMAINTENSETBIT(1) -#define SSS_FCINTENSET_PKDMAINTENSETBIT(0) - -#define SSS_REG_FCINTENCLR 0x0008 -#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3) -#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2) -#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1) -#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0) - -#define SSS_REG_FCINTPEND 0x000C -#define SSS_FCINTPEND_BRDMAINTP BIT(3) -#define SSS_FCINTPEND_BTDMAINTP BIT(2) -#define SSS_FCINTPEND_HRDMAINTP BIT(1) -#define SSS_FCINTPEND_PKDMAINTP BIT(0) - -#define SSS_REG_FCFIFOSTAT 0x0010 -#define SSS_FCFIFOSTAT_BRFIFOFULBIT(7) -#define SSS_FCFIFOSTAT_BRFIFOEMPBIT(6) -#define SSS_FCFIFOSTAT_BTFIFOFULBIT(5) -#define SSS_FCFIFOSTAT_BTFIFOEMPBIT(4) -#define SSS_FCFIFOSTAT_HRFIFOFULBIT(3) -#define SSS_FCFIFOSTAT_HRFIFOEMPBIT(2) -#define SSS_FCFIFOSTAT_PKFIFOFULBIT(1) -#define SSS_FCFIFOSTAT_PKFIFOEMPBIT(0) - -#define SSS_REG_FCFIFOCTRL 0x0014 -#define SSS_FCFIFOCTRL_DESSEL BIT(2) -#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) -#define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01) -#define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02) - -#define SSS_REG_FCBRDMAS0x0020 -#define SSS_REG_FCBRDMAL0x0024 -#define SSS_REG_FCBRDMAC0x0028 -#define SSS_FCBRDMAC_BYTESWAP BIT(1) -#define SSS_FCBRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCBTDMAS0x0030 -#define SSS_REG_FCBTDMAL0x0034 -#define SSS_REG_FCBTDMAC0x0038 -#define SSS_FCBTDMAC_BYTESWAP BIT(1) -#define SSS_FCBTDMAC_FLUSH BIT(0) - -#define SSS_REG_FCHRDMAS0x0040 -#define SSS_REG_FCHRDMAL0x0044 -#define SSS_REG_FCHRDMAC0x0048 -#define SSS_FCHRDMAC_BYTESWAP BIT(1) -#define SSS_FCHRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAS0x0050 -#define SSS_REG_FCPKDMAL0x0054 -#define SSS_REG_FCPKDMAC0x0058 -#define SSS_FCPKDMAC_BYTESWAP BIT(3) -#define SSS_FCPKDMAC_DESCENDBIT(2) -#define SSS_FCPKDMAC_TRANSMIT BIT(1) -#define SSS_FCPKDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAO0x005C +#define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_BRDMAINT BIT(3) +#define SSS_FCINTSTAT_BTDMAINT BIT(2) +#define SSS_FCINTSTAT_HRDMAINT BIT(1) +#define SSS_FCINTSTAT_PKDMAINT BIT(0) + +#define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_BRDMAINTENSET BIT(3) +#define SSS_FCINTENSET_BTDMAINTENSET BIT(2) +#define SSS_FCINTENSET_HRDMAINTENSET BIT(1) +#define SSS_FCINTENSET_PKDMAINTENSET BIT(0) + +#define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) +#define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) +#define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) +#define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) + +#define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_BRDMAINTPBIT(3) +#define SSS_FCINTPEND_BTDMAINTPBIT(2) +#define SSS_FCINTPEND_HRDMAINTPBIT(1) +#define SSS_FCINTPEND_PKDMAINTPBIT(0) + +#define SSS_REG_FCFIFOSTAT 0x0010 +#define SSS_FCFIFOSTAT_BRFIFOFUL BIT(7) +#define SSS_FCFIFOSTAT_BRFIFOEMP BIT(6) +#define SSS_FCFIFOSTAT_BTFIFOFUL BIT(5) +#define SSS_FCFIFOSTAT_BTFIFOEMP BIT(4) +#define SSS_FCFIFOSTAT_HRFIFOFUL BIT(3) +#define SSS_FCFIFOSTAT_HRFIFOEMP BIT(2) +#define SSS_FCFIFOSTAT_PKFIFOFUL BIT(1) +#define SSS_FCFIFOSTAT_PKFIFOEMP BIT(0) + +#define SSS_REG_FCFIFOCTRL 0x0014 +#define SSS_FCFIFOCTRL_DESSEL BIT(2) +#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) +#define SSS_HASHIN_CIPHER_INPUT_SBF
Re: [PATCH 0/2] crypto: s5p-sss: Add HASH support for Exynos
Hi all, sorry for error in subject line, it should be [PATCH v6 0/2] and so on, I can resend this if needed ? On 16.10.2017 19:43, Kamil Konieczny wrote: > First patch cleans up spaces in defines, second adds HASH support for Exynos. > Changes: -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH 2/2] Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are nedded for fallback. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1406 +- 2 files changed, 1410 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 4b75084fabad..dea4d33d9c7f 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index dfae1865c384..b5a7e49f9285 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,18 +1,21 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include #include +#include #include #include #include @@ -30,28 +33,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSET BIT(3) #define SSS_FCINTENSET_BTDMAINTENSET BIT(2) #define SSS_FCINTENSET_HRDMAINTENSET BIT(1) #define SSS_FCINTENSET_PKDMAINTENSET BIT(0) #define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7) +#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5) #define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) #define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) #define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) #define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) #define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_HPARTINTPBIT(7) +#define SSS_FCINTPEND_HDONEINTPBIT(5) #define SSS_FCINTPEND_BRDMAINTPBIT(3) #define SSS_FCINTPEND_BTDMAINTPBIT(2) #define SSS_FCINTPEND_HRDMAINTPBIT(1) @@ -72,6 +88,7 @@ #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) #define SSS_HASHIN_CIPHER_INPUT_SBF(0, 0x01) #define SSS_HASHIN_CIPHER_OUTPUT _SBF(0, 0x02) +#define SSS_HASHIN_MASK_SBF(0, 0x03) #define SSS_REG_FCBRDMAS 0x0020 #define SSS_REG_FCBRDMAL 0x0024 @@ -146,9 +163,80 @@ #define AES_KEY_LEN16 #define CRYPTO_QUEUE_LEN 1 +/* HASH registers */ +#define SSS_REG_HASH_CTRL 0x00 + +#define SSS_HASH_USER_IV_EN
[PATCH 1/2] change spaces into tabs in defines
Change spaces into tabs in defines. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- drivers/crypto/s5p-sss.c | 190 +++ 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..dfae1865c384 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -30,98 +30,98 @@ #include #include -#define _SBF(s, v) ((v) << (s)) +#define _SBF(s, v) ((v) << (s)) /* Feed control registers */ -#define SSS_REG_FCINTSTAT 0x -#define SSS_FCINTSTAT_BRDMAINT BIT(3) -#define SSS_FCINTSTAT_BTDMAINT BIT(2) -#define SSS_FCINTSTAT_HRDMAINT BIT(1) -#define SSS_FCINTSTAT_PKDMAINT BIT(0) - -#define SSS_REG_FCINTENSET 0x0004 -#define SSS_FCINTENSET_BRDMAINTENSETBIT(3) -#define SSS_FCINTENSET_BTDMAINTENSETBIT(2) -#define SSS_FCINTENSET_HRDMAINTENSETBIT(1) -#define SSS_FCINTENSET_PKDMAINTENSETBIT(0) - -#define SSS_REG_FCINTENCLR 0x0008 -#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3) -#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2) -#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1) -#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0) - -#define SSS_REG_FCINTPEND 0x000C -#define SSS_FCINTPEND_BRDMAINTP BIT(3) -#define SSS_FCINTPEND_BTDMAINTP BIT(2) -#define SSS_FCINTPEND_HRDMAINTP BIT(1) -#define SSS_FCINTPEND_PKDMAINTP BIT(0) - -#define SSS_REG_FCFIFOSTAT 0x0010 -#define SSS_FCFIFOSTAT_BRFIFOFULBIT(7) -#define SSS_FCFIFOSTAT_BRFIFOEMPBIT(6) -#define SSS_FCFIFOSTAT_BTFIFOFULBIT(5) -#define SSS_FCFIFOSTAT_BTFIFOEMPBIT(4) -#define SSS_FCFIFOSTAT_HRFIFOFULBIT(3) -#define SSS_FCFIFOSTAT_HRFIFOEMPBIT(2) -#define SSS_FCFIFOSTAT_PKFIFOFULBIT(1) -#define SSS_FCFIFOSTAT_PKFIFOEMPBIT(0) - -#define SSS_REG_FCFIFOCTRL 0x0014 -#define SSS_FCFIFOCTRL_DESSEL BIT(2) -#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) -#define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01) -#define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02) - -#define SSS_REG_FCBRDMAS0x0020 -#define SSS_REG_FCBRDMAL0x0024 -#define SSS_REG_FCBRDMAC0x0028 -#define SSS_FCBRDMAC_BYTESWAP BIT(1) -#define SSS_FCBRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCBTDMAS0x0030 -#define SSS_REG_FCBTDMAL0x0034 -#define SSS_REG_FCBTDMAC0x0038 -#define SSS_FCBTDMAC_BYTESWAP BIT(1) -#define SSS_FCBTDMAC_FLUSH BIT(0) - -#define SSS_REG_FCHRDMAS0x0040 -#define SSS_REG_FCHRDMAL0x0044 -#define SSS_REG_FCHRDMAC0x0048 -#define SSS_FCHRDMAC_BYTESWAP BIT(1) -#define SSS_FCHRDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAS0x0050 -#define SSS_REG_FCPKDMAL0x0054 -#define SSS_REG_FCPKDMAC0x0058 -#define SSS_FCPKDMAC_BYTESWAP BIT(3) -#define SSS_FCPKDMAC_DESCENDBIT(2) -#define SSS_FCPKDMAC_TRANSMIT BIT(1) -#define SSS_FCPKDMAC_FLUSH BIT(0) - -#define SSS_REG_FCPKDMAO0x005C +#define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_BRDMAINT BIT(3) +#define SSS_FCINTSTAT_BTDMAINT BIT(2) +#define SSS_FCINTSTAT_HRDMAINT BIT(1) +#define SSS_FCINTSTAT_PKDMAINT BIT(0) + +#define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_BRDMAINTENSET BIT(3) +#define SSS_FCINTENSET_BTDMAINTENSET BIT(2) +#define SSS_FCINTENSET_HRDMAINTENSET BIT(1) +#define SSS_FCINTENSET_PKDMAINTENSET BIT(0) + +#define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_BRDMAINTENCLR BIT(3) +#define SSS_FCINTENCLR_BTDMAINTENCLR BIT(2) +#define SSS_FCINTENCLR_HRDMAINTENCLR BIT(1) +#define SSS_FCINTENCLR_PKDMAINTENCLR BIT(0) + +#define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_BRDMAINTPBIT(3) +#define SSS_FCINTPEND_BTDMAINTPBIT(2) +#define SSS_FCINTPEND_HRDMAINTPBIT(1) +#define SSS_FCINTPEND_PKDMAINTPBIT(0) + +#define SSS_REG_FCFIFOSTAT 0x0010 +#define SSS_FCFIFOSTAT_BRFIFOFUL BIT(7) +#define SSS_FCFIFOSTAT_BRFIFOEMP BIT(6) +#define SSS_FCFIFOSTAT_BTFIFOFUL BIT(5) +#define SSS_FCFIFOSTAT_BTFIFOEMP BIT(4) +#define SSS_FCFIFOSTAT_HRFIFOFUL BIT(3) +#define SSS_FCFIFOSTAT_HRFIFOEMP BIT(2) +#define SSS_FCFIFOSTAT_PKFIFOFUL BIT(1) +#define SSS_FCFIFOSTAT_PKFIFOEMP BIT(0) + +#define SSS_REG_FCFIFOCTRL 0x0014 +#define SSS_FCFIFOCTRL_DESSEL BIT(2) +#define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) +#define SSS_HASHIN_CIPHER_INPUT_SBF
[PATCH v5] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are nedded for fallback. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- version 5: - fix suggested by Krzysztof Kozlowski: change defines HASH_OP into enum, fix comments version 4: - fixes suggested by Krzysztof Kozlowski: reformat comments, convert context flags into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in place of ifdef, remove sss_hash_algs_info and simplify register and deregister HASH algs version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1441 +- 2 files changed, 1445 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index fe33c199fc1a..01cf07ce34c5 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..29bec5a69fe8 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,18 +1,21 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include #include +#include #include #include #include @@ -30,28 +33,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT
[PATCH v4] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are nedded for fallback. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- version 4: - fixes suggested by Krzysztof Kozlowski: reformat comments, convert context flag into two bool vars, drop SSS_ALIGNED, change name of SSS_DMA_ALIGN and SSS_DMA_ALIGN_MASK, split assignments into separate lines, use IS_ENABLED in place of ifdef, remove sss_hash_algs_info and simplify register and deregister HASH algs version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1441 +- 2 files changed, 1445 insertions(+), 10 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index fe33c199fc1a..01cf07ce34c5 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..2864efeaf8f8 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,18 +1,21 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include #include +#include #include #include #include @@ -30,28 +33,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x00
Re: [PATCH v3] crypto: s5p-sss: Add HASH support for Exynos
On 03.10.2017 21:30, Krzysztof Kozlowski wrote:
> On Tue, Oct 03, 2017 at 04:57:43PM +0200, Kamil Konieczny wrote:
>
>>>> [...]
>>>> +static struct ahash_alg algs_sha256[] = {
>>>> +{
>>>> + .init = s5p_hash_init,
>>>> + .update = s5p_hash_update,
>>>> + .final = s5p_hash_final,
>>>> + .finup = s5p_hash_finup,
>>>> + .digest = s5p_hash_digest,
>>>> + .halg.digestsize= SHA256_DIGEST_SIZE,
>>>> + .halg.base = {
>>>> + .cra_name = "sha256",
>>>> + .cra_driver_name= "exynos-sha256",
>>>> + .cra_priority = 100,
>>>> + .cra_flags = CRYPTO_ALG_TYPE_AHASH |
>>>> +CRYPTO_ALG_KERN_DRIVER_ONLY |
>>>> +CRYPTO_ALG_ASYNC |
>>>> +CRYPTO_ALG_NEED_FALLBACK,
>>>> + .cra_blocksize = HASH_BLOCK_SIZE,
>>>> + .cra_ctxsize= sizeof(struct s5p_hash_ctx),
>>>> + .cra_alignmask = SSS_DMA_ALIGN_MASK,
>>>> + .cra_module = THIS_MODULE,
>>>> + .cra_init = s5p_hash_cra_init,
>>>> + .cra_exit = s5p_hash_cra_exit,
>>>> + }
>>>> +}
>>>> +
>>>> +};
>>>> +
>>>> +static struct sss_hash_algs_info exynos_hash_algs_info[] = {
>>>
>>> You have warnings in your code. Please be sure that all compiler,
>>> Smatch, Sparse, checkpatch and coccicheck warnings are fixed.
>>>
>>> ../drivers/crypto/s5p-sss.c:1896:34: warning: ‘exynos_hash_algs_info’
>>> defined but not used [-Wunused-variable]
>>> static struct sss_hash_algs_info exynos_hash_algs_info[] = {
>>>
>>> Probably this should be __maybe_unused.
>>
>> You are right, I did not check this with EXYNOS_HASH disabled, I will
>> rewrite it.
>>
>>> Also this should be const. I do not understand why you have to add one
>>> more static variable (which sticks the driver to only one instance...)
>>> and then modify it during runtime. Everything should be stored in device
>>> state container (s5p_aes_dev) - directly or through some other pointers.
>>
>> There is .registered field which is incremented with each algo register.
>> I can move assignes to fields .import, .export and .statesize into struct.
>> When I tried add const, I got compiler warn:
>> drivers/crypto/s5p-sss.c: In function ‘s5p_aes_remove’:
>> drivers/crypto/s5p-sss.c:2397:6: warning: passing argument 1 of
>> ‘crypto_unregister_ahash’ discards ‘const’ qualifier from pointer target
>> type [-Wdiscarded-qualifiers]
>> _algs_i[i].algs_list[j]);
>> so it was not designed to be const (in crypto framework).
>> In AES code the alg struct is also static:
>> static struct crypto_alg algs[] = {
>
> The crypto_alg and ahash_alg must indeed stay non-const but
> sss_hash_algs_info is different. You do not pass it to crypto-core.
It was again from omap driver, which has descriptions for omap2, omap4 and
omap5,
but here I over-complicated this. I will just remove it and simplify register
and deregister hash algs.
>
>> What you mean by 'stick the driver to only one instance' ? In Exynos 4412
>> there
>> is only one SecSS block, in some other Exynos there is SlimSS, but it is not
>> the same (it has lower capabilities and other io addresses), so there should
>> not
>> be two s5p_aes_dev drivers loaded at the same time.
>
> Current driver matches hardware in one-to-one so indeed there cannot be
> two s5p_aes_dev devices. However this might change thus almost every
> driver tries to follow the pattern of state-container passed to device
> (e.g. platform_set_drvdata()). With this approach the code is nicely
> encapsulated and usually much easier to review. Globals (or file-scope
> variables) usually makes code more difficult to maintain.
>
> In this driver this is not entirely possible as some crypto-functions do
> not allow passing driver-supplied opaque pointer. But except this case,
> everywhere else the driver should follow common convention - do not use
> static variables.
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH v3] crypto: s5p-sss: Add HASH support for Exynos
On 30.09.2017 21:50, Krzysztof Kozlowski wrote: > On Wed, Sep 27, 2017 at 02:25:50PM +0200, Kamil Konieczny wrote: >> Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. >> It uses the crypto framework asynchronous hash api. >> It is based on omap-sham.c driver. >> S5P has some HW differencies and is not implemented. >> >> Modifications in s5p-sss: >>[...] >> +/* >> + * HASH bit numbers, used by device, setting in dev->hash_flags with >> + * functions set_bit(), clear_bit() or tested with test_bit() or BIT(), >> + * to keep HASH state BUSY or FREE, or to signal state from irq_handler >> + * to hash_tasklet. SGS keep track of allocated memory for scatterlist >> + */ >> +#define HASH_FLAGS_BUSY 0 >> +#define HASH_FLAGS_FINAL1 >> +#define HASH_FLAGS_DMA_ACTIVE 2 >> +#define HASH_FLAGS_OUTPUT_READY 3 >> +#define HASH_FLAGS_DMA_READY4 >> +#define HASH_FLAGS_SGS_COPIED 5 >> +#define HASH_FLAGS_SGS_ALLOCED 6 >> + >> +/* >> + * HASH bit numbers used in request context >> + * FINUP mark last hash operation >> + */ >> +#define HASH_FLAGS_FINUP7 >> +#define HASH_FLAGS_ERROR8 > > I spent some time on s5p_hash_finish_req() and other code around flags, > confused by two different flags (ctx->flags, device->hash_flags) and > different API used to play with them next to each other (once test_bit, > line later just |=). > > This is just confusing. AFAIU, you use only two bits in ctx->flags, so > just convert it to two bools. This will remove the confuse: > 1. between the defines before and here, > 2. around mixing xxx_bit() and regular |= operations. > Good point, I will rewrite them into two bool vars. >> + >> +/* HASH op codes */ >> +#define HASH_OP_UPDATE 1 >> +#define HASH_OP_FINAL 2 >> + >> +/* HASH HW constants */ >> +#define BUFLEN HASH_BLOCK_SIZE >> + >> +#define SSS_DMA_ALIGN 16 >> +#define SSS_ALIGNED __attribute__((aligned(SSS_DMA_ALIGN))) >> +#define SSS_DMA_ALIGN_MASK (SSS_DMA_ALIGN - 1) > > No changes here... I asked for making this consistent with current code > so please bring a patch which introduces new macro to existing code and > then re-use it for new code. > > Dropping inconsistent code and then promising "I will fix it up later" > does not work. This align stuff was again taken from omap driver. AES code does not need any DMA_ALIGN, it sets ".cra_align = 0x0f", but it is not needed, it can be simple zero 0x00, as AES operate on blocks of 64 bytes long, and SecSS DMA engine can operate on non-aligned addresses. I am not sure how CPU will handle HASH corner case at last update, when input data will point to last byte of page, length will be 1, and FeedControl will round up length to 8, so DMA will try (?) reading bytes after page. To sum it up, AES code needs cleanup ".cra_align = 0", but I do not think it is so critical to make it before HASH. You have good point here, as the names are confusing, so I will remove SSS_ALIGNED from HASH struct definitions, and change SSS_DMA_ALIGN into SSS_HASH_DMA_LEN_ALIGN, and SSS_DMA_ALIGN_MASK into SSS_HASH_DMA_ALIGN_MASK. >> [...] >> * @lock: Lock for protecting both access to device hardware registers >> - * and fields related to current request (including the busy >> field). >> + * and fields related to current request (including the busy >> + * field). > > Why wrapping this line? Sorry, I will remove this cleanup (line is 81 bytes long). >> + * @res:Resources for hash. >> + * @io_hash_base: Per-variant offset for HASH block IO memory. >> + * @hash_lock: Lock for protecting hash_req, hash_queue and hash_flags >> + * variable. >> + * @hash_tasklet: New HASH request scheduling job. >> + * @xmit_buf: Buffer for current HASH request transfer into SSS block. >> + * @hash_flags: Flags for current HASH op. >> + * @hash_queue: Async hash queue. >> + * @hash_req: Current request sending to SSS HASH block. >> + * @hash_sg_iter: Scatterlist transferred through DMA into SSS HASH block. >> + * @hash_sg_cnt: Counter for hash_sg_iter. >> + * >> +/** >> + * s5p_hash_rx - get next hash_sg_iter >> + * @dev:device >> + * >> + * Return: >> + * 2if there is no more data and it is UPDATE op >> + * 1if new receiving (input) data is ready and can be written to >> + * device > > Why wrapping so early? OK, I will reformat comments up to 80 bytes per line, thi
[PATCH v3] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are nedded for fallback. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- version 3: - many fixes suggested by Krzysztof Kozlowski: comments, uppercases in const, remove unused defines, remove unused variable bs, constify aes_variant, remove global var use_hash, remove WARN_ON, improve hash_import(), change goto label into 'out' in s5p_hash_handle_queue(), reorder variable declarations, add spinlock to protect clearing HASH_FLAGS_BUSY - simplify code: replace one-line functions s5p_hash_update_req(), s5p_hash_final_req() with call to s5p_hash_xmit_dma(), and delete them - replace call to s5p_hash_hw_init() into s5p_ahash_dma_init() and delete it - fix clearing shash flag CRYPTO_TFM_REQ_MAY_SLEEP - fix s5p_hash_set_flow() version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments drivers/crypto/Kconfig | 14 + drivers/crypto/s5p-sss.c | 1507 +- 2 files changed, 1509 insertions(+), 12 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index fe33c199fc1a..01cf07ce34c5 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,20 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + This will select software SHA1, MD5 and SHA256 as they are + needed for small and zero-size messages. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..e801ec4bfd8e 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,18 +1,21 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include #include +#include #include #include #include @@ -30,28 +33,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSETBIT(3) #define SSS_FCINTENSET_BTDMAINTENSETBIT(2) #define SSS_FCINTENSET_HRDMAINTENSETBIT(1) #define SSS_FCINTENSET_PKDMAINTENSETBIT(0) #define SSS_REG_FCINTENCLR 0x00
Re: [PATCH v2] crypto: s5p-sss: Add HASH support for Exynos
On 26.09.2017 21:28, Krzysztof Kozlowski wrote:
> On Tue, Sep 26, 2017 at 05:54:42PM +0200, Kamil Konieczny wrote:
>> On 25.09.2017 20:27, Krzysztof Kozlowski wrote:
>> [...]
>>>>>> +struct tasklet_struct hash_tasklet;
>>>>>> +u8 xmit_buf[BUFLEN] SSS_ALIGNED;
>>>>>> +
>>>>>> +unsigned long hash_flags;
>>>>>
>>>>> This should be probably DECLARE_BITMAP.
>>>>
>>>> I do not get it, it is used for bits HASH_FLAGS_... and is not HW related.
>>>> This will grow this patch even longer.
>>>
>>> Why longer? Only declaration changes, rest should be the same.
>>> Just instead of playing with bits over unsigned long explicitly use a
>>> type for that purpose - DECLARE_BITMAP.
>>
>> It will make code ugly, using >hash_flags[0] instead of
>> >hash_flags.
>
> Wait, all the set_bit and clear_bit should be even simpler:
> -set_bit(HASH_FLAGS_DMA_READY, >hash_flags);
> +set_bit(HASH_FLAGS_DMA_READY, dev->hash_flags);
> This looks actually better..
>
>> I have 9 bits used, and I will not anticipate it grow above 64.
>
> You mean 32.
>
>> If you insist, I can add DECLARE_BITMAP and rewrite all calls to _bits()
>> functions.
>
> Actually not, I do not insist. Just for me it makes the code simpler
> (lack of "&" operand) and more robust as you explicitly declare number
> of used bits (instead of assumption that your data type on given
> compiler matches the amount of flags). Both arguments are not that
> important.
I see, but I will postpone this to later refactoring and code cleanup,
as none crypto drivers uses it now, and the declaration after macro unwind
will look like:
unsigned long hash_flags[1];
[...]
>>>>>> +} else {
>>>>>> +ctx->flags |= BIT(HASH_FLAGS_ERROR);
>>>>>> +}
>>>>>> +
>>>>>> +/* atomic operation is not needed here */
>>>>>
>>>>> Ok, but why?
>>>>>
>>>>
>>>> Good question, I frankly copy this. The hash vars in s5p_aes_dev
>>>> are no longer used as all transfers ended, we have req context here
>>>> so after complete we can work on next request,
>>>> and clearing bit HASH_FLAGS_BUSY allows this.
>>>
>>> I think you need them. Here and few lines before. In
>>> s5p_hash_handle_queue() you use spin-lock because it might be executed
>>> multiple times. Having that assumption (multiple calls to
>>> s5p_hash_handle_queue()) you can have:
>>>
>>> Thread #1: Thread #2
>>> s5p_hash_finish_req() s5p_hash_handle_queue()
>>> dd->hash_flags &= ...
>>>which equals to:
>>
>> somewhere here will be (in Thread #2):
>> if(test_bit(HASH_FLAGS_BUSY,dd->hash_flags)) {
>> unlock, return; }
>
> Right, good point! The HASH_FLAGS_BUSY bit is cleared only in
> s5p_hash_finish_req() which will be executed only if the bit was set
> (checked with test_bits()). This should be then put next to the sentence
> "atomics are not needed here".
>
>>
>>>tmp = dd->hash_flags;
>>>tmp &= ~(BIT...)
>>> set_bit(HASH_FLAGS_BUSY, >hash_flags);
>>>dd->hash_flags = tmp
>>>
>>> Which means that in last assignment you effectively lost the
>>> HASH_FLAGS_BUSY bit.
>>>
>>> In general, you need to use atomics (or locks) everywhere, unless you
>>> are 100% sure that given lockless section cannot be executed with other
>>> code which uses locks. Otherwise the atomics/locks become uneffective.
>>
>> I can add spinlock as I am not 100% sure if below is true:
>>
>> HASH_FLAGS_BUSY is protected by order of instructions and by test_bit.
>
> In SMP code, one should not expect too much of order. :)
> For sections not guarded by barriers (thus also locks) compiler can
> re-order a lot. CPU can re-order even more. Although these
> test_bit() and clear_bit() are atomic operations but they do not provide
> barriers so they can be re-ordered.
>
>> First, this bit is set only in one place, in s5p_hash_handle_queue() and
>> cleared
>> also only in one place, s5p_hash_finish_req().
>>
>> In function s5p_hash_handle_queue(), after spinlock, bit HASH_FLA
Re: [PATCH v2] crypto: s5p-sss: Add HASH support for Exynos
On 25.09.2017 20:27, Krzysztof Kozlowski wrote: > On Fri, Sep 22, 2017 at 08:00:17PM +0200, Kamil Konieczny wrote: >> On 19.09.2017 21:03, Krzysztof Kozlowski wrote: >>> On Fri, Sep 15, 2017 at 07:50:06PM +0200, Kamil Konieczny wrote: > > (...) > >>>> + >>>> +/* HASH flags */ >>> >>> All defines below have "HASH_FLAGS" prefix... so actually useful would >>> be to explain also what is a hash flag. >> >> These are bit numbers used by device driver, setting in dev->hash_flags, >> used with set_bit(), clear_bit() or test_bit(), and with macro BIT(), >> to keep device state BUSY or FREE, or to signal state from irq_handler >> to hash_tasklet. > > Then add something similar as comment to the code. OK. [...] >>>> * @lock: Lock for protecting both access to device hardware registers >>>> - *and fields related to current request (including the >>>> busy field). >>>> + *and fields related to current request (including the >>>> busy >>>> + *field). >>>> + * @res: Resources for hash. >>>> + * @io_hash_base: Per-variant offset for HASH block IO memory. >>>> + * @hash_lock:Lock for protecting hash_req and other HASH variables. >>> >>> I must admit that I do not see how it protects the hash_req. The >>> hash_req looks untouched (not read, not modified) during this lock >>> critical section. Can you share some more thoughts about it? >> >> It protects dev->hash_queue and dev->hash_flags >> in begin of function s5p_hash_handle_queue. >> >> After dequeue non-null request, bit HASH_BUSY is set in dd->hash_flags, >> and this bit protects dd->hash_req and other fields. From there device >> works with dd->hash_req until finish in irq_handler and hash_tasklet. > > Few thoughts here: > 1. Other accesses to HASH_BUSY bit of hash_flags are not protected with >lock and you are using set_bit/clear_bit which are atomic operations, >so from this perspective it does not protect hash_flags. > 2. This means the only field protected here is hash_queue so I guess you >expect possibility of multiple concurrent calls to s5p_hash_handle_queue(). >This makes sense but then description has to be updated. OK, I will update it. Other bits in dev->hash_flags can be set and cleared only in BUSY state. [...] >>>> + struct tasklet_struct hash_tasklet; >>>> + u8 xmit_buf[BUFLEN] SSS_ALIGNED; >>>> + >>>> + unsigned long hash_flags; >>> >>> This should be probably DECLARE_BITMAP. >> >> I do not get it, it is used for bits HASH_FLAGS_... and is not HW related. >> This will grow this patch even longer. > > Why longer? Only declaration changes, rest should be the same. > Just instead of playing with bits over unsigned long explicitly use a > type for that purpose - DECLARE_BITMAP. It will make code ugly, using >hash_flags[0] instead of >hash_flags. I have 9 bits used, and I will not anticipate it grow above 64. If you insist, I can add DECLARE_BITMAP and rewrite all calls to _bits() functions. [...] >>>> + struct samsung_aes_variant *pdata; >>> >>> This should be const as pdata should not be modified. >> >> You are right, and I do not modify _offsets parts, but only hash >> functions pointers and hash array size. It is made non-const due >> to keeping it in the place, and not moving struct samsung_aes_variant >> below hash functions definitions. >> >> If I need to keep them const, then I will move whole part below, just >> before _probe, so I can properly set hash_algs_info and >> hash_algs_size (and again, small change, but bigger patch). >> >> I can do this, if you think it is safer to have them const. > > I see, you are modifiying the variant->hash_algs_info and > variant->hash_algs_size. However all of them should be static const as > this is not a dynamic data. It does not depend on any runtime > conditions, except of course choosing the variant itself. You are right, I will make samsung_aes_variant const again. [...] >>>> +/** >>>> + * s5p_hash_finish_req - finish request >>>> + * @req: AHASH request >>>> + * @err: error >>>> + * >>>> + * Clear flags, free memory, >>>> + * if FINAL then read output into ctx->digest, >>>> + * call completetion >>>> + */ >>>> +static void s5p_hash_finish_req(struct ahash_requ
Re: Question about ahash export and import
On 26.09.2017 12:34, Gilad Ben-Yossef wrote: > On Tue, Sep 26, 2017 at 12:36 PM, Christophe LEROY > <christophe.le...@c-s.fr> wrote: >> Hello, >> >> Today, the talitos driver dma maps/unmaps context and keys at every single >> request. >> I'm looking at doing the dma mapping at hash init and doing unmapping at >> final/finup/digest >> >> However, I'm wondering how to manage hash exports and imports. I was >> initially thinking about doing a dma_sync_for_cpu() before any export and a >> dma_sync_for_device() after any export, but that supposes that the dma area >> is already mapped, which means that we have done the init and not yet done a >> final/finup/digest. >> >> I'm a bit sceptic when reading the following text in include/crypto/hash.h : >> >> @export: Export partial state of the transformation. This function dumps the >> * entire state of the ongoing transformation into a provided block >> of >> * data so it can be @import 'ed back later on. This is useful in >> case >> * you want to save partial result of the transformation after >> * processing certain amount of data and reload this partial result >> * multiple times later on for multiple re-use >> >> >> Does it mean that import may be called in lieu of hash init, or is hash init >> always called before doing an import ? >> > > I believe import is called in lieu of init. > > See the example is testmgr.c here: > http://elixir.free-electrons.com/linux/latest/source > > import is called on a freshly allocated request (save for set_crypt > and set_callback), followed by > a call to update. Both import()/export() are synchronous op, this means they are not allowed to return -EINPROGRESS nor call complete(). Can import() be called without _any_ init(), for example after reboot of machine ? Is following scenario valid: init(), update() 0 or more times, export(), save exported data to pernament storage reboot machine load crypto driver, import() saved state ? -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: [PATCH v2] crypto: s5p-sss: Add HASH support for Exynos
On 19.09.2017 21:03, Krzysztof Kozlowski wrote:
> On Fri, Sep 15, 2017 at 07:50:06PM +0200, Kamil Konieczny wrote:
>> Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW.
>> It uses the crypto framework asynchronous hash api.
>> It is based on omap-sham.c driver.
>> S5P has some HW differencies and is not implemented.
>>
>> Modifications in s5p-sss: [...]
Below I will address only 'const' questions.
>>[...]
>> +struct crypto_queue hash_queue;
>> +struct ahash_request*hash_req;
>> +struct scatterlist *hash_sg_iter;
>> +int hash_sg_cnt;
>> +
>> +struct samsung_aes_variant *pdata;
>
> This should be const as pdata should not be modified.
I will remove this.
>> [...]
>> -static const struct samsung_aes_variant s5p_aes_data = {
>> +static struct samsung_aes_variant s5p_aes_data = {
>
> Why do you need to drop the const? This should not be modified.
OK, I will not modify this.
> [...]
>> [...]
>> + */
>> +static struct sss_hash_algs_info exynos_hash_algs_info[] = {
>
> Can it be const?
No, it contains '.registered' var, used at probe/error path/remove.
>>[...]
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [PATCH v2] crypto: s5p-sss: Add HASH support for Exynos
On 19.09.2017 21:03, Krzysztof Kozlowski wrote: > On Fri, Sep 15, 2017 at 07:50:06PM +0200, Kamil Konieczny wrote: >> Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. >> It uses the crypto framework asynchronous hash api. >> It is based on omap-sham.c driver. >> S5P has some HW differencies and is not implemented. >> >> Modifications in s5p-sss: >> >> - Add hash supporting structures and functions. >> >> - Modify irq handler to handle both aes and hash signals. >> >> - Resize resource end in probe if EXYNOS_HASH is enabled in >> Kconfig. >> >> - Add new copyright line and new author. >> >> - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 >> with crypto run-time self test testmgr >> and with tcrypt module with: modprobe tcrypt sec=1 mode=N >> where N=402, 403, 404 (MD5, SHA1, SHA256). >> >> Modifications in drivers/crypto/Kconfig: >> >> - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG >> and CRYPTO_DEV_S5P >> >> - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH >> as they are nedded for fallback. >> >> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> >> --- >> version 2: >> - change patch format so number of lines drops >> - change in Kconfig as suggested by Krzysztof Kozlowski, add >> EXYNOS_HASH subsection >> - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH >> - remove style fixups in aes, as they should go in separate patch >> - remove FLOW_LOG, FLOW_DUMP macros and its uses >> - remove #if 0 ... endif >> - remove unused function hash_wait and its defines >> - fix compiler warning in dev_dbg >> - remove some comments >> - other minor fixes in comments > > Thanks for the changes. I must admit that I did not finish the review... > I found a lot of minor stuff but actually I did not look at overall > concept, architecture and how it really works. Sorry, will do it > later... No problem, I will rework this patch. Also see answers to your questions below. As I see crypto framework, transformation describes what to do, request is 'a handle' for actual operation. State for operation is kept in request context. Init, update, finup, final are async, but import and export must be synchronous, as can be deduced from crypto/testmgr.c tests. > >> drivers/crypto/Kconfig | 12 + >> drivers/crypto/s5p-sss.c | 1683 >> +- >> 2 files changed, 1674 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig >> index fe33c199fc1a..2f094c433346 100644 >> --- a/drivers/crypto/Kconfig >> +++ b/drivers/crypto/Kconfig >> @@ -439,6 +439,18 @@ config CRYPTO_DEV_S5P >>Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES >>algorithms execution. >> >> +config CRYPTO_DEV_EXYNOS_HASH >> +bool "Support for Samsung Exynos HASH accelerator" >> +depends on CRYPTO_DEV_S5P >> +depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m >> +select CRYPTO_SHA1 >> +select CRYPTO_MD5 >> +select CRYPTO_SHA256 >> +help >> + Select this to offload Exynos from HASH MD5/SHA1/SHA256. >> + HASH algorithms will be disabled if EXYNOS_RNG >> + is enabled due to hw conflict. >> + >> config CRYPTO_DEV_NX >> bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" >> depends on PPC64 >> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c >> index 7ac657f46d15..e951f0ffe49b 100644 >> --- a/drivers/crypto/s5p-sss.c >> +++ b/drivers/crypto/s5p-sss.c >> @@ -1,18 +1,21 @@ >> /* >> * Cryptographic API. >> * >> - * Support for Samsung S5PV210 HW acceleration. >> + * Support for Samsung S5PV210 and Exynos HW acceleration. >> * >> * Copyright (C) 2011 NetUP Inc. All rights reserved. >> + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. >> * >> * This program is free software; you can redistribute it and/or modify >> * it under the terms of the GNU General Public License version 2 as >> published >> * by the Free Software Foundation. >> * >> + * Hash part based on omap-sham.c driver. >> */ >> >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -30,28 +33,41 @@ >> #include >> #include >> >> +#include >> +#include
[PATCH v2] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. It uses the crypto framework asynchronous hash api. It is based on omap-sham.c driver. S5P has some HW differencies and is not implemented. Modifications in s5p-sss: - Add hash supporting structures and functions. - Modify irq handler to handle both aes and hash signals. - Resize resource end in probe if EXYNOS_HASH is enabled in Kconfig. - Add new copyright line and new author. - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 with crypto run-time self test testmgr and with tcrypt module with: modprobe tcrypt sec=1 mode=N where N=402, 403, 404 (MD5, SHA1, SHA256). Modifications in drivers/crypto/Kconfig: - Add new CRYPTO_DEV_EXYNOS_HASH, depend on !EXYNOS_RNG and CRYPTO_DEV_S5P - Select sw algorithms MD5, SHA1 and SHA256 in EXYNOS_HASH as they are nedded for fallback. Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> --- version 2: - change patch format so number of lines drops - change in Kconfig as suggested by Krzysztof Kozlowski, add EXYNOS_HASH subsection - change #ifndef EXYNOS_RNG into #ifdef CRYPTO_DEV_EXYNOS_HASH - remove style fixups in aes, as they should go in separate patch - remove FLOW_LOG, FLOW_DUMP macros and its uses - remove #if 0 ... endif - remove unused function hash_wait and its defines - fix compiler warning in dev_dbg - remove some comments - other minor fixes in comments drivers/crypto/Kconfig | 12 + drivers/crypto/s5p-sss.c | 1683 +- 2 files changed, 1674 insertions(+), 21 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index fe33c199fc1a..2f094c433346 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -439,6 +439,18 @@ config CRYPTO_DEV_S5P Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES algorithms execution. +config CRYPTO_DEV_EXYNOS_HASH + bool "Support for Samsung Exynos HASH accelerator" + depends on CRYPTO_DEV_S5P + depends on !CRYPTO_DEV_EXYNOS_RNG && CRYPTO_DEV_EXYNOS_RNG!=m + select CRYPTO_SHA1 + select CRYPTO_MD5 + select CRYPTO_SHA256 + help + Select this to offload Exynos from HASH MD5/SHA1/SHA256. + HASH algorithms will be disabled if EXYNOS_RNG + is enabled due to hw conflict. + config CRYPTO_DEV_NX bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration" depends on PPC64 diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 7ac657f46d15..e951f0ffe49b 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -1,18 +1,21 @@ /* * Cryptographic API. * - * Support for Samsung S5PV210 HW acceleration. + * Support for Samsung S5PV210 and Exynos HW acceleration. * * Copyright (C) 2011 NetUP Inc. All rights reserved. + * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as published * by the Free Software Foundation. * + * Hash part based on omap-sham.c driver. */ #include #include +#include #include #include #include @@ -30,28 +33,41 @@ #include #include +#include +#include +#include +#include + #define _SBF(s, v) ((v) << (s)) /* Feed control registers */ #define SSS_REG_FCINTSTAT 0x +#define SSS_FCINTSTAT_HPARTINT BIT(7) +#define SSS_FCINTSTAT_HDONEINT BIT(5) #define SSS_FCINTSTAT_BRDMAINT BIT(3) #define SSS_FCINTSTAT_BTDMAINT BIT(2) #define SSS_FCINTSTAT_HRDMAINT BIT(1) #define SSS_FCINTSTAT_PKDMAINT BIT(0) #define SSS_REG_FCINTENSET 0x0004 +#define SSS_FCINTENSET_HPARTINTENSET BIT(7) +#define SSS_FCINTENSET_HDONEINTENSET BIT(5) #define SSS_FCINTENSET_BRDMAINTENSETBIT(3) #define SSS_FCINTENSET_BTDMAINTENSETBIT(2) #define SSS_FCINTENSET_HRDMAINTENSETBIT(1) #define SSS_FCINTENSET_PKDMAINTENSETBIT(0) #define SSS_REG_FCINTENCLR 0x0008 +#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7) +#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5) #define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3) #define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2) #define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1) #define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0) #define SSS_REG_FCINTPEND 0x000C +#define SSS_FCINTPEND_HPARTINTPBIT(7) +#define SSS_FCINTPEND_HDONEINTPBIT(5) #define SSS_FCINTPEND_BRDMAINTP BIT(3) #define SSS_FCINTPEND_BTDMAINTP BIT(2) #define SSS_FCINTPEND_HRDMAINTP BIT(1) @@ -72,6 +88,7 @@ #define SSS_HASHIN_INDEPENDENT _SBF(0, 0x00) #define SSS_HASHIN_CIPHER_INPUT _SBF(0, 0x01) #define SSS_HASHIN_CIPHER_OUTPUT_SBF(0, 0x02) +#define SSS_H
Re: [PATCH] crypto: s5p-sss: Add HASH support for Exynos
Hi Krzysztof, On 13.09.2017 15:18, Krzysztof Kozlowski wrote: > On Wed, Sep 13, 2017 at 2:44 PM, Kamil Konieczny > <k.koniec...@partner.samsung.com> wrote: >> Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. >> It uses the crypto framework asynchronous hash api. >> It is based on omap-sham.c driver. >> S5P has some HW differencies and is not implemented. >> >> Modifications in s5p-sss: >> >> - Add hash supporting structures and functions. >> >> - Modify irq handler to handle both aes and hash signals. >> >> - Disable HASH in probe if Exynos PRNG is enabled. >> >> - Add new copyright line and new author. >> >> - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 >> with crypto run-time self test testmgr >> and with tcrypt module with: modprobe tcrypt sec=1 mode=N >> where N=402, 403, 404 (MD5, SHA1, SHA256). >> >> Modifications in drivers/crypto/Kconfig: >> >> - Select sw algorithms MD5, SHA1 and SHA256 in S5P >> as they are nedded for fallback. >> >> Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com> >> --- >> drivers/crypto/Kconfig |6 + >> drivers/crypto/s5p-sss.c | 2062 >> +++--- >> 2 files changed, 1939 insertions(+), 129 deletions(-) >> > > Nice work, thanks! > > You need to split the patch, it is just too huge making it very > difficult to review. Please split it per logically sensible > improvements. Can you suggest how to break it up ? It is now big update, added working functionallity in one piece, but I agree it can be hard to read as git did some strange things, like delete few aes functions (and mixing this delete with '+' lines) and then adding the same aes without any change. > I see some cleanups mixed with new features - this > definitely must be split out. What cleanups do you see ? You mean this one "#if 0" ? > This looks more or less like an early work or RFC... because I see > things like "#if 0", You are right, I will remove it. > "HACK" or "CONFIG_". If so, ask the question > about your problems directly. Do not force readers to find them out... The problem is in dts there are described only AES registers (the range is too small), and HASH and PRNG uses the same HASH_CONTROL_1 register (setting SecSS engine). Both AES and HASH must use FC control registers for irq and flow setting. So this 'hack' prevents using both exynos-prng and s5p-sss HASH, and in presence of exynos-prng it will load only s5p-sss aes part. This will disable exynos-prng in scenario: remove exynos-prng remove s5p-sss load s5p-sss now exynos-prng module will not load. -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH] crypto: s5p-sss: Add HASH support for Exynos
Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW.
It uses the crypto framework asynchronous hash api.
It is based on omap-sham.c driver.
S5P has some HW differencies and is not implemented.
Modifications in s5p-sss:
- Add hash supporting structures and functions.
- Modify irq handler to handle both aes and hash signals.
- Disable HASH in probe if Exynos PRNG is enabled.
- Add new copyright line and new author.
- Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6
with crypto run-time self test testmgr
and with tcrypt module with: modprobe tcrypt sec=1 mode=N
where N=402, 403, 404 (MD5, SHA1, SHA256).
Modifications in drivers/crypto/Kconfig:
- Select sw algorithms MD5, SHA1 and SHA256 in S5P
as they are nedded for fallback.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
drivers/crypto/Kconfig |6 +
drivers/crypto/s5p-sss.c | 2062 +++---
2 files changed, 1939 insertions(+), 129 deletions(-)
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index fe33c199fc1a..a8bc7e73c6bc 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -434,10 +434,16 @@ config CRYPTO_DEV_S5P
depends on HAS_IOMEM && HAS_DMA
select CRYPTO_AES
select CRYPTO_BLKCIPHER
+ select CRYPTO_SHA1
+ select CRYPTO_MD5
+ select CRYPTO_SHA256
help
This option allows you to have support for S5P crypto acceleration.
Select this to offload Samsung S5PV210 or S5PC110, Exynos from AES
algorithms execution.
+ Select this to offload Exynos from HASH MD5/SHA1/SHA256.
+ HASH algorithms will be disabled in runtime if EXYNOS_RNG
+ was enabled due to hw conflict.
config CRYPTO_DEV_NX
bool "Support for IBM PowerPC Nest (NX) cryptographic acceleration"
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 7ac657f46d15..ed2739198508 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -1,18 +1,21 @@
/*
* Cryptographic API.
*
- * Support for Samsung S5PV210 HW acceleration.
+ * Support for Samsung S5PV210 and Exynos HW acceleration.
*
* Copyright (C) 2011 NetUP Inc. All rights reserved.
+ * Copyright (c) 2017 Samsung Electronics Co., Ltd. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as published
* by the Free Software Foundation.
*
+ * Hash part based on omap-sham.c driver.
*/
#include
#include
+#include
#include
#include
#include
@@ -30,28 +33,67 @@
#include
#include
+#include
+#include
+#include
+#include
+
#define _SBF(s, v) ((v) << (s))
+#ifdef DEBUG
+
+static int flow_debug_logging;
+static int flow_debug_dump;
+
+/* from crypto/bcm/util.h */
+#define FLOW_LOG(...) \
+ do {\
+ if (flow_debug_logging) { \
+ printk(__VA_ARGS__);\
+ } \
+ } while (0)
+#define FLOW_DUMP(msg, var, var_len) \
+ do {\
+ if (flow_debug_dump) { \
+ print_hex_dump(KERN_ALERT, msg, DUMP_PREFIX_NONE, \
+ 16, 1, var, var_len, false); \
+ } \
+ } while (0)
+#else /* !DEBUG */
+
+#define FLOW_LOG(...) do {} while (0)
+#define FLOW_DUMP(msg, var, var_len) do {} while (0)
+
+#endif /* DEBUG */
+
/* Feed control registers */
#define SSS_REG_FCINTSTAT 0x
+#define SSS_FCINTSTAT_HPARTINT BIT(7)
+#define SSS_FCINTSTAT_HDONEINT BIT(5)
#define SSS_FCINTSTAT_BRDMAINT BIT(3)
#define SSS_FCINTSTAT_BTDMAINT BIT(2)
#define SSS_FCINTSTAT_HRDMAINT BIT(1)
#define SSS_FCINTSTAT_PKDMAINT BIT(0)
#define SSS_REG_FCINTENSET 0x0004
+#define SSS_FCINTENSET_HPARTINTENSET BIT(7)
+#define SSS_FCINTENSET_HDONEINTENSET BIT(5)
#define SSS_FCINTENSET_BRDMAINTENSETBIT(3)
#define SSS_FCINTENSET_BTDMAINTENSETBIT(2)
#define SSS_FCINTENSET_HRDMAINTENSETBIT(1)
#define SSS_FCINTENSET_PKDMAINTENSETBIT(0)
#define SSS_REG_FCINTENCLR 0x0008
+#define SSS_FCINTENCLR_HPARTINTENCLR BIT(7)
+#define SSS_FCINTENCLR_HDONEINTENCLR BIT(5)
#define SSS_FCINTENCLR_BRDMAINTENCLRBIT(3)
#define SSS_FCINTENCLR_BTDMAINTENCLRBIT(2)
#define SSS_FCINTENCLR_HRDMAINTENCLRBIT(1)
#define SSS_FCINTENCLR_PKDMAINTENCLRBIT(0)
#define SSS_REG_FCINTPEND 0x000C
+#define SSS_FCINTPEND_HPARTINTPBIT(7)
+#define SSS_FCINTPEND_HDONEINTPBIT(5)
#define SSS_FCINTPEND_BRDMAINTP BIT(3)
#define SSS_FCINTPEND_BTD
[PATCH] crypto: doc - clarify return values for async hash methods
>From af1b10e1e6aaf67f8dc45ed78de89b0469794a98 Mon Sep 17 00:00:00 2001
From: Kamil Konieczny <k.koniec...@partner.samsung.com>
Date: Thu, 17 Aug 2017 12:11:36 +0200
Subject: [PATCH] crypto: doc - clarify return values for async hash methods
* fix documentation of return values for crypto_ahash_init(),
crypto_ahash_finup(), crypto_ahash_final(),
crypto_ahash_digest() and crypto_ahash_update()
Also while at it:
* add notes for device driver developers in struct ahash_alg
description
* fix description of @final method in struct ahash_alg
* fix typo in crypto_ahash_finup() description
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
include/crypto/hash.h | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index b5727bcd2336..0ed31fd80242 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -75,6 +75,7 @@ struct ahash_request {
* state of the HASH transformation at the beginning. This shall fill in
* the internal structures used during the entire duration of the whole
* transformation. No data processing happens at this point.
+ * Note: mandatory.
* @update: Push a chunk of data into the driver for transformation. This
*function actually pushes blocks of data from upper layers into the
*driver, which then passes those to the hardware as seen fit. This
@@ -84,16 +85,20 @@ struct ahash_request {
*context, as this function may be called in parallel with the same
*transformation object. Data processing can happen synchronously
*[SHASH] or asynchronously [AHASH] at this point.
+ *Note: mandatory.
* @final: Retrieve result from the driver. This function finalizes the
*transformation and retrieves the resulting hash from the driver and
*pushes it back to upper layers. No data processing happens at this
- *point.
+ *point unless hardware requires it to finish the transformation
+ *(then the data buffered by the device driver is processed).
+ *Note: mandatory.
* @finup: Combination of @update and @final. This function is effectively a
*combination of @update and @final calls issued in sequence. As some
*hardware cannot do @update and @final separately, this callback was
*added to allow such hardware to be used at least by IPsec. Data
*processing can happen synchronously [SHASH] or asynchronously [AHASH]
*at this point.
+ *Note: optional.
* @digest: Combination of @init and @update and @final. This function
* effectively behaves as the entire chain of operations, @init,
* @update and @final issued in sequence. Just like @finup, this was
@@ -416,11 +421,10 @@ static inline bool crypto_ahash_has_setkey(struct
crypto_ahash *tfm)
* needed to perform the cipher operation
*
* This function is a "short-hand" for the function calls of
- * crypto_ahash_update and crypto_shash_final. The parameters have the same
+ * crypto_ahash_update and crypto_ahash_final. The parameters have the same
* meaning as discussed for those separate functions.
*
- * Return: 0 if the message digest creation was successful; < 0 if an error
- *occurred
+ * Return: see crypto_ahash_final()
*/
int crypto_ahash_finup(struct ahash_request *req);
@@ -433,8 +437,11 @@ int crypto_ahash_finup(struct ahash_request *req);
* based on all data added to the cipher handle. The message digest is placed
* into the output buffer registered with the ahash_request handle.
*
- * Return: 0 if the message digest creation was successful; < 0 if an error
- *occurred
+ * Return:
+ * 0 if the message digest was successfully calculated;
+ * -EINPROGRESSif data is feeded into hardware (DMA) or queued for
later;
+ * -EBUSY if queue is full and request should be resubmitted later;
+ * other < 0 if an error occurred
*/
int crypto_ahash_final(struct ahash_request *req);
@@ -447,8 +454,7 @@ int crypto_ahash_final(struct ahash_request *req);
* crypto_ahash_update and crypto_ahash_final. The parameters have the same
* meaning as discussed for those separate three functions.
*
- * Return: 0 if the message digest creation was successful; < 0 if an error
- *occurred
+ * Return: see crypto_ahash_final()
*/
int crypto_ahash_digest(struct ahash_request *req);
@@ -493,8 +499,7 @@ static inline int crypto_ahash_import(struct ahash_request
*req, const void *in)
* handle. Any potentially existing state created by previous operations is
* discarded.
*
- * Return: 0 if the message digest initialization was successful; < 0 if an
- *error occurred
+ * Return: see crypto_ahash_final()
*/
static inline int crypto_ahash_init(struct ahash_request *req)
{
@@ -510,8 +515,7 @@ static inline int cryp
Re: [PATCH 2/2] crypto: stm32 - Support for STM32 HASH module
On 13.07.2017 15:32, Lionel Debieve wrote:
> This module register a HASH module that support multiples
> algorithms: MD5, SHA1, SHA224, SHA256. [...]
> +static irqreturn_t stm32_hash_irq_thread(int irq, void *dev_id)
> +{
> + struct stm32_hash_dev *hdev = dev_id;
> + int err;
The 'err' var is used without initialize.
> +
> + if (HASH_FLAGS_CPU & hdev->flags) {
> + if (HASH_FLAGS_OUTPUT_READY & hdev->flags) {
> + hdev->flags &= ~HASH_FLAGS_OUTPUT_READY;
> + goto finish;
> + }
> + } else if (HASH_FLAGS_DMA_READY & hdev->flags) {
> + if (HASH_FLAGS_DMA_ACTIVE & hdev->flags) {
> + hdev->flags &= ~HASH_FLAGS_DMA_ACTIVE;
> + goto finish;
> + }
> + }
> +
> + return IRQ_HANDLED;
> +
> +finish:
> + /*Finish current request */
> + stm32_hash_finish_req(hdev->req, err);
> +
> + return IRQ_HANDLED;
> +}
> +
and here is beginnig for finish_req:
+static void stm32_hash_finish_req(struct ahash_request *req, int err)
+{
+ struct stm32_hash_request_ctx *rctx = ahash_request_ctx(req);
+ struct stm32_hash_dev *hdev = rctx->hdev;
+
+ if (!err && (HASH_FLAGS_FINAL & hdev->flags)) {
--
Best regards,
Kamil Konieczny
Samsung R Institute Poland
Re: [RFC Part2 PATCH v3 02/26] crypto: ccp: Add Platform Security Processor (PSP) device support
Hi, minor misspelling, On 24.07.2017 22:02, Brijesh Singh wrote: > Platform Security Processor (PSP) is part of AMD Secure Processor (AMD-SP), > PSP is a dedicated processor that provides the support for key management > commands in a Secure Encrypted Virtualiztion (SEV) mode, along with > software-based Tursted Executation Environment (TEE) to enable the - ^ Trusted > third-party tursted applications. -- ^ trusted [...] -- Best regards, Kamil Konieczny Samsung R Institute Poland
Re: HELP writing crypto driver for HW with blocksize hash
On 11.07.2017 12:30, Gilad Ben-Yossef wrote: > On Tue, Jul 11, 2017 at 10:52 AM, Kamil Konieczny > <k.koniec...@partner.samsung.com> wrote: >> >> >> I am writing crypto driver for hash MD5/SHA1/SHA256 on Exynos 4412, >> and I am facing some (minor?) difficulties. [...] >> So there is no [...] final method. >> >> It must be feeded with at least one message byte to produce hash. >> >> One more constrain is to process data in constant-sized chunks, >> here it is 64 bytes, the same as for AES block, >> i.e. it cannot stop and export state while in middle of block, >> example - if we feed 16 bytes, we must then feed 48 bytes >> to stop, but ideally we should feed it always 64 bytes. [...] >> >> Any suggestions ? >> >> Can i keep some bytes unfeeded from ahash_request >> and return -EINPROGRESS ? >> Should i set timer and copy rest bytes after some timeout, >> where no more requests are incoming ? Or not ? cause it is >> async mode ? >> can i wait for more requests for processing waiting one ? > Your two constraints are actually inter-related - > > If you can only feed the HW a constant size chunk, than indeed need to > keey bytes fed > to the driver the are below the chunk size in a software buffer, but > than you need the final() > method to feed these bytes (padded as needed) to the HW if they are > the last bytes > > Gilad HW will done padding I want to avoid memcpy iff possible for async hash -- Best regards, Kamil Konieczny Samsung R Institute Poland
HELP writing crypto driver for HW with blocksize hash
Hi, I am writing crypto driver for hash MD5/SHA1/SHA256 on Exynos 4412, and I am facing some (minor?) difficulties. In old days, hadware (HW) can only do basic hash block operation, so at the end it needed to finalize hash, and driver need to write some bits into buffer to get message hash. Time passes, and hardware (HW) was designed with improved cappabilities, so it can add itself bits after message and calculate hash, it can stop then export state, import and resume, but ... it can not process null-length (or zero-length) ending. So there is no more final method. It must be feeded with at least one message byte to produce hash. One more constrain is to process data in constant-sized chunks, here it is 64 bytes, the same as for AES block, i.e. it cannot stop and export state while in middle of block, example - if we feed 16 bytes, we must then feed 48 bytes to stop, but ideally we should feed it always 64 bytes. Some crypto drivers with similar problem(s): omap-sham.c - no final and blocksize needed, broadcom bcm/spu.c - no final, ccp/ccp-crypto-sha.c - no final, nx/nx-sha256.c - blocksize needed, One more thing - in algorithm description for methods: final, finup, update, digest, export, import there is note that finup is for those hardware that cannot do final, but again, it looks like crypto framework is ignoring that and every finup is translated into "update, final". HW driver will do opposite - it will translate final into finup. >From this follows that for every such HW crypto drivers authors duplicate code for keeping at least blocksize cache of message. One more point - use of block size in algo structure. It is for informing framework about HW limitation, but seems to be ignored again... Any suggestions ? Can i keep some bytes unfeeded from ahash_request and return -EINPROGRESS ? Should i set timer and copy rest bytes after some timeout, where no more requests are incoming ? Or not ? cause it is async mode ? can i wait for more requests for processing waiting one ? -- Best regards, Kamil Konieczny Samsung R Institute Poland
[PATCH] Documentation: crypto: Fixed bugs, added example usage of calc_hash().
- Fixed bugs in example for shash and rng (added missing "*" and " *").
- Corrected pr_info() in calc_hash().
- Added example usage of calc_hash().
- No need for negate PTR_ERR to get error code, as crypto_alloc_rng
already returns negative values like ERR_PTR(-ENOMEM). Fixed.
Signed-off-by: Kamil Konieczny <k.koniec...@partner.samsung.com>
---
Documentation/crypto/api-samples.rst | 38 ++--
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/Documentation/crypto/api-samples.rst
b/Documentation/crypto/api-samples.rst
index d021fd96a76d..2531948db89f 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -155,9 +155,9 @@ Code Example For Use of Operational State Memory With SHASH
char ctx[];
};
-static struct sdesc init_sdesc(struct crypto_shash *alg)
+static struct sdesc *init_sdesc(struct crypto_shash *alg)
{
-struct sdesc sdesc;
+struct sdesc *sdesc;
int size;
size = sizeof(struct shash_desc) + crypto_shash_descsize(alg);
@@ -169,15 +169,16 @@ Code Example For Use of Operational State Memory With
SHASH
return sdesc;
}
-static int calc_hash(struct crypto_shashalg,
- const unsigned chardata, unsigned int datalen,
- unsigned chardigest) {
-struct sdesc sdesc;
+static int calc_hash(struct crypto_shash *alg,
+ const unsigned char *data, unsigned int datalen,
+ unsigned char *digest)
+{
+struct sdesc *sdesc;
int ret;
sdesc = init_sdesc(alg);
if (IS_ERR(sdesc)) {
-pr_info("trusted_key: can't alloc %s\n", hash_alg);
+pr_info("can't alloc sdesc\n");
return PTR_ERR(sdesc);
}
@@ -186,6 +187,23 @@ Code Example For Use of Operational State Memory With SHASH
return ret;
}
+static int test_hash(const unsigned char *data, unsigned int datalen,
+ unsigned char *digest)
+{
+struct crypto_shash *alg;
+char *hash_alg_name = "sha1-padlock-nano";
+int ret;
+
+alg = crypto_alloc_shash(hash_alg_name, CRYPTO_ALG_TYPE_SHASH, 0);
+if (IS_ERR(alg)) {
+pr_info("can't alloc alg %s\n", hash_alg_name);
+return PTR_ERR(alg);
+}
+ret = calc_hash(alg, data, datalen, digest);
+crypto_free_shash(alg);
+return ret;
+}
+
Code Example For Random Number Generator Usage
--
@@ -195,8 +213,8 @@ Code Example For Random Number Generator Usage
static int get_random_numbers(u8 *buf, unsigned int len)
{
-struct crypto_rngrng = NULL;
-chardrbg = "drbg_nopr_sha256"; /* Hash DRBG with SHA-256, no PR */
+struct crypto_rng *rng = NULL;
+char *drbg = "drbg_nopr_sha256"; /* Hash DRBG with SHA-256, no PR */
int ret;
if (!buf || !len) {
@@ -207,7 +225,7 @@ Code Example For Random Number Generator Usage
rng = crypto_alloc_rng(drbg, 0, 0);
if (IS_ERR(rng)) {
pr_debug("could not allocate RNG handle for %s\n", drbg);
-return -PTR_ERR(rng);
+return PTR_ERR(rng);
}
ret = crypto_rng_get_bytes(rng, buf, len);
--
2.7.4
