Re: [PATCH] crypto: sm3 - use the more precise type u32 instead of unsigned int
Hi,
Thank you for the patch!
On Fri, Mar 26, 2021 at 5:21 AM Tianjia Zhang
wrote:
>
> In the process of calculating the hash, use the more accurate type
> 'u32' instead of the original 'unsigned int' to avoid ambiguity.
I don't think there is any ambiguity here, as both forms are always
the same size.
Generally, I tend to use the convention of using 'u32' as denoting
variables where the size is meaningful - e.g. mathematical operations
that are defined in the standard on 32 bit buffers, versus using
plain 'int' types where it isn't - e.g. loop counters etc.
Having said that, even under my own definition possibly the w and wt
arrays in sm3_trandform() should be changed to u32.
I don't object to changing those if it bugs you :-)
Cheers,
Gilad
> Signed-off-by: Tianjia Zhang
> ---
> crypto/sm3_generic.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/crypto/sm3_generic.c b/crypto/sm3_generic.c
> index 193c4584bd00..562e96f92f64 100644
> --- a/crypto/sm3_generic.c
> +++ b/crypto/sm3_generic.c
> @@ -36,17 +36,17 @@ static inline u32 p1(u32 x)
> return x ^ rol32(x, 15) ^ rol32(x, 23);
> }
>
> -static inline u32 ff(unsigned int n, u32 a, u32 b, u32 c)
> +static inline u32 ff(u32 n, u32 a, u32 b, u32 c)
> {
> return (n < 16) ? (a ^ b ^ c) : ((a & b) | (a & c) | (b & c));
> }
>
> -static inline u32 gg(unsigned int n, u32 e, u32 f, u32 g)
> +static inline u32 gg(u32 n, u32 e, u32 f, u32 g)
> {
> return (n < 16) ? (e ^ f ^ g) : ((e & f) | ((~e) & g));
> }
>
> -static inline u32 t(unsigned int n)
> +static inline u32 t(u32 n)
> {
> return (n < 16) ? SM3_T1 : SM3_T2;
> }
> @@ -54,7 +54,7 @@ static inline u32 t(unsigned int n)
> static void sm3_expand(u32 *t, u32 *w, u32 *wt)
> {
> int i;
> - unsigned int tmp;
> + u32 tmp;
>
> /* load the input */
> for (i = 0; i <= 15; i++)
> @@ -123,8 +123,8 @@ static void sm3_compress(u32 *w, u32 *wt, u32 *m)
>
> static void sm3_transform(struct sm3_state *sst, u8 const *src)
> {
> - unsigned int w[68];
> - unsigned int wt[64];
> + u32 w[68];
> + u32 wt[64];
>
> sm3_expand((u32 *)src, w, wt);
> sm3_compress(w, wt, sst->state);
> --
> 2.19.1.3.ge56e4f7
>
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
Re: [PATCH v3 0/4] crypto: switch to crypto API for EBOIV generation
Hi, On Thu, Oct 29, 2020 at 12:05 PM Gilad Ben-Yossef wrote: > > > This series creates an EBOIV template that produces a skcipher > transform which passes through all operations to the skcipher, while > using the same skcipher and key to encrypt the input IV, which is > assumed to be a sector offset, although this is not enforced. I hope I didn't miss anything, but it seems I didn't get an ACK, NACK or request of changes to the latest iteration. Any feedback is very much welcome. Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH 075/141] crypto: ccree - Fix fall-through warnings for Clang
On Fri, Nov 20, 2020 at 8:34 PM Gustavo A. R. Silva wrote: > > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple > warnings by explicitly adding multiple break statements instead of > letting the code fall through to the next case. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva > --- > drivers/crypto/ccree/cc_cipher.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/crypto/ccree/cc_cipher.c > b/drivers/crypto/ccree/cc_cipher.c > index dafa6577a845..cdfee501fbd9 100644 > --- a/drivers/crypto/ccree/cc_cipher.c > +++ b/drivers/crypto/ccree/cc_cipher.c > @@ -97,6 +97,7 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p, > u32 size) > case S_DIN_to_SM4: > if (size == SM4_KEY_SIZE) > return 0; > + break; > default: > break; > } > @@ -139,9 +140,11 @@ static int validate_data_size(struct cc_cipher_ctx > *ctx_p, > case DRV_CIPHER_CBC: > if (IS_ALIGNED(size, SM4_BLOCK_SIZE)) > return 0; > + break; > default: > break; > } > + break; > default: > break; > } > -- > 2.27.0 > Acked-by: Gilad Ben-Yossef Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
[PATCH v3] crypto: ccree - rework cache parameters handling
Rework the setting of DMA cache parameters, program more appropriate
values and explicitly set sharability domain.
Signed-off-by: Gilad Ben-Yossef
---
Changes from previous versions:
- After discussion with Rob H., drop notion of setting the parameters
from device tree and just use good defaults for cached/non cached.
drivers/crypto/ccree/cc_driver.c | 75 +---
drivers/crypto/ccree/cc_driver.h | 6 +--
drivers/crypto/ccree/cc_pm.c | 2 +-
3 files changed, 63 insertions(+), 20 deletions(-)
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 6f519d3e896c..d0e59e942568 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -100,6 +100,57 @@ static const struct of_device_id arm_ccree_dev_of_match[]
= {
};
MODULE_DEVICE_TABLE(of, arm_ccree_dev_of_match);
+static void init_cc_cache_params(struct cc_drvdata *drvdata)
+{
+ struct device *dev = drvdata_to_dev(drvdata);
+ u32 cache_params, ace_const, val, mask;
+
+ /* compute CC_AXIM_CACHE_PARAMS */
+ cache_params = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
+ dev_dbg(dev, "Cache params previous: 0x%08X\n", cache_params);
+
+ /* non cached or write-back, write allocate */
+ val = drvdata->coherent ? 0xb : 0x2;
+
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_AWCACHE);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_AWCACHE_LAST);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_ARCACHE);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ drvdata->cache_params = cache_params;
+
+ dev_dbg(dev, "Cache params current: 0x%08X\n", cache_params);
+
+ if (drvdata->hw_rev <= CC_HW_REV_710)
+ return;
+
+ /* compute CC_AXIM_ACE_CONST */
+ ace_const = cc_ioread(drvdata, CC_REG(AXIM_ACE_CONST));
+ dev_dbg(dev, "ACE-const previous: 0x%08X\n", ace_const);
+
+ /* system or outer-sharable */
+ val = drvdata->coherent ? 0x2 : 0x3;
+
+ mask = CC_GENMASK(CC_AXIM_ACE_CONST_ARDOMAIN);
+ ace_const &= ~mask;
+ ace_const |= FIELD_PREP(mask, val);
+
+ mask = CC_GENMASK(CC_AXIM_ACE_CONST_AWDOMAIN);
+ ace_const &= ~mask;
+ ace_const |= FIELD_PREP(mask, val);
+
+ dev_dbg(dev, "ACE-const current: 0x%08X\n", ace_const);
+
+ drvdata->ace_const = ace_const;
+}
+
static u32 cc_read_idr(struct cc_drvdata *drvdata, const u32 *idr_offsets)
{
int i;
@@ -218,9 +269,9 @@ bool cc_wait_for_reset_completion(struct cc_drvdata
*drvdata)
return false;
}
-int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe)
+int init_cc_regs(struct cc_drvdata *drvdata)
{
- unsigned int val, cache_params;
+ unsigned int val;
struct device *dev = drvdata_to_dev(drvdata);
/* Unmask all AXI interrupt sources AXI_CFG1 register */
@@ -245,19 +296,9 @@ int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe)
cc_iowrite(drvdata, CC_REG(HOST_IMR), ~val);
- cache_params = (drvdata->coherent ? CC_COHERENT_CACHE_PARAMS : 0x0);
-
- val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
-
- if (is_probe)
- dev_dbg(dev, "Cache params previous: 0x%08X\n", val);
-
- cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), cache_params);
- val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
-
- if (is_probe)
- dev_dbg(dev, "Cache params current: 0x%08X (expect: 0x%08X)\n",
- val, cache_params);
+ cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), drvdata->cache_params);
+ if (drvdata->hw_rev >= CC_HW_REV_712)
+ cc_iowrite(drvdata, CC_REG(AXIM_ACE_CONST), drvdata->ace_const);
return 0;
}
@@ -445,7 +486,9 @@ static int init_cc_resources(struct platform_device
*plat_dev)
}
dev_dbg(dev, "Registered to IRQ: %d\n", irq);
- rc = init_cc_regs(new_drvdata, true);
+ init_cc_cache_params(new_drvdata);
+
+ rc = init_cc_regs(new_drvdata);
if (rc) {
dev_err(dev, "init_cc_regs failed\n");
goto post_pm_err;
diff --git a/drivers/crypto/ccree/cc_driver.h b/drivers/crypto/ccree/cc_driver.h
index af77b2020350..12cd68d59baa 100644
--- a/drivers/crypto/ccree/cc_driver.h
+++ b/drivers/crypto/ccree/cc_driver.h
@@ -49,8 +49,6 @@ enum cc_std_body {
CC_STD_ALL = 0x3
};
-#define CC_COHERENT_CACHE_PARAMS 0xEEE
-
#define CC_PINS_FULL 0x0
#define CC_PINS_SLIM 0x9F
@@ -155,6 +153,8 @@ struct cc_drvdata {
int std_bodies;
bool sec_disabled;
u32 comp_mask;
+ u32 cache_params;
+ u32 a
Re: [PATCH 1/2] dt-bindings: crypto: update ccree optional params
Hi, On Tue, Nov 17, 2020 at 4:07 PM Robin Murphy wrote: > > On 2020-11-16 18:54, Rob Herring wrote: > > On Thu, Oct 22, 2020 at 1:18 AM Gilad Ben-Yossef > > wrote: > >> ... > > IMO if this is like PL330 where you just stick some raw AXI attributes > in a register and that's what goes out on transactions then it's not > really part of the platform description anyway, it's just driver policy. > If folks want to tweak the driver's behaviour for secret SoC-specific > performance optimisation, then give them some module parameters or a > sysfs interface. I'm assuming this has to be purely a performance thing, > because I can't see how two different integrations could reasonably > depend on different baseline attributes for correctness, and even if > they did then you'd be able to determine that from the compatible > strings, because they would be different, because those integrations > would be fundamentally *not* compatible with each other. > So, I checked internally where we get this requirement and it seems you are right. I'm dropping the Dt bindings and in fact the ability to customize those attributes beyond the basic coherent/non coherent configuration which we already support and will just update the setting to what we think is best. Thanks for clearing this up. Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH 1/2] dt-bindings: crypto: update ccree optional params
On Tue, Nov 17, 2020 at 4:58 PM Rob Herring wrote: > > > > > >> > These could also just be implied by the compatible string (and > > > >> > requiring > > > >> > an SoC specific one). > > > >> > > > >> hm... we could do it but this will require us to know (and publicly > > > >> acknowledge) of every SoC making use of this piece of hardware design. > > > > > > That's already a requirement in general. Sometimes we can avoid it, > > > but that's cases of getting lucky. > > > > > > >> There is currently no other part of the driver that needs this. > > > > > > If your DT is part of firmware, then waiting until adding some driver > > > feature or quirk based on a new DT property is too late. Whereas with > > > a SoC specific compatible, you can handle any new feature or quirk > > > without a DT change (e.g. just a stable kernel update). Some platforms > > > may not care about that model, but in general that's the policy we > > > follow. Not doing that, we end up with the DWC3 binding. > > > > > > A fallback compatible is how we avoid updating drivers for every > > > single SoC unless needed. > > > > OK, I now better understand what you meant and that does make some > > sense and I will defer to your better judgment about the proper way > > to do this. > > > > Having said that, there is still something that bugs me about this, > > even just at the level of better understanding why we do things: > > > > Way back when, before DT, we had SoC specific code that identified the > > SoC somehow and set things up in a SoC specific way. > > Then we introduced DT as a way to say - "hey look, this is how my > > busses looks like, these are the devices I have, deal with it" and I > > always assumed that this was meant as a way to release us from having > > SoC specific setup code. > > Yes, but in the end it's a judgement call as to what the boundary is. > Take clocks for example, in the beginning we were trying to describe > clocks on a mux/divider/gate level in DT. We realized this would > result in hundreds to thousands of DT nodes and it would never be > completely correct. So we model only the leaf clocks for the most part > and there's lots of SoC code for clock controller blocks still. > > The questions for having properties or not to ask is: > > Is this board specific? Yes, then use a property. > > Who needs to change this and when? Should/would you want to control > this in your PC BIOS or via a BIOS update? > > > Zero SoC code was never a goal. It was about a standard way to define > SoCs and having common frameworks (we have a common clock api, but not > implementation at the time). We have *way* less SoC code per SoC than > we used to. At the start of the DT conversion for Arm, we had ~400 > boards and now we're at ~1400 last I checked. > > > It seems now potentially every SoC vendor needs to modify not just the > > device tree source (which makes sense of course) but also the driver > > supporting their platform. > > It now looks like we've come a full circle to me :-) > > As I said, if the h/w is 'exactly the same' (hint: it rarely is), then > use a fallback compatible. Then the new SoC specific compatible is > there just in case. > > Think of compatible just as a VID/PID in PCI and USB land (though the > closest thing to a fallback there is class codes). They are the only > way we can uniquely identify h/w. Thanks Rob, this makes sense. Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH 1/2] dt-bindings: crypto: update ccree optional params
On Mon, Nov 16, 2020 at 8:54 PM Rob Herring wrote: > > On Thu, Oct 22, 2020 at 1:18 AM Gilad Ben-Yossef wrote: > > > > > > Hi again, > > > > Any opinion on the suggested below? > > Sorry, lost in the pile... No problem at all. I know how it is... > > > > > > On Tue, Sep 29, 2020 at 9:08 PM Gilad Ben-Yossef > > wrote: > >> > >> > >> On Wed, Sep 23, 2020 at 4:57 AM Rob Herring wrote: > >> > > >> > On Wed, Sep 16, 2020 at 10:19:49AM +0300, Gilad Ben-Yossef wrote: > >> > > Document ccree driver supporting new optional parameters allowing to > >> > > customize the DMA transactions cache parameters and ACE bus sharability > >> > > properties. > >> > > > >> > > Signed-off-by: Gilad Ben-Yossef > >> > > --- > >> > > Documentation/devicetree/bindings/crypto/arm-cryptocell.txt | 4 > >> > > 1 file changed, 4 insertions(+) > >> > > > >> > > diff --git > >> > > a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt > >> > > b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt > >> > > index 6130e6eb4af8..1a1603e457a8 100644 > >> > > --- a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt > >> > > +++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt > >> > > @@ -13,6 +13,10 @@ Required properties: > >> > > Optional properties: > >> > > - clocks: Reference to the crypto engine clock. > >> > > - dma-coherent: Present if dma operations are coherent. > >> > > +- awcache: Set write transactions cache attributes > >> > > +- arcache: Set read transactions cache attributes > >> > > >> > These could also just be implied by the compatible string (and requiring > >> > an SoC specific one). > >> > >> hm... we could do it but this will require us to know (and publicly > >> acknowledge) of every SoC making use of this piece of hardware design. > > That's already a requirement in general. Sometimes we can avoid it, > but that's cases of getting lucky. > > >> There is currently no other part of the driver that needs this. > > If your DT is part of firmware, then waiting until adding some driver > feature or quirk based on a new DT property is too late. Whereas with > a SoC specific compatible, you can handle any new feature or quirk > without a DT change (e.g. just a stable kernel update). Some platforms > may not care about that model, but in general that's the policy we > follow. Not doing that, we end up with the DWC3 binding. > > A fallback compatible is how we avoid updating drivers for every > single SoC unless needed. OK, I now better understand what you meant and that does make some sense and I will defer to your better judgment about the proper way to do this. Having said that, there is still something that bugs me about this, even just at the level of better understanding why we do things: Way back when, before DT, we had SoC specific code that identified the SoC somehow and set things up in a SoC specific way. Then we introduced DT as a way to say - "hey look, this is how my busses looks like, these are the devices I have, deal with it" and I always assumed that this was meant as a way to release us from having SoC specific setup code. It seems now potentially every SoC vendor needs to modify not just the device tree source (which makes sense of course) but also the driver supporting their platform. It now looks like we've come a full circle to me :-) Thanks! Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
[PATCH v2 1/2] dt-bindings: crypto: update ccree optional params
Document ccree driver supporting new optional parameters allowing to override cache parameters. Signed-off-by: Gilad Ben-Yossef Cc: Rob Herring --- Documentation/devicetree/bindings/crypto/arm-cryptocell.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt index 6130e6eb4af8..fb4ba4a3af4c 100644 --- a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt +++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt @@ -13,6 +13,10 @@ Required properties: Optional properties: - clocks: Reference to the crypto engine clock. - dma-coherent: Present if dma operations are coherent. +- cache-attributes: override default cache attributes with "write-through" or "write-back". + Default is write through, write allocate. +- sharability-domain: override default cache sharability domain with "inner-sharable". + Default is outer-sharable (712, 703, 713 only). Examples: -- 2.29.2
[PATCH v2 0/2] add optional cache params override from DT
Rework the setting of cache parameters, including optionally allowing overiding the defaults from device tree Changes from v1: - As per Rob Herring's suggestions, lose the distinction between read and write and limit options to a few that make sense. Also use strings to make the meaning of the setting clearer. Gilad Ben-Yossef (2): dt-bindings: crypto: update ccree optional params crypto: ccree - add custom cache params from DT file .../bindings/crypto/arm-cryptocell.txt| 4 + drivers/crypto/ccree/cc_driver.c | 100 +++--- drivers/crypto/ccree/cc_driver.h | 4 +- drivers/crypto/ccree/cc_pm.c | 2 +- 4 files changed, 92 insertions(+), 18 deletions(-) -- 2.29.2
[PATCH v2 2/2] crypto: ccree - add custom cache params from DT file
Add optinal ability to override cache parameters and
set new defaults.
Signed-off-by: Gilad Ben-Yossef
Cc: Rob Herring
---
drivers/crypto/ccree/cc_driver.c | 100 ++-
drivers/crypto/ccree/cc_driver.h | 4 +-
drivers/crypto/ccree/cc_pm.c | 2 +-
3 files changed, 88 insertions(+), 18 deletions(-)
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 6f519d3e896c..3064bd196ebc 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -100,6 +100,82 @@ static const struct of_device_id arm_ccree_dev_of_match[]
= {
};
MODULE_DEVICE_TABLE(of, arm_ccree_dev_of_match);
+static void init_cc_dt_params(struct cc_drvdata *drvdata)
+{
+ struct device *dev = drvdata_to_dev(drvdata);
+ struct device_node *np = dev->of_node;
+ u32 cache_params, ace_const, val, mask;
+ const char *str;
+ int rc;
+
+ /* register CC_AXIM_CACHE_PARAMS */
+ cache_params = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
+ dev_dbg(dev, "Cache params previous: 0x%08X\n", cache_params);
+
+ if (drvdata->coherent) {
+
+ rc = of_property_read_string(np, "cache-attributes", );
+
+ if (rc && !strcmp(str, "write-through"))
+ val = 0x6;
+ else if (rc && !strcmp(str, "write-back"))
+ val = 0xa;
+ else
+ val = 0xb; /* Write-Back Write-Allocate */
+ } else {
+ /* Non-cacheable */
+ val = 0x2;
+ }
+
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_AWCACHE);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_AWCACHE_LAST);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_ARCACHE);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ drvdata->cache_params = cache_params;
+
+ dev_dbg(dev, "Cache params current: 0x%08X\n", cache_params);
+
+ if (drvdata->hw_rev <= CC_HW_REV_710)
+ return;
+
+ /* register CC_AXIM_ACE_CONST */
+ ace_const = cc_ioread(drvdata, CC_REG(AXIM_ACE_CONST));
+ dev_dbg(dev, "ACE-const previous: 0x%08X\n", ace_const);
+
+ if (drvdata->coherent) {
+
+ rc = of_property_read_string(np, "sharability-domain", );
+
+ if (rc && !strcmp(str, "inner"))
+ val = 0x1;
+ else
+ val = 0x2; /* Outer Sharable */
+ } else {
+ /* System */
+ val = 0x3;
+ }
+
+ mask = CC_GENMASK(CC_AXIM_ACE_CONST_ARDOMAIN);
+ ace_const &= ~mask;
+ ace_const |= FIELD_PREP(mask, val);
+
+ mask = CC_GENMASK(CC_AXIM_ACE_CONST_AWDOMAIN);
+ ace_const &= ~mask;
+ ace_const |= FIELD_PREP(mask, val);
+
+ dev_dbg(dev, "ACE-const current: 0x%08X\n", ace_const);
+
+ drvdata->ace_const = ace_const;
+}
+
static u32 cc_read_idr(struct cc_drvdata *drvdata, const u32 *idr_offsets)
{
int i;
@@ -218,9 +294,9 @@ bool cc_wait_for_reset_completion(struct cc_drvdata
*drvdata)
return false;
}
-int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe)
+int init_cc_regs(struct cc_drvdata *drvdata)
{
- unsigned int val, cache_params;
+ unsigned int val;
struct device *dev = drvdata_to_dev(drvdata);
/* Unmask all AXI interrupt sources AXI_CFG1 register */
@@ -245,19 +321,9 @@ int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe)
cc_iowrite(drvdata, CC_REG(HOST_IMR), ~val);
- cache_params = (drvdata->coherent ? CC_COHERENT_CACHE_PARAMS : 0x0);
-
- val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
-
- if (is_probe)
- dev_dbg(dev, "Cache params previous: 0x%08X\n", val);
-
- cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), cache_params);
- val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
-
- if (is_probe)
- dev_dbg(dev, "Cache params current: 0x%08X (expect: 0x%08X)\n",
- val, cache_params);
+ cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), drvdata->cache_params);
+ if (drvdata->hw_rev >= CC_HW_REV_712)
+ cc_iowrite(drvdata, CC_REG(AXIM_ACE_CONST), drvdata->ace_const);
return 0;
}
@@ -445,7 +511,9 @@ static int init_cc_resources(struct platform_device
*plat_dev)
}
dev_dbg(dev, "Registered to IRQ: %d\n", irq);
- rc = init_cc_regs(new_drvdata, true);
+ init_cc_dt_params(new_drvdata);
+
+ rc = init_cc_regs(new_drvdata);
if (rc) {
Re: [PATCH v3 1/4] crypto: add eboiv as a crypto API template
Hi, On Fri, Oct 30, 2020 at 12:33 PM Milan Broz wrote: > > On 29/10/2020 11:05, Gilad Ben-Yossef wrote: > > > > +config CRYPTO_EBOIV > > + tristate "EBOIV support for block encryption" > > + default DM_CRYPT > > + select CRYPTO_CBC > > + help > > + Encrypted byte-offset initialization vector (EBOIV) is an IV > > + generation method that is used in some cases by dm-crypt for > > + supporting the BitLocker volume encryption used by Windows 8 > > + and onwards as a backwards compatible version in lieu of XTS > > + support. > > + > > + It uses the block encryption key as the symmetric key for a > > + block encryption pass applied to the sector offset of the block. > > + Additional details can be found at > > + https://www.jedec.org/sites/default/files/docs/JESD223C.pdf > > This page is not available. Are you sure this is the proper documentation? You need to register at the JEDEC web site to get the PDF. The registration is free though. It's the only standard I am aware of that describe this mode, as opposed to a paper. > > I think the only description we used (for dm-crypt) was original Ferguson's > Bitlocker doc: > https://download.microsoft.com/download/0/2/3/0238acaf-d3bf-4a6d-b3d6-0a0be4bbb36e/bitlockercipher200608.pdf Yes, the JEDEC has a reference to that as well, but the white paper doesn't actually describe the option without the diffuser. > > IIRC EBOIV was a shortcut I added to dm-crypt because we found no official > terminology for this IV. > And after lunchtime, nobody invented anything better, so it stayed as it is > now :-) Well, I still don't have any better name to offer, LOL :-) Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
[PATCH v3 3/4] dm crypt: switch to EBOIV crypto API template
Replace the explicit EBOIV handling in the dm-crypt driver with calls
into the crypto API, which now possesses the capability to perform
this processing within the crypto subsystem.
Signed-off-by: Gilad Ben-Yossef
---
drivers/md/dm-crypt.c | 61 ++-
1 file changed, 19 insertions(+), 42 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 148960721254..86b7c7ee3225 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -716,47 +716,18 @@ static int crypt_iv_random_gen(struct crypt_config *cc,
u8 *iv,
return 0;
}
-static int crypt_iv_eboiv_ctr(struct crypt_config *cc, struct dm_target *ti,
- const char *opts)
-{
- if (crypt_integrity_aead(cc)) {
- ti->error = "AEAD transforms not supported for EBOIV";
- return -EINVAL;
- }
-
- if (crypto_skcipher_blocksize(any_tfm(cc)) != cc->iv_size) {
- ti->error = "Block size of EBOIV cipher does "
- "not match IV size of block cipher";
- return -EINVAL;
- }
-
- return 0;
-}
-
static int crypt_iv_eboiv_gen(struct crypt_config *cc, u8 *iv,
struct dm_crypt_request *dmreq)
{
- u8 buf[MAX_CIPHER_BLOCKSIZE] __aligned(__alignof__(__le64));
- struct skcipher_request *req;
- struct scatterlist src, dst;
- struct crypto_wait wait;
- int err;
-
- req = skcipher_request_alloc(any_tfm(cc), GFP_NOIO);
- if (!req)
- return -ENOMEM;
-
- memset(buf, 0, cc->iv_size);
- *(__le64 *)buf = cpu_to_le64(dmreq->iv_sector * cc->sector_size);
- sg_init_one(, page_address(ZERO_PAGE(0)), cc->iv_size);
- sg_init_one(, iv, cc->iv_size);
- skcipher_request_set_crypt(req, , , cc->iv_size, buf);
- skcipher_request_set_callback(req, 0, crypto_req_done, );
- err = crypto_wait_req(crypto_skcipher_encrypt(req), );
- skcipher_request_free(req);
+ /*
+* ESSIV encryption of the IV is handled by the crypto API,
+* so compute and pass the sector offset here.
+*/
+ memset(iv, 0, cc->iv_size);
+ *(__le64 *)iv = cpu_to_le64(dmreq->iv_sector * cc->sector_size);
- return err;
+ return 0;
}
static void crypt_iv_elephant_dtr(struct crypt_config *cc)
@@ -771,18 +742,14 @@ static int crypt_iv_elephant_ctr(struct crypt_config *cc,
struct dm_target *ti,
const char *opts)
{
struct iv_elephant_private *elephant = >iv_gen_private.elephant;
- int r;
+ int r = 0;
elephant->tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
if (IS_ERR(elephant->tfm)) {
r = PTR_ERR(elephant->tfm);
elephant->tfm = NULL;
- return r;
}
- r = crypt_iv_eboiv_ctr(cc, ti, NULL);
- if (r)
- crypt_iv_elephant_dtr(cc);
return r;
}
@@ -1092,7 +1059,6 @@ static struct crypt_iv_operations crypt_iv_random_ops = {
};
static struct crypt_iv_operations crypt_iv_eboiv_ops = {
- .ctr = crypt_iv_eboiv_ctr,
.generator = crypt_iv_eboiv_gen
};
@@ -2739,6 +2705,15 @@ static int crypt_ctr_cipher_new(struct dm_target *ti,
char *cipher_in, char *key
cipher_api = buf;
}
+ if (*ivmode && (!strcmp(*ivmode, "eboiv") || !strcmp(*ivmode,
"elephant"))) {
+ ret = snprintf(buf, CRYPTO_MAX_ALG_NAME, "eboiv(%s)",
cipher_api);
+ if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME) {
+ ti->error = "Cannot allocate cipher string";
+ return -ENOMEM;
+ }
+ cipher_api = buf;
+ }
+
cc->key_parts = cc->tfms_count;
/* Allocate cipher */
@@ -2817,6 +2792,8 @@ static int crypt_ctr_cipher_old(struct dm_target *ti,
char *cipher_in, char *key
}
ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"essiv(%s(%s),%s)", chainmode, cipher, *ivopts);
+ } else if (*ivmode && (!strcmp(*ivmode, "eboiv") || !strcmp(*ivmode,
"elephant"))) {
+ ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"eboiv(%s(%s))", chainmode, cipher);
} else {
ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"%s(%s)", chainmode, cipher);
--
2.28.0
[PATCH v3 2/4] crypto: add eboiv(cbc(aes)) test vectors
Add test vectors for the use of the EBOIV template with cbc(aes)
modes as it is being used by dm-crypt for BitLocker support.
Vectors taken from dm-crypt test suite images.
Signed-off-by: Gilad Ben-Yossef
---
crypto/tcrypt.c | 9 ++
crypto/testmgr.c | 6 +
crypto/testmgr.h | 279 +++
3 files changed, 294 insertions(+)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 234b1adcfbcb..583c027fe8f5 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -2344,6 +2344,15 @@ static int do_test(const char *alg, u32 type, u32 mask,
int m, u32 num_mb)
NULL, 0, 16, 8, speed_template_16);
break;
+ case 222:
+ test_acipher_speed("eboiv(cbc(aes))",
+ ENCRYPT, sec, NULL, 0,
+ speed_template_16_32);
+ test_acipher_speed("eboiv(cbc(aes))",
+ DECRYPT, sec, NULL, 0,
+ speed_template_16_32);
+ break;
+
case 300:
if (alg) {
test_hash_speed(alg, sec, generic_hash_speed_template);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index a64a639eddfa..7d1f409fa807 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4779,6 +4779,12 @@ static const struct alg_test_desc alg_test_descs[] = {
.alg = "drbg_pr_sha512",
.fips_allowed = 1,
.test = alg_test_null,
+ }, {
+ .alg = "eboiv(cbc(aes))",
+ .test = alg_test_skcipher,
+ .suite = {
+ .cipher = __VECS(eboiv_aes_cbc_tv_template)
+ }
}, {
.alg = "ecb(aes)",
.test = alg_test_skcipher,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 8c83811c0e35..6429f115289d 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -32736,6 +32736,285 @@ static const struct comp_testvec
zstd_decomp_tv_template[] = {
},
};
+/* vectors taken from cryptosetup test suite images */
+static const struct cipher_testvec eboiv_aes_cbc_tv_template[] = {
+ {
+ /* cbc-aes-128 with 512b sector size, 0th sector */
+ .key= "\x6c\x96\xf8\x2a\x94\x2e\x87\x5f"
+ "\x02\x9c\x3d\xd9\xe4\x35\x17\x73",
+ .klen = 16,
+ .iv = "\x00\x50\x1a\x02\x00\x00\x00\x00"
+ "\x00\x00\x00\x00\x00\x00\x00\x00",
+ .ptext = "\xeb\x52\x90\x4e\x54\x46\x53\x20"
+ "\x20\x20\x20\x00\x02\x08\x00\x00"
+ "\x00\x00\x00\x00\x00\xf8\x00\x00"
+ "\x3f\x00\xff\x00\x00\x08\x00\x00"
+ "\x00\x00\x00\x00\x80\x00\x00\x00"
+ "\xff\x1f\x03\x00\x00\x00\x00\x00"
+ "\x55\x21\x00\x00\x00\x00\x00\x00"
+ "\x02\x00\x00\x00\x00\x00\x00\x00"
+ "\xf6\x00\x00\x00\x01\x00\x00\x00"
+ "\x13\x1e\xf1\xd4\x56\xf1\xd4\xf2"
+ "\x00\x00\x00\x00\xfa\x33\xc0\x8e"
+ "\xd0\xbc\x00\x7c\xfb\x68\xc0\x07"
+ "\x1f\x1e\x68\x66\x00\xcb\x88\x16"
+ "\x0e\x00\x66\x81\x3e\x03\x00\x4e"
+ "\x54\x46\x53\x75\x15\xb4\x41\xbb"
+ "\xaa\x55\xcd\x13\x72\x0c\x81\xfb"
+ "\x55\xaa\x75\x06\xf7\xc1\x01\x00"
+ "\x75\x03\xe9\xdd\x00\x1e\x83\xec"
+ "\x18\x68\x1a\x00\xb4\x48\x8a\x16"
+ "\x0e\x00\x8b\xf4\x16\x1f\xcd\x13"
+ "\x9f\x83\xc4\x18\x9e\x58\x1f\x72"
+ "\xe1\x3b\x06\x0b\x00\x75\xdb\xa3"
+ "\x0f\x00\xc1\x2e\x0f\x00\x04\x1e"
+ "\x5a\x33\xdb\xb9\x00\x20\x2b\xc8"
+ "\x66\xff\x06\x11\x00\x03\x16\x0f"
+ "\x00\x8e\xc2\xff\x06\x16\x00\xe8"
+ "\x4b\x00\x2b\xc8\x77\xef\xb8\x00"
+ "\xbb\xcd\x1a\x66\x23\xc0\x75\x2d"
+ "\x66\x81\xfb\x54\x43\x50\x41\x75"
+ "\x24\x81\xf9\x02\x01\x72\x1e\x16"
+ "\x68\x07\xbb\x16\x68\x52\x11\x16"
+ "\x68\x09\x00\x66\x53\x66\x53\x66"
+ "\x55\x16\x16\x16\x68\xb8\x01\x66"
+
[PATCH v3 4/4] crypto: ccree: re-introduce ccree eboiv support
BitLocker eboiv support, which was removed in
commit 1d8b41ff6991 ("crypto: ccree - remove bitlocker cipher")
is reintroduced based on the crypto API new support for
eboiv.
Signed-off-by: Gilad Ben-Yossef
Fixes: 1d8b41ff6991 ("crypto: ccree - remove bitlocker cipher")
---
drivers/crypto/ccree/cc_cipher.c | 132 +++
drivers/crypto/ccree/cc_crypto_ctx.h | 1 +
2 files changed, 96 insertions(+), 37 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index dafa6577a845..a13ae60189ed 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -74,10 +74,14 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p,
u32 size)
case S_DIN_to_AES:
switch (size) {
case CC_AES_128_BIT_KEY_SIZE:
- case CC_AES_192_BIT_KEY_SIZE:
if (ctx_p->cipher_mode != DRV_CIPHER_XTS)
return 0;
break;
+ case CC_AES_192_BIT_KEY_SIZE:
+ if (ctx_p->cipher_mode != DRV_CIPHER_XTS &&
+ ctx_p->cipher_mode != DRV_CIPHER_BITLOCKER)
+ return 0;
+ break;
case CC_AES_256_BIT_KEY_SIZE:
return 0;
case (CC_AES_192_BIT_KEY_SIZE * 2):
@@ -120,6 +124,7 @@ static int validate_data_size(struct cc_cipher_ctx *ctx_p,
case DRV_CIPHER_ECB:
case DRV_CIPHER_CBC:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
if (IS_ALIGNED(size, AES_BLOCK_SIZE))
return 0;
break;
@@ -345,7 +350,8 @@ static int cc_cipher_sethkey(struct crypto_skcipher *sktfm,
const u8 *key,
}
if (ctx_p->cipher_mode == DRV_CIPHER_XTS ||
- ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
+ ctx_p->cipher_mode == DRV_CIPHER_ESSIV ||
+ ctx_p->cipher_mode == DRV_CIPHER_BITLOCKER) {
if (hki.hw_key1 == hki.hw_key2) {
dev_err(dev, "Illegal hw key numbers (%d,%d)\n",
hki.hw_key1, hki.hw_key2);
@@ -543,6 +549,7 @@ static void cc_setup_readiv_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
/* IV */
hw_desc_init([*seq_size]);
set_setup_mode([*seq_size], SETUP_WRITE_STATE1);
@@ -597,6 +604,7 @@ static void cc_setup_state_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
break;
default:
dev_err(dev, "Unsupported cipher mode (%d)\n", cipher_mode);
@@ -616,56 +624,70 @@ static void cc_setup_xex_state_desc(struct crypto_tfm
*tfm,
int flow_mode = ctx_p->flow_mode;
int direction = req_ctx->gen_ctx.op_type;
dma_addr_t key_dma_addr = ctx_p->user.key_dma_addr;
- unsigned int key_len = (ctx_p->keylen / 2);
dma_addr_t iv_dma_addr = req_ctx->gen_ctx.iv_dma_addr;
- unsigned int key_offset = key_len;
+ unsigned int key_len;
+ unsigned int key_offset;
switch (cipher_mode) {
case DRV_CIPHER_ECB:
- break;
case DRV_CIPHER_CBC:
case DRV_CIPHER_CBC_CTS:
case DRV_CIPHER_CTR:
case DRV_CIPHER_OFB:
- break;
- case DRV_CIPHER_XTS:
- case DRV_CIPHER_ESSIV:
+ /* No secondary key for these ciphers, so just return */
+ return;
- if (cipher_mode == DRV_CIPHER_ESSIV)
- key_len = SHA256_DIGEST_SIZE;
+ case DRV_CIPHER_XTS:
+ /* Secondary key is same size as primary key and stored after
primary key */
+ key_len = ctx_p->keylen / 2;
+ key_offset = key_len;
+ break;
- /* load XEX key */
- hw_desc_init([*seq_size]);
- set_cipher_mode([*seq_size], cipher_mode);
- set_cipher_config0([*seq_size], direction);
- if (cc_key_type(tfm) == CC_HW_PROTECTED_KEY) {
- set_hw_crypto_key([*seq_size],
- ctx_p->hw.key2_slot);
- } else {
- set_din_type([*seq_size], DMA_DLLI,
-(key_dma_addr + key_offset),
-key_len, NS_BIT);
- }
- set_xex_data_unit_size([*seq_size], nbytes);
- set_flow_mode([*seq_size], S_DIN_to_AES
[PATCH v3 1/4] crypto: add eboiv as a crypto API template
Encrypted byte-offset initialization vector (EBOIV) is an IV
generation method that is used in some cases by dm-crypt for
supporting the BitLocker volume encryption used by Windows 8
and onwards as a backwards compatible version in lieu of XTS
support. Support for eboiv was added to dm-crypt in 5.6.
This patch re-implements eboiv as a generic crypto API
template, thus allowing use of a alternative architecture
specific optimzied implementations (as well as saving a
memory allocation along the way).
Signed-off-by: Gilad Ben-Yossef
Cc: Eric Biggers
---
crypto/Kconfig | 23 +
crypto/Makefile | 1 +
crypto/eboiv.c | 269
3 files changed, 293 insertions(+)
create mode 100644 crypto/eboiv.c
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 094ef56ab7b4..57676a2de01e 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -538,6 +538,29 @@ config CRYPTO_ESSIV
combined with ESSIV the only feasible mode for h/w accelerated
block encryption)
+config CRYPTO_EBOIV
+ tristate "EBOIV support for block encryption"
+ default DM_CRYPT
+ select CRYPTO_CBC
+ help
+ Encrypted byte-offset initialization vector (EBOIV) is an IV
+ generation method that is used in some cases by dm-crypt for
+ supporting the BitLocker volume encryption used by Windows 8
+ and onwards as a backwards compatible version in lieu of XTS
+ support.
+
+ It uses the block encryption key as the symmetric key for a
+ block encryption pass applied to the sector offset of the block.
+ Additional details can be found at
+ https://www.jedec.org/sites/default/files/docs/JESD223C.pdf
+
+ Note that the use of EBOIV is not recommended for new deployments,
+ and so this only needs to be enabled when interoperability with
+ existing encrypted volumes of filesystems is required, or when
+ building for a particular system that requires it (e.g., when
+ interop with BitLocker encrypted volumes of Windows systems
+ prior to Windows 10 is required)
+
comment "Hash modes"
config CRYPTO_CMAC
diff --git a/crypto/Makefile b/crypto/Makefile
index b279483fba50..9f00b2f1ad9c 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -174,6 +174,7 @@ obj-$(CONFIG_CRYPTO_ZSTD) += zstd.o
obj-$(CONFIG_CRYPTO_OFB) += ofb.o
obj-$(CONFIG_CRYPTO_ECC) += ecc.o
obj-$(CONFIG_CRYPTO_ESSIV) += essiv.o
+obj-$(CONFIG_CRYPTO_EBOIV) += eboiv.o
obj-$(CONFIG_CRYPTO_CURVE25519) += curve25519-generic.o
ecdh_generic-y += ecdh.o
diff --git a/crypto/eboiv.c b/crypto/eboiv.c
new file mode 100644
index ..4c364aad2ff2
--- /dev/null
+++ b/crypto/eboiv.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * EBOIV skcipher template for block encryption
+ *
+ * This template encapsulates the Encrypted byte-offset IV generation
algorithm
+ * used in Bitlocker in CBC mode by dm-crypt, which converts the initial vector
+ * for the skcipher used for block encryption, by encrypting it using the same
+ * skcipher key as encryption key. Usually, the input IV is a 64-bit sector
+ * offset (the byte offset of the start of the sector) in LE representation
+ * zero-padded to the size of the IV, but this * is not assumed by this driver.
+ *
+ * The typical use of this template is to instantiate the skcipher
+ * 'eboiv(cbc(aes))', which is the only instantiation used by
+ * dm-crypt for supporting BitLocker AES-CBC mode as specified in
+ * https://www.jedec.org/sites/default/files/docs/JESD223C.pdf
+ *
+ * Copyright (C) 2020 ARM Limited or its affiliates.
+ * Written by Gilad Ben-Yossef
+ *
+ * Heavily based on:
+ *
+ * ESSIV skcipher and aead template for block encryption
+ * Copyright (c) 2019 Linaro, Ltd.
+ *
+ * and
+ *
+ * dm-crypt eboiv code
+ * by Milan Broz and Ard Biesheuvel
+ *
+ */
+
+#include
+#include
+#include
+
+struct eboiv_instance_ctx {
+ struct crypto_skcipher_spawn skcipher_spawn;
+};
+
+struct eboiv_tfm_ctx {
+ struct crypto_skcipher *skcipher;
+};
+
+struct eboiv_req_ctx {
+ struct skcipher_request *req;
+ bool enc;
+ struct scatterlist src;
+ struct scatterlist dst;
+ u8 iv[MAX_CIPHER_BLOCKSIZE] __aligned(__alignof__(__le64));
+ struct skcipher_request subreq;
+};
+
+static int eboiv_skcipher_setkey(struct crypto_skcipher *tfm,
+const u8 *key, unsigned int keylen)
+{
+ struct eboiv_tfm_ctx *tctx = crypto_skcipher_ctx(tfm);
+
+ crypto_skcipher_clear_flags(tctx->skcipher, CRYPTO_TFM_REQ_MASK);
+ crypto_skcipher_set_flags(tctx->skcipher,
+ crypto_skcipher_get_flags(tfm) &
+ CRYPTO_TFM_REQ_MASK);
+ return crypto_skcipher_setkey(tctx->skcipher, key, keylen);
+}
+
+static void eboiv_skcipher_done(struct crypto_async_request *areq, int err)
+{
+
[PATCH v3 0/4] crypto: switch to crypto API for EBOIV generation
This series creates an EBOIV template that produces a skcipher transform which passes through all operations to the skcipher, while using the same skcipher and key to encrypt the input IV, which is assumed to be a sector offset, although this is not enforced. This matches dm-crypt use of EBOIV to provide BitLocker support, and so it is proposed as a replacement in patch #3. Replacing the dm-crypt specific EBOIV implementation to a Crypto API based one allows us to save a memory allocation per each request, as well as opening the way for use of compatible alternative transform providers, one of which, based on the Arm TrustZone CryptoCell hardware, is proposed as patch #4. Future potential work to allow encapsulating the handling of multiple subsequent blocks by the Crypto API may also benefit from this work. The code has been tested on both x86_64 virtual machine with the dm-crypt test suite and on an arm 32 bit board with the CryptoCell hardware. Since no offical source for eboiv test vectors is known, the test vectors supplied as patch #2 are derived from sectors which are part of the dm-crypt test suite. Signed-off-by: Gilad Ben-Yossef Cc: Eric Biggers Cc: Milan Broz Changes from v2: - Remove needless internal include left over by mistake. Changes from v1: - Incorporated feedback from Eric Biggers regarding eboiv template code. - Incorporated fixes for issues found by kernel test robot. - Moved from a Kconfig dependency of DM_CRYPT to EBOIV to EBOIV default of DM_CRYPT as suggested by Herbert Xu. Gilad Ben-Yossef (4): crypto: add eboiv as a crypto API template crypto: add eboiv(cbc(aes)) test vectors dm crypt: switch to EBOIV crypto API template crypto: ccree: re-introduce ccree eboiv support crypto/Kconfig | 23 +++ crypto/Makefile | 1 + crypto/eboiv.c | 269 ++ crypto/tcrypt.c | 9 + crypto/testmgr.c | 6 + crypto/testmgr.h | 279 +++ drivers/crypto/ccree/cc_cipher.c | 132 + drivers/crypto/ccree/cc_crypto_ctx.h | 1 + drivers/md/dm-crypt.c| 61 ++ 9 files changed, 702 insertions(+), 79 deletions(-) create mode 100644 crypto/eboiv.c -- 2.28.0
[PATCH v2 4/4] crypto: ccree: re-introduce ccree eboiv support
BitLocker eboiv support, which was removed in
commit 1d8b41ff6991 ("crypto: ccree - remove bitlocker cipher")
is reintroduced based on the crypto API new support for
eboiv.
Signed-off-by: Gilad Ben-Yossef
Fixes: 1d8b41ff6991 ("crypto: ccree - remove bitlocker cipher")
---
drivers/crypto/ccree/cc_cipher.c | 132 +++
drivers/crypto/ccree/cc_crypto_ctx.h | 1 +
2 files changed, 96 insertions(+), 37 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index dafa6577a845..a13ae60189ed 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -74,10 +74,14 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p,
u32 size)
case S_DIN_to_AES:
switch (size) {
case CC_AES_128_BIT_KEY_SIZE:
- case CC_AES_192_BIT_KEY_SIZE:
if (ctx_p->cipher_mode != DRV_CIPHER_XTS)
return 0;
break;
+ case CC_AES_192_BIT_KEY_SIZE:
+ if (ctx_p->cipher_mode != DRV_CIPHER_XTS &&
+ ctx_p->cipher_mode != DRV_CIPHER_BITLOCKER)
+ return 0;
+ break;
case CC_AES_256_BIT_KEY_SIZE:
return 0;
case (CC_AES_192_BIT_KEY_SIZE * 2):
@@ -120,6 +124,7 @@ static int validate_data_size(struct cc_cipher_ctx *ctx_p,
case DRV_CIPHER_ECB:
case DRV_CIPHER_CBC:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
if (IS_ALIGNED(size, AES_BLOCK_SIZE))
return 0;
break;
@@ -345,7 +350,8 @@ static int cc_cipher_sethkey(struct crypto_skcipher *sktfm,
const u8 *key,
}
if (ctx_p->cipher_mode == DRV_CIPHER_XTS ||
- ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
+ ctx_p->cipher_mode == DRV_CIPHER_ESSIV ||
+ ctx_p->cipher_mode == DRV_CIPHER_BITLOCKER) {
if (hki.hw_key1 == hki.hw_key2) {
dev_err(dev, "Illegal hw key numbers (%d,%d)\n",
hki.hw_key1, hki.hw_key2);
@@ -543,6 +549,7 @@ static void cc_setup_readiv_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
/* IV */
hw_desc_init([*seq_size]);
set_setup_mode([*seq_size], SETUP_WRITE_STATE1);
@@ -597,6 +604,7 @@ static void cc_setup_state_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
break;
default:
dev_err(dev, "Unsupported cipher mode (%d)\n", cipher_mode);
@@ -616,56 +624,70 @@ static void cc_setup_xex_state_desc(struct crypto_tfm
*tfm,
int flow_mode = ctx_p->flow_mode;
int direction = req_ctx->gen_ctx.op_type;
dma_addr_t key_dma_addr = ctx_p->user.key_dma_addr;
- unsigned int key_len = (ctx_p->keylen / 2);
dma_addr_t iv_dma_addr = req_ctx->gen_ctx.iv_dma_addr;
- unsigned int key_offset = key_len;
+ unsigned int key_len;
+ unsigned int key_offset;
switch (cipher_mode) {
case DRV_CIPHER_ECB:
- break;
case DRV_CIPHER_CBC:
case DRV_CIPHER_CBC_CTS:
case DRV_CIPHER_CTR:
case DRV_CIPHER_OFB:
- break;
- case DRV_CIPHER_XTS:
- case DRV_CIPHER_ESSIV:
+ /* No secondary key for these ciphers, so just return */
+ return;
- if (cipher_mode == DRV_CIPHER_ESSIV)
- key_len = SHA256_DIGEST_SIZE;
+ case DRV_CIPHER_XTS:
+ /* Secondary key is same size as primary key and stored after
primary key */
+ key_len = ctx_p->keylen / 2;
+ key_offset = key_len;
+ break;
- /* load XEX key */
- hw_desc_init([*seq_size]);
- set_cipher_mode([*seq_size], cipher_mode);
- set_cipher_config0([*seq_size], direction);
- if (cc_key_type(tfm) == CC_HW_PROTECTED_KEY) {
- set_hw_crypto_key([*seq_size],
- ctx_p->hw.key2_slot);
- } else {
- set_din_type([*seq_size], DMA_DLLI,
-(key_dma_addr + key_offset),
-key_len, NS_BIT);
- }
- set_xex_data_unit_size([*seq_size], nbytes);
- set_flow_mode([*seq_size], S_DIN_to_AES
[PATCH v2 0/4] crypto: switch to crypto API for EBOIV generation
This series creates an EBOIV template that produces a skcipher transform which passes through all operations to the skcipher, while using the same skcipher and key to encrypt the input IV, which is assumed to be a sector offset, although this is not enforced. This matches dm-crypt use of EBOIV to provide BitLocker support, and so it is proposed as a replacement in patch #3. Replacing the dm-crypt specific EBOIV implementation to a Crypto API based one allows us to save a memory allocation per each request, as well as opening the way for use of compatible alternative transform providers, one of which, based on the Arm TrustZone CryptoCell hardware, is proposed as patch #4. Future potential work to allow encapsulating the handling of multiple subsequent blocks by the Crypto API may also benefit from this work. The code has been tested on both x86_64 virtual machine with the dm-crypt test suite and on an arm 32 bit board with the CryptoCell hardware. Since no offical source for eboiv test vectors is known, the test vectors supplied as patch #2 are derived from sectors which are part of the dm-crypt test suite. Signed-off-by: Gilad Ben-Yossef Cc: Eric Biggers Cc: Milan Broz Changes from v1: - Incorporated feedback from Eric Biggers regarding eboiv template code. - Incorporated fixes for issues found by kernel test robot. - Moved from a Kconfig dependency of DM_CRYPT to EBOIV to EBOIV default of DM_CRYPT as suggested by Herbert Xu. Gilad Ben-Yossef (4): crypto: add eboiv as a crypto API template crypto: add eboiv(cbc(aes)) test vectors dm crypt: switch to EBOIV crypto API template crypto: ccree: re-introduce ccree eboiv support crypto/Kconfig | 23 +++ crypto/Makefile | 1 + crypto/eboiv.c | 271 ++ crypto/tcrypt.c | 9 + crypto/testmgr.c | 6 + crypto/testmgr.h | 279 +++ drivers/crypto/ccree/cc_cipher.c | 132 + drivers/crypto/ccree/cc_crypto_ctx.h | 1 + drivers/md/dm-crypt.c| 61 ++ 9 files changed, 704 insertions(+), 79 deletions(-) create mode 100644 crypto/eboiv.c -- 2.28.0
Re: [PATCH 3/4] dm crypt: switch to EBOIV crypto API template
On Mon, Oct 26, 2020 at 8:44 PM Herbert Xu wrote: > > On Tue, Oct 27, 2020 at 05:41:55AM +1100, Herbert Xu wrote: > > > > The point is that people rebuilding their kernel can end up with a > > broken system. Just set a default on EBOIV if dm-crypt is on. > > That's not enough as it's an existing option. So we need to > add a new Kconfig option with a default equal to dm-crypt. Sorry if I'm being daft, but what did you refer to be "an existing option"? there was no CONFIG_EBOIV before my patchset, it was simply built as part of dm-crypt so it seems that setting CONFIG_EBOIV default to dm-crypto Kconfig option value does solves the problem, or have I missed something? Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
[PATCH v2 2/4] crypto: add eboiv(cbc(aes)) test vectors
Add test vectors for the use of the EBOIV template with cbc(aes)
modes as it is being used by dm-crypt for BitLocker support.
Vectors taken from dm-crypt test suite images.
Signed-off-by: Gilad Ben-Yossef
---
crypto/tcrypt.c | 9 ++
crypto/testmgr.c | 6 +
crypto/testmgr.h | 279 +++
3 files changed, 294 insertions(+)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 234b1adcfbcb..583c027fe8f5 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -2344,6 +2344,15 @@ static int do_test(const char *alg, u32 type, u32 mask,
int m, u32 num_mb)
NULL, 0, 16, 8, speed_template_16);
break;
+ case 222:
+ test_acipher_speed("eboiv(cbc(aes))",
+ ENCRYPT, sec, NULL, 0,
+ speed_template_16_32);
+ test_acipher_speed("eboiv(cbc(aes))",
+ DECRYPT, sec, NULL, 0,
+ speed_template_16_32);
+ break;
+
case 300:
if (alg) {
test_hash_speed(alg, sec, generic_hash_speed_template);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index a64a639eddfa..7d1f409fa807 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4779,6 +4779,12 @@ static const struct alg_test_desc alg_test_descs[] = {
.alg = "drbg_pr_sha512",
.fips_allowed = 1,
.test = alg_test_null,
+ }, {
+ .alg = "eboiv(cbc(aes))",
+ .test = alg_test_skcipher,
+ .suite = {
+ .cipher = __VECS(eboiv_aes_cbc_tv_template)
+ }
}, {
.alg = "ecb(aes)",
.test = alg_test_skcipher,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 8c83811c0e35..6429f115289d 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -32736,6 +32736,285 @@ static const struct comp_testvec
zstd_decomp_tv_template[] = {
},
};
+/* vectors taken from cryptosetup test suite images */
+static const struct cipher_testvec eboiv_aes_cbc_tv_template[] = {
+ {
+ /* cbc-aes-128 with 512b sector size, 0th sector */
+ .key= "\x6c\x96\xf8\x2a\x94\x2e\x87\x5f"
+ "\x02\x9c\x3d\xd9\xe4\x35\x17\x73",
+ .klen = 16,
+ .iv = "\x00\x50\x1a\x02\x00\x00\x00\x00"
+ "\x00\x00\x00\x00\x00\x00\x00\x00",
+ .ptext = "\xeb\x52\x90\x4e\x54\x46\x53\x20"
+ "\x20\x20\x20\x00\x02\x08\x00\x00"
+ "\x00\x00\x00\x00\x00\xf8\x00\x00"
+ "\x3f\x00\xff\x00\x00\x08\x00\x00"
+ "\x00\x00\x00\x00\x80\x00\x00\x00"
+ "\xff\x1f\x03\x00\x00\x00\x00\x00"
+ "\x55\x21\x00\x00\x00\x00\x00\x00"
+ "\x02\x00\x00\x00\x00\x00\x00\x00"
+ "\xf6\x00\x00\x00\x01\x00\x00\x00"
+ "\x13\x1e\xf1\xd4\x56\xf1\xd4\xf2"
+ "\x00\x00\x00\x00\xfa\x33\xc0\x8e"
+ "\xd0\xbc\x00\x7c\xfb\x68\xc0\x07"
+ "\x1f\x1e\x68\x66\x00\xcb\x88\x16"
+ "\x0e\x00\x66\x81\x3e\x03\x00\x4e"
+ "\x54\x46\x53\x75\x15\xb4\x41\xbb"
+ "\xaa\x55\xcd\x13\x72\x0c\x81\xfb"
+ "\x55\xaa\x75\x06\xf7\xc1\x01\x00"
+ "\x75\x03\xe9\xdd\x00\x1e\x83\xec"
+ "\x18\x68\x1a\x00\xb4\x48\x8a\x16"
+ "\x0e\x00\x8b\xf4\x16\x1f\xcd\x13"
+ "\x9f\x83\xc4\x18\x9e\x58\x1f\x72"
+ "\xe1\x3b\x06\x0b\x00\x75\xdb\xa3"
+ "\x0f\x00\xc1\x2e\x0f\x00\x04\x1e"
+ "\x5a\x33\xdb\xb9\x00\x20\x2b\xc8"
+ "\x66\xff\x06\x11\x00\x03\x16\x0f"
+ "\x00\x8e\xc2\xff\x06\x16\x00\xe8"
+ "\x4b\x00\x2b\xc8\x77\xef\xb8\x00"
+ "\xbb\xcd\x1a\x66\x23\xc0\x75\x2d"
+ "\x66\x81\xfb\x54\x43\x50\x41\x75"
+ "\x24\x81\xf9\x02\x01\x72\x1e\x16"
+ "\x68\x07\xbb\x16\x68\x52\x11\x16"
+ "\x68\x09\x00\x66\x53\x66\x53\x66"
+ "\x55\x16\x16\x16\x68\xb8\x01\x66"
+
[PATCH v2 3/4] dm crypt: switch to EBOIV crypto API template
Replace the explicit EBOIV handling in the dm-crypt driver with calls
into the crypto API, which now possesses the capability to perform
this processing within the crypto subsystem.
Signed-off-by: Gilad Ben-Yossef
---
drivers/md/dm-crypt.c | 61 ++-
1 file changed, 19 insertions(+), 42 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 148960721254..86b7c7ee3225 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -716,47 +716,18 @@ static int crypt_iv_random_gen(struct crypt_config *cc,
u8 *iv,
return 0;
}
-static int crypt_iv_eboiv_ctr(struct crypt_config *cc, struct dm_target *ti,
- const char *opts)
-{
- if (crypt_integrity_aead(cc)) {
- ti->error = "AEAD transforms not supported for EBOIV";
- return -EINVAL;
- }
-
- if (crypto_skcipher_blocksize(any_tfm(cc)) != cc->iv_size) {
- ti->error = "Block size of EBOIV cipher does "
- "not match IV size of block cipher";
- return -EINVAL;
- }
-
- return 0;
-}
-
static int crypt_iv_eboiv_gen(struct crypt_config *cc, u8 *iv,
struct dm_crypt_request *dmreq)
{
- u8 buf[MAX_CIPHER_BLOCKSIZE] __aligned(__alignof__(__le64));
- struct skcipher_request *req;
- struct scatterlist src, dst;
- struct crypto_wait wait;
- int err;
-
- req = skcipher_request_alloc(any_tfm(cc), GFP_NOIO);
- if (!req)
- return -ENOMEM;
-
- memset(buf, 0, cc->iv_size);
- *(__le64 *)buf = cpu_to_le64(dmreq->iv_sector * cc->sector_size);
- sg_init_one(, page_address(ZERO_PAGE(0)), cc->iv_size);
- sg_init_one(, iv, cc->iv_size);
- skcipher_request_set_crypt(req, , , cc->iv_size, buf);
- skcipher_request_set_callback(req, 0, crypto_req_done, );
- err = crypto_wait_req(crypto_skcipher_encrypt(req), );
- skcipher_request_free(req);
+ /*
+* ESSIV encryption of the IV is handled by the crypto API,
+* so compute and pass the sector offset here.
+*/
+ memset(iv, 0, cc->iv_size);
+ *(__le64 *)iv = cpu_to_le64(dmreq->iv_sector * cc->sector_size);
- return err;
+ return 0;
}
static void crypt_iv_elephant_dtr(struct crypt_config *cc)
@@ -771,18 +742,14 @@ static int crypt_iv_elephant_ctr(struct crypt_config *cc,
struct dm_target *ti,
const char *opts)
{
struct iv_elephant_private *elephant = >iv_gen_private.elephant;
- int r;
+ int r = 0;
elephant->tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
if (IS_ERR(elephant->tfm)) {
r = PTR_ERR(elephant->tfm);
elephant->tfm = NULL;
- return r;
}
- r = crypt_iv_eboiv_ctr(cc, ti, NULL);
- if (r)
- crypt_iv_elephant_dtr(cc);
return r;
}
@@ -1092,7 +1059,6 @@ static struct crypt_iv_operations crypt_iv_random_ops = {
};
static struct crypt_iv_operations crypt_iv_eboiv_ops = {
- .ctr = crypt_iv_eboiv_ctr,
.generator = crypt_iv_eboiv_gen
};
@@ -2739,6 +2705,15 @@ static int crypt_ctr_cipher_new(struct dm_target *ti,
char *cipher_in, char *key
cipher_api = buf;
}
+ if (*ivmode && (!strcmp(*ivmode, "eboiv") || !strcmp(*ivmode,
"elephant"))) {
+ ret = snprintf(buf, CRYPTO_MAX_ALG_NAME, "eboiv(%s)",
cipher_api);
+ if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME) {
+ ti->error = "Cannot allocate cipher string";
+ return -ENOMEM;
+ }
+ cipher_api = buf;
+ }
+
cc->key_parts = cc->tfms_count;
/* Allocate cipher */
@@ -2817,6 +2792,8 @@ static int crypt_ctr_cipher_old(struct dm_target *ti,
char *cipher_in, char *key
}
ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"essiv(%s(%s),%s)", chainmode, cipher, *ivopts);
+ } else if (*ivmode && (!strcmp(*ivmode, "eboiv") || !strcmp(*ivmode,
"elephant"))) {
+ ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"eboiv(%s(%s))", chainmode, cipher);
} else {
ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"%s(%s)", chainmode, cipher);
--
2.28.0
[PATCH v2 1/4] crypto: add eboiv as a crypto API template
Encrypted byte-offset initialization vector (EBOIV) is an IV
generation method that is used in some cases by dm-crypt for
supporting the BitLocker volume encryption used by Windows 8
and onwards as a backwards compatible version in lieu of XTS
support. Support for eboiv was added to dm-crypt in 5.6.
This patch re-implements eboiv as a generic crypto API
template, thus allowing use of a alternative architecture
specific optimzied implementations (as well as saving a
memory allocation along the way).
Signed-off-by: Gilad Ben-Yossef
Cc: Eric Biggers
---
crypto/Kconfig | 23
crypto/Makefile | 1 +
crypto/eboiv.c | 271
3 files changed, 295 insertions(+)
create mode 100644 crypto/eboiv.c
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 094ef56ab7b4..57676a2de01e 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -538,6 +538,29 @@ config CRYPTO_ESSIV
combined with ESSIV the only feasible mode for h/w accelerated
block encryption)
+config CRYPTO_EBOIV
+ tristate "EBOIV support for block encryption"
+ default DM_CRYPT
+ select CRYPTO_CBC
+ help
+ Encrypted byte-offset initialization vector (EBOIV) is an IV
+ generation method that is used in some cases by dm-crypt for
+ supporting the BitLocker volume encryption used by Windows 8
+ and onwards as a backwards compatible version in lieu of XTS
+ support.
+
+ It uses the block encryption key as the symmetric key for a
+ block encryption pass applied to the sector offset of the block.
+ Additional details can be found at
+ https://www.jedec.org/sites/default/files/docs/JESD223C.pdf
+
+ Note that the use of EBOIV is not recommended for new deployments,
+ and so this only needs to be enabled when interoperability with
+ existing encrypted volumes of filesystems is required, or when
+ building for a particular system that requires it (e.g., when
+ interop with BitLocker encrypted volumes of Windows systems
+ prior to Windows 10 is required)
+
comment "Hash modes"
config CRYPTO_CMAC
diff --git a/crypto/Makefile b/crypto/Makefile
index b279483fba50..9f00b2f1ad9c 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -174,6 +174,7 @@ obj-$(CONFIG_CRYPTO_ZSTD) += zstd.o
obj-$(CONFIG_CRYPTO_OFB) += ofb.o
obj-$(CONFIG_CRYPTO_ECC) += ecc.o
obj-$(CONFIG_CRYPTO_ESSIV) += essiv.o
+obj-$(CONFIG_CRYPTO_EBOIV) += eboiv.o
obj-$(CONFIG_CRYPTO_CURVE25519) += curve25519-generic.o
ecdh_generic-y += ecdh.o
diff --git a/crypto/eboiv.c b/crypto/eboiv.c
new file mode 100644
index ..28b9e349b458
--- /dev/null
+++ b/crypto/eboiv.c
@@ -0,0 +1,271 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * EBOIV skcipher template for block encryption
+ *
+ * This template encapsulates the Encrypted byte-offset IV generation
algorithm
+ * used in Bitlocker in CBC mode by dm-crypt, which converts the initial vector
+ * for the skcipher used for block encryption, by encrypting it using the same
+ * skcipher key as encryption key. Usually, the input IV is a 64-bit sector
+ * offset (the byte offset of the start of the sector) in LE representation
+ * zero-padded to the size of the IV, but this * is not assumed by this driver.
+ *
+ * The typical use of this template is to instantiate the skcipher
+ * 'eboiv(cbc(aes))', which is the only instantiation used by
+ * dm-crypt for supporting BitLocker AES-CBC mode as specified in
+ * https://www.jedec.org/sites/default/files/docs/JESD223C.pdf
+ *
+ * Copyright (C) 2020 ARM Limited or its affiliates.
+ * Written by Gilad Ben-Yossef
+ *
+ * Heavily based on:
+ *
+ * ESSIV skcipher and aead template for block encryption
+ * Copyright (c) 2019 Linaro, Ltd.
+ *
+ * and
+ *
+ * dm-crypt eboiv code
+ * by Milan Broz and Ard Biesheuvel
+ *
+ */
+
+#include
+#include
+#include
+
+#include "internal.h"
+
+struct eboiv_instance_ctx {
+ struct crypto_skcipher_spawn skcipher_spawn;
+};
+
+struct eboiv_tfm_ctx {
+ struct crypto_skcipher *skcipher;
+};
+
+struct eboiv_req_ctx {
+ struct skcipher_request *req;
+ bool enc;
+ struct scatterlist src;
+ struct scatterlist dst;
+ u8 iv[MAX_CIPHER_BLOCKSIZE] __aligned(__alignof__(__le64));
+ struct skcipher_request subreq;
+};
+
+static int eboiv_skcipher_setkey(struct crypto_skcipher *tfm,
+const u8 *key, unsigned int keylen)
+{
+ struct eboiv_tfm_ctx *tctx = crypto_skcipher_ctx(tfm);
+
+ crypto_skcipher_clear_flags(tctx->skcipher, CRYPTO_TFM_REQ_MASK);
+ crypto_skcipher_set_flags(tctx->skcipher,
+ crypto_skcipher_get_flags(tfm) &
+ CRYPTO_TFM_REQ_MASK);
+ return crypto_skcipher_setkey(tctx->skcipher, key, keylen);
+}
+
+static void eboiv_skcipher_done
Re: [PATCH 3/4] dm crypt: switch to EBOIV crypto API template
On Mon, Oct 26, 2020 at 9:04 PM Milan Broz wrote: > > > > On 26/10/2020 19:39, Eric Biggers wrote: > > On Mon, Oct 26, 2020 at 07:29:57PM +0100, Milan Broz wrote: > >> On 26/10/2020 18:52, Eric Biggers wrote: > >>> On Mon, Oct 26, 2020 at 03:04:46PM +0200, Gilad Ben-Yossef wrote: > >>>> Replace the explicit EBOIV handling in the dm-crypt driver with calls > >>>> into the crypto API, which now possesses the capability to perform > >>>> this processing within the crypto subsystem. > >>>> > >>>> Signed-off-by: Gilad Ben-Yossef > >>>> > >>>> --- > >>>> drivers/md/Kconfig| 1 + > >>>> drivers/md/dm-crypt.c | 61 ++- > >>>> 2 files changed, 20 insertions(+), 42 deletions(-) > >>>> > >>>> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig > >>>> index 30ba3573626c..ca6e56a72281 100644 > >>>> --- a/drivers/md/Kconfig > >>>> +++ b/drivers/md/Kconfig > >>>> @@ -273,6 +273,7 @@ config DM_CRYPT > >>>>select CRYPTO > >>>>select CRYPTO_CBC > >>>>select CRYPTO_ESSIV > >>>> + select CRYPTO_EBOIV > >>>>help > >>>> This device-mapper target allows you to create a device that > >>>> transparently encrypts the data on it. You'll need to activate > >>> > >>> Can CRYPTO_EBOIV please not be selected by default? If someone really > >>> wants > >>> Bitlocker compatibility support, they can select this option themselves. > >> > >> Please no! Until this move of IV to crypto API, we can rely on > >> support in dm-crypt (if it is not supported, it is just a very old kernel). > >> (Actually, this was the first thing I checked in this patchset - if it is > >> unconditionally enabled for compatibility once dmcrypt is selected.) > >> > >> People already use removable devices with BitLocker. > >> It was the whole point that it works out-of-the-box without enabling > >> anything. > >> > >> If you insist on this to be optional, please better keep this IV inside > >> dmcrypt. > >> (EBOIV has no other use than for disk encryption anyway.) > >> > >> Or maybe another option would be to introduce option under dm-crypt > >> Kconfig that > >> defaults to enabled (like support for foreign/legacy disk encryption > >> schemes) and that > >> selects these IVs/modes. > >> But requiring some random switch in crypto API will only confuse users. > > > > CONFIG_DM_CRYPT can either select every weird combination of algorithms > > anyone > > can ever be using, or it can select some defaults and require any other > > needed > > algorithms to be explicitly selected. > > > > In reality, dm-crypt has never even selected any particular block ciphers, > > even > > AES. Nor has it ever selected XTS. So it's actually always made users (or > > kernel distributors) explicitly select algorithms. Why the Bitlocker > > support > > suddenly different? > > > > I'd think a lot of dm-crypt users don't want to bloat their kernels with > > random > > legacy algorithms. > > Yes, but IV is in reality not a cryptographic algorithm, it is kind-of a > configuration > "option" of sector encryption mode here. > > We had all of disk-IV inside dmcrypt before - but once it is partially moved > into crypto API > (ESSIV, EBOIV for now), it becomes much more complicated for user to select > what he needs. > > I think we have no way to check that IV is available from userspace - it > will report the same error as if block cipher is not available, not helping > user much > with the error. > > But then I also think we should add abstract dm-crypt options here (Legacy > TrueCrypt modes, > Bitlocker modes) that will select these crypto API configuration switches. > Otherwise it will be only a complicated matrix of crypto API options... hm... just thinking out loud, but maybe the right say to go is to not have a build dependency, but add some user assistance code in cryptosetup that parses /proc/crypto after failures to try and suggest the user with a way forward? e.g. if eboiv mapping initiation fails, scan /proc/crypto and either warn of a lack of AES or, assuming some instance of AES is found, warn of lack of EBOIV. It's a little messy and heuristic code for sure, but it lives in a user space utility. Does that sound sane? -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH 1/4] crypto: add eboiv as a crypto API template
On Mon, Oct 26, 2020 at 8:26 PM Eric Biggers wrote:
>
> Here's the version of eboiv_create() I recommend (untested):
>
> static int eboiv_create(struct crypto_template *tmpl, struct rtattr **tb)
> {
> struct skcipher_instance *inst;
> struct eboiv_instance_ctx *ictx;
> struct skcipher_alg *alg;
> u32 mask;
> int err;
...
Thank you very much for the review and assistance. I will send out a
revised version.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
[PATCH 2/4] crypto: add eboiv(cbc(aes)) test vectors
Add test vectors for the use of the EBOIV template with cbc(aes)
modes as it is being used by dm-crypt for BitLocker support.
Vectors taken from dm-crypt test suite images.
Signed-off-by: Gilad Ben-Yossef
---
crypto/tcrypt.c | 9 ++
crypto/testmgr.c | 6 +
crypto/testmgr.h | 279 +++
3 files changed, 294 insertions(+)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 234b1adcfbcb..583c027fe8f5 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -2344,6 +2344,15 @@ static int do_test(const char *alg, u32 type, u32 mask,
int m, u32 num_mb)
NULL, 0, 16, 8, speed_template_16);
break;
+ case 222:
+ test_acipher_speed("eboiv(cbc(aes))",
+ ENCRYPT, sec, NULL, 0,
+ speed_template_16_32);
+ test_acipher_speed("eboiv(cbc(aes))",
+ DECRYPT, sec, NULL, 0,
+ speed_template_16_32);
+ break;
+
case 300:
if (alg) {
test_hash_speed(alg, sec, generic_hash_speed_template);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 66ee3bbc9872..9c30db79f323 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4771,6 +4771,12 @@ static const struct alg_test_desc alg_test_descs[] = {
.alg = "drbg_pr_sha512",
.fips_allowed = 1,
.test = alg_test_null,
+ }, {
+ .alg = "eboiv(cbc(aes))",
+ .test = alg_test_skcipher,
+ .suite = {
+ .cipher = __VECS(eboiv_aes_cbc_tv_template)
+ }
}, {
.alg = "ecb(aes)",
.test = alg_test_skcipher,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index cc9dc4cfec8c..0dc18148ea04 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -33045,6 +33045,285 @@ static const struct comp_testvec
zstd_decomp_tv_template[] = {
},
};
+/* vectors taken from cryptosetup test suite images */
+static const struct cipher_testvec eboiv_aes_cbc_tv_template[] = {
+ {
+ /* cbc-aes-128 with 512b sector size, 0th sector */
+ .key= "\x6c\x96\xf8\x2a\x94\x2e\x87\x5f"
+ "\x02\x9c\x3d\xd9\xe4\x35\x17\x73",
+ .klen = 16,
+ .iv = "\x00\x50\x1a\x02\x00\x00\x00\x00"
+ "\x00\x00\x00\x00\x00\x00\x00\x00",
+ .ptext = "\xeb\x52\x90\x4e\x54\x46\x53\x20"
+ "\x20\x20\x20\x00\x02\x08\x00\x00"
+ "\x00\x00\x00\x00\x00\xf8\x00\x00"
+ "\x3f\x00\xff\x00\x00\x08\x00\x00"
+ "\x00\x00\x00\x00\x80\x00\x00\x00"
+ "\xff\x1f\x03\x00\x00\x00\x00\x00"
+ "\x55\x21\x00\x00\x00\x00\x00\x00"
+ "\x02\x00\x00\x00\x00\x00\x00\x00"
+ "\xf6\x00\x00\x00\x01\x00\x00\x00"
+ "\x13\x1e\xf1\xd4\x56\xf1\xd4\xf2"
+ "\x00\x00\x00\x00\xfa\x33\xc0\x8e"
+ "\xd0\xbc\x00\x7c\xfb\x68\xc0\x07"
+ "\x1f\x1e\x68\x66\x00\xcb\x88\x16"
+ "\x0e\x00\x66\x81\x3e\x03\x00\x4e"
+ "\x54\x46\x53\x75\x15\xb4\x41\xbb"
+ "\xaa\x55\xcd\x13\x72\x0c\x81\xfb"
+ "\x55\xaa\x75\x06\xf7\xc1\x01\x00"
+ "\x75\x03\xe9\xdd\x00\x1e\x83\xec"
+ "\x18\x68\x1a\x00\xb4\x48\x8a\x16"
+ "\x0e\x00\x8b\xf4\x16\x1f\xcd\x13"
+ "\x9f\x83\xc4\x18\x9e\x58\x1f\x72"
+ "\xe1\x3b\x06\x0b\x00\x75\xdb\xa3"
+ "\x0f\x00\xc1\x2e\x0f\x00\x04\x1e"
+ "\x5a\x33\xdb\xb9\x00\x20\x2b\xc8"
+ "\x66\xff\x06\x11\x00\x03\x16\x0f"
+ "\x00\x8e\xc2\xff\x06\x16\x00\xe8"
+ "\x4b\x00\x2b\xc8\x77\xef\xb8\x00"
+ "\xbb\xcd\x1a\x66\x23\xc0\x75\x2d"
+ "\x66\x81\xfb\x54\x43\x50\x41\x75"
+ "\x24\x81\xf9\x02\x01\x72\x1e\x16"
+ "\x68\x07\xbb\x16\x68\x52\x11\x16"
+ "\x68\x09\x00\x66\x53\x66\x53\x66"
+ "\x55\x16\x16\x16\x68\xb8\x01\x66"
+
[PATCH 4/4] crypto: ccree: re-introduce ccree eboiv support
BitLocker eboiv support, which was removed in
commit 1d8b41ff6991 ("crypto: ccree - remove bitlocker cipher")
is reintroduced based on the crypto API new support for
eboiv.
Signed-off-by: Gilad Ben-Yossef
Fixes: 1d8b41ff6991 ("crypto: ccree - remove bitlocker cipher")
---
drivers/crypto/ccree/cc_cipher.c | 130 +++
drivers/crypto/ccree/cc_crypto_ctx.h | 1 +
2 files changed, 94 insertions(+), 37 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index b5568de86ca4..23407063bd40 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -95,10 +95,14 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p,
u32 size)
case S_DIN_to_AES:
switch (size) {
case CC_AES_128_BIT_KEY_SIZE:
- case CC_AES_192_BIT_KEY_SIZE:
if (ctx_p->cipher_mode != DRV_CIPHER_XTS)
return 0;
break;
+ case CC_AES_192_BIT_KEY_SIZE:
+ if (ctx_p->cipher_mode != DRV_CIPHER_XTS &&
+ ctx_p->cipher_mode != DRV_CIPHER_BITLOCKER)
+ return 0;
+ break;
case CC_AES_256_BIT_KEY_SIZE:
return 0;
case (CC_AES_192_BIT_KEY_SIZE * 2):
@@ -141,6 +145,7 @@ static int validate_data_size(struct cc_cipher_ctx *ctx_p,
case DRV_CIPHER_ECB:
case DRV_CIPHER_CBC:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
if (IS_ALIGNED(size, AES_BLOCK_SIZE))
return 0;
break;
@@ -366,7 +371,8 @@ static int cc_cipher_sethkey(struct crypto_skcipher *sktfm,
const u8 *key,
}
if (ctx_p->cipher_mode == DRV_CIPHER_XTS ||
- ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
+ ctx_p->cipher_mode == DRV_CIPHER_ESSIV ||
+ ctx_p->cipher_mode == DRV_CIPHER_BITLOCKER) {
if (hki.hw_key1 == hki.hw_key2) {
dev_err(dev, "Illegal hw key numbers (%d,%d)\n",
hki.hw_key1, hki.hw_key2);
@@ -564,6 +570,7 @@ static void cc_setup_readiv_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
/* IV */
hw_desc_init([*seq_size]);
set_setup_mode([*seq_size], SETUP_WRITE_STATE1);
@@ -618,6 +625,7 @@ static void cc_setup_state_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
break;
default:
dev_err(dev, "Unsupported cipher mode (%d)\n", cipher_mode);
@@ -637,56 +645,68 @@ static void cc_setup_xex_state_desc(struct crypto_tfm
*tfm,
int flow_mode = ctx_p->flow_mode;
int direction = req_ctx->gen_ctx.op_type;
dma_addr_t key_dma_addr = ctx_p->user.key_dma_addr;
- unsigned int key_len = (ctx_p->keylen / 2);
dma_addr_t iv_dma_addr = req_ctx->gen_ctx.iv_dma_addr;
- unsigned int key_offset = key_len;
+ unsigned int key_len;
+ unsigned int key_offset;
switch (cipher_mode) {
case DRV_CIPHER_ECB:
- break;
case DRV_CIPHER_CBC:
case DRV_CIPHER_CBC_CTS:
case DRV_CIPHER_CTR:
case DRV_CIPHER_OFB:
- break;
- case DRV_CIPHER_XTS:
- case DRV_CIPHER_ESSIV:
+ /* No secondary key for these ciphers, so just return */
+ return;
- if (cipher_mode == DRV_CIPHER_ESSIV)
- key_len = SHA256_DIGEST_SIZE;
+ case DRV_CIPHER_XTS:
+ /* Secondary key is same size as primary key and stored after
primary key */
+ key_len = ctx_p->keylen / 2;
+ key_offset = key_len;
+ break;
- /* load XEX key */
- hw_desc_init([*seq_size]);
- set_cipher_mode([*seq_size], cipher_mode);
- set_cipher_config0([*seq_size], direction);
- if (cc_key_type(tfm) == CC_HW_PROTECTED_KEY) {
- set_hw_crypto_key([*seq_size],
- ctx_p->hw.key2_slot);
- } else {
- set_din_type([*seq_size], DMA_DLLI,
-(key_dma_addr + key_offset),
-key_len, NS_BIT);
- }
- set_xex_data_unit_size([*seq_size], nbytes);
- set_flow_mode([*seq_size], S_DIN_to_AES
[PATCH 1/4] crypto: add eboiv as a crypto API template
Encrypted byte-offset initialization vector (EBOIV) is an IV
generation method that is used in some cases by dm-crypt for
supporting the BitLocker volume encryption used by Windows 8
and onwards as a backwards compatible version in lieu of XTS
support. Support for eboiv was added to dm-crypt in 5.6.
This patch re-implements eboiv as a generic crypto API
template, thus allowing use of a alternative architecture
specific optimzied implementations (as well as saving a
memory allocation along the way).
Signed-off-by: Gilad Ben-Yossef
---
crypto/Kconfig | 21
crypto/Makefile | 1 +
crypto/eboiv.c | 295
3 files changed, 317 insertions(+)
create mode 100644 crypto/eboiv.c
diff --git a/crypto/Kconfig b/crypto/Kconfig
index e85d8a059489..a29aac2b10d2 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -521,6 +521,27 @@ config CRYPTO_ESSIV
combined with ESSIV the only feasible mode for h/w accelerated
block encryption)
+config CRYPTO_EBOIV
+ tristate "EBOIV support for block encryption"
+ help
+ Encrypted byte-offset initialization vector (EBOIV) is an IV
+ generation method that is used in some cases by dm-crypt for
+ supporting the BitLocker volume encryption used by Windows 8
+ and onwards as a backwards compatible version in lieu of XTS
+ support.
+
+ It uses the block encryption key as the symmetric key for a
+ block encryption pass applied to the sector offset of the block.
+ Additional details can be found at
+ https://www.jedec.org/sites/default/files/docs/JESD223C.pdf
+
+ Note that the use of EBOIV is not recommended for new deployments,
+ and so this only needs to be enabled when interoperability with
+ existing encrypted volumes of filesystems is required, or when
+ building for a particular system that requires it (e.g., when
+ interop with BitLocker encrypted volumes of Windows systems
+ prior to Windows 10 is required)
+
comment "Hash modes"
config CRYPTO_CMAC
diff --git a/crypto/Makefile b/crypto/Makefile
index 4ca12b6044f7..bf47ee2ad5cf 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -166,6 +166,7 @@ obj-$(CONFIG_CRYPTO_ZSTD) += zstd.o
obj-$(CONFIG_CRYPTO_OFB) += ofb.o
obj-$(CONFIG_CRYPTO_ECC) += ecc.o
obj-$(CONFIG_CRYPTO_ESSIV) += essiv.o
+obj-$(CONFIG_CRYPTO_EBOIV) += eboiv.o
obj-$(CONFIG_CRYPTO_CURVE25519) += curve25519-generic.o
ecdh_generic-y += ecdh.o
diff --git a/crypto/eboiv.c b/crypto/eboiv.c
new file mode 100644
index ..467833e89139
--- /dev/null
+++ b/crypto/eboiv.c
@@ -0,0 +1,295 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * EBOIV skcipher template for block encryption
+ *
+ * This template encapsulates the Encrypted byte-offset IV generation
algorithm
+ * used in Bitlocker in CBC mode by dm-crypt, which converts the initial vector
+ * for the skcipher used for block encryption, by encrypting it using the same
+ * skcipher key as encryption key. Usually, the input IV is a 64-bit sector
+ * offset (the byte offset of the start of the sector) in LE representation
+ * zero-padded to the size of the IV, but this * is not assumed by this driver.
+ *
+ * The typical use of this template is to instantiate the skcipher
+ * 'eboiv(cbc(aes))', which is the only instantiation used by
+ * dm-crypt for supporting BitLocker AES-CBC mode as specified in
+ * https://www.jedec.org/sites/default/files/docs/JESD223C.pdf
+ *
+ * Copyright (C) 2020 ARM Limited or its affiliates.
+ * Written by Gilad Ben-Yossef
+ *
+ * Heavily based on:
+ *
+ * ESSIV skcipher and aead template for block encryption
+ * Copyright (c) 2019 Linaro, Ltd.
+ *
+ * and
+ *
+ * dm-crypt eboiv code
+ * by Milan Broz and Ard Biesheuvel
+ *
+ */
+
+#include
+#include
+#include
+
+#include "internal.h"
+
+struct eboiv_instance_ctx {
+ struct crypto_skcipher_spawn skcipher_spawn;
+ char eboiv_cipher_name[CRYPTO_MAX_ALG_NAME];
+};
+
+struct eboiv_tfm_ctx {
+ struct crypto_skcipher *skcipher;
+};
+
+struct eboiv_req_ctx {
+ struct skcipher_request *req;
+ bool enc;
+ struct scatterlist src;
+ struct scatterlist dst;
+ u8 iv[MAX_CIPHER_BLOCKSIZE] __aligned(__alignof__(__le64));
+ struct skcipher_request subreq;
+};
+
+static int eboiv_skcipher_setkey(struct crypto_skcipher *tfm,
+const u8 *key, unsigned int keylen)
+{
+ struct eboiv_tfm_ctx *tctx = crypto_skcipher_ctx(tfm);
+
+ crypto_skcipher_clear_flags(tctx->skcipher, CRYPTO_TFM_REQ_MASK);
+ crypto_skcipher_set_flags(tctx->skcipher,
+ crypto_skcipher_get_flags(tfm) &
+ CRYPTO_TFM_REQ_MASK);
+ return crypto_skcipher_setkey(tctx->skcipher, key, keylen);
+}
+
+static void eboiv_skcipher_done(struct crypto_asyn
[PATCH 0/4] crypto: switch to crypto API for EBOIV generation
This series creates an EBOIV template that produces a skcipher transform which passes through all operations to the skcipher, while using the same skcipher and key to encrypt the input IV, which is assumed to be a sector offset, although this is not enforced. This matches dm-crypt use of EBOIV to provide BitLocker support, and so it is proposed as a replacement in patch #3. Replacing the dm-crypt specific EBOIV implementation to a Crypto API based one allows us to save a memory allocation per each request, as well as opening the way for use of compatible alternative transform providers, one of which, based on the Arm TrustZone CryptoCell hardware, is proposed as patch #4. Future potential work to allow encapsulating the handling of multiple subsequent blocks by the Crypto API may also benefit from this work. The code has been tested on both x86_64 virtual machine with the dm-crypt test suite and on an arm 32 bit board with the CryptoCell hardware. Since no offical source for eboiv test vectors is known, the test vectors supplied as patch #2 are derived from sectors which are part of the dm-crypt test suite. Gilad Ben-Yossef (4): crypto: add eboiv as a crypto API template crypto: add eboiv(cbc(aes)) test vectors dm crypt: switch to EBOIV crypto API template crypto: ccree: re-introduce ccree eboiv support crypto/Kconfig | 21 ++ crypto/Makefile | 1 + crypto/eboiv.c | 295 +++ crypto/tcrypt.c | 9 + crypto/testmgr.c | 6 + crypto/testmgr.h | 279 + drivers/crypto/ccree/cc_cipher.c | 130 drivers/crypto/ccree/cc_crypto_ctx.h | 1 + drivers/md/Kconfig | 1 + drivers/md/dm-crypt.c| 61 ++ 10 files changed, 725 insertions(+), 79 deletions(-) create mode 100644 crypto/eboiv.c -- 2.28.0
[PATCH 3/4] dm crypt: switch to EBOIV crypto API template
Replace the explicit EBOIV handling in the dm-crypt driver with calls
into the crypto API, which now possesses the capability to perform
this processing within the crypto subsystem.
Signed-off-by: Gilad Ben-Yossef
---
drivers/md/Kconfig| 1 +
drivers/md/dm-crypt.c | 61 ++-
2 files changed, 20 insertions(+), 42 deletions(-)
diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 30ba3573626c..ca6e56a72281 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -273,6 +273,7 @@ config DM_CRYPT
select CRYPTO
select CRYPTO_CBC
select CRYPTO_ESSIV
+ select CRYPTO_EBOIV
help
This device-mapper target allows you to create a device that
transparently encrypts the data on it. You'll need to activate
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 148960721254..cad8f4e3f5d9 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -716,47 +716,18 @@ static int crypt_iv_random_gen(struct crypt_config *cc,
u8 *iv,
return 0;
}
-static int crypt_iv_eboiv_ctr(struct crypt_config *cc, struct dm_target *ti,
- const char *opts)
-{
- if (crypt_integrity_aead(cc)) {
- ti->error = "AEAD transforms not supported for EBOIV";
- return -EINVAL;
- }
-
- if (crypto_skcipher_blocksize(any_tfm(cc)) != cc->iv_size) {
- ti->error = "Block size of EBOIV cipher does "
- "not match IV size of block cipher";
- return -EINVAL;
- }
-
- return 0;
-}
-
static int crypt_iv_eboiv_gen(struct crypt_config *cc, u8 *iv,
struct dm_crypt_request *dmreq)
{
- u8 buf[MAX_CIPHER_BLOCKSIZE] __aligned(__alignof__(__le64));
- struct skcipher_request *req;
- struct scatterlist src, dst;
- struct crypto_wait wait;
- int err;
-
- req = skcipher_request_alloc(any_tfm(cc), GFP_NOIO);
- if (!req)
- return -ENOMEM;
-
- memset(buf, 0, cc->iv_size);
- *(__le64 *)buf = cpu_to_le64(dmreq->iv_sector * cc->sector_size);
- sg_init_one(, page_address(ZERO_PAGE(0)), cc->iv_size);
- sg_init_one(, iv, cc->iv_size);
- skcipher_request_set_crypt(req, , , cc->iv_size, buf);
- skcipher_request_set_callback(req, 0, crypto_req_done, );
- err = crypto_wait_req(crypto_skcipher_encrypt(req), );
- skcipher_request_free(req);
+ /*
+* ESSIV encryption of the IV is handled by the crypto API,
+* so compute and pass the sector offset here.
+*/
+ memset(iv, 0, cc->iv_size);
+ *(__le64 *)iv = cpu_to_le64(dmreq->iv_sector * cc->sector_size);
- return err;
+ return 0;
}
static void crypt_iv_elephant_dtr(struct crypt_config *cc)
@@ -777,13 +748,9 @@ static int crypt_iv_elephant_ctr(struct crypt_config *cc,
struct dm_target *ti,
if (IS_ERR(elephant->tfm)) {
r = PTR_ERR(elephant->tfm);
elephant->tfm = NULL;
- return r;
}
- r = crypt_iv_eboiv_ctr(cc, ti, NULL);
- if (r)
- crypt_iv_elephant_dtr(cc);
- return r;
+ return 0;
}
static void diffuser_disk_to_cpu(u32 *d, size_t n)
@@ -1092,7 +1059,6 @@ static struct crypt_iv_operations crypt_iv_random_ops = {
};
static struct crypt_iv_operations crypt_iv_eboiv_ops = {
- .ctr = crypt_iv_eboiv_ctr,
.generator = crypt_iv_eboiv_gen
};
@@ -2739,6 +2705,15 @@ static int crypt_ctr_cipher_new(struct dm_target *ti,
char *cipher_in, char *key
cipher_api = buf;
}
+ if (*ivmode && (!strcmp(*ivmode, "eboiv") || !strcmp(*ivmode,
"elephant"))) {
+ ret = snprintf(buf, CRYPTO_MAX_ALG_NAME, "eboiv(%s)",
cipher_api);
+ if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME) {
+ ti->error = "Cannot allocate cipher string";
+ return -ENOMEM;
+ }
+ cipher_api = buf;
+ }
+
cc->key_parts = cc->tfms_count;
/* Allocate cipher */
@@ -2817,6 +2792,8 @@ static int crypt_ctr_cipher_old(struct dm_target *ti,
char *cipher_in, char *key
}
ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"essiv(%s(%s),%s)", chainmode, cipher, *ivopts);
+ } else if (*ivmode && (!strcmp(*ivmode, "eboiv") || !strcmp(*ivmode,
"elephant"))) {
+ ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"eboiv(%s(%s))", chainmode, cipher);
} else {
ret = snprintf(cipher_api, CRYPTO_MAX_ALG_NAME,
"%s(%s)", chainmode, cipher);
--
2.28.0
Re: [PATCH 1/2] dt-bindings: crypto: update ccree optional params
Hןת On Wed, Sep 23, 2020 at 4:57 AM Rob Herring wrote: > > On Wed, Sep 16, 2020 at 10:19:49AM +0300, Gilad Ben-Yossef wrote: > > Document ccree driver supporting new optional parameters allowing to > > customize the DMA transactions cache parameters and ACE bus sharability > > properties. > > > > Signed-off-by: Gilad Ben-Yossef > > --- > > Documentation/devicetree/bindings/crypto/arm-cryptocell.txt | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt > > b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt > > index 6130e6eb4af8..1a1603e457a8 100644 > > --- a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt > > +++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt > > @@ -13,6 +13,10 @@ Required properties: > > Optional properties: > > - clocks: Reference to the crypto engine clock. > > - dma-coherent: Present if dma operations are coherent. > > +- awcache: Set write transactions cache attributes > > +- arcache: Set read transactions cache attributes > > dma-coherent already implies these are 011x, 101x or 111x. In my limited > experience configuring these (Calxeda SATA and ethernet), writeback, > write-allocate was pretty much always optimal. Indeed and these are the default. But not all SoC are born equal and we got a request to allow setting these. Maybe instead of numerical values have three possible verbal setting would be better? > > +- awdomain: Set write transactions ACE sharability domain (712, 703, 713 > > only) > > +- ardomain: Set read transactions ACE sharability domain (712, 703, 713 > > only) > > This probably needs something common. We may need something for Mali, > too. I don't think different settings for read and write makes much > sense nor does anything beyond IS or OS. I agree. Maybe sharability_domain: either "IS" or "OS"? > > These could also just be implied by the compatible string (and requiring > an SoC specific one). hm... we could do it but this will require us to know (and publicly acknowledge) of every SoC making use of this piece of hardware design. There is currently no other part of the driver that needs this. Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH 2/2] crypto: ccree - add custom cache params from DT file
Hi,
On Fri, Sep 18, 2020 at 10:39 PM Nick Desaulniers
wrote:
>
> On Thu, Sep 17, 2020 at 12:20 AM Gilad Ben-Yossef wrote:
> >
...
> >
> > I am unable to understand this warning. It looks like it is
> > complaining about a FIELD_GET sanity check that is always false, which
> > makes sense since we're using a constant.
> >
> > Anyone can enlighten me if I've missed something?
>
> Looked at some of this code recently. I think it may have an issue
> for masks where sizeof(mask) < sizeof(unsigned long long).
>
> In your code, via 0day bot:
>
>107 u32 cache_params, ace_const, val, mask;
> ...
> > 120 cache_params |= FIELD_PREP(mask, val);
>
> then in include/linux/bitfield.h, we have:
>
> 92 #define FIELD_PREP(_mask, _val) \
> 93 ({\
> 94 __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
>
> 44 #define __BF_FIELD_CHECK(_mask, _reg, _val, _pfx) \
> ...
> 52 BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull, \
> 53 _pfx "type of reg too small for mask"); \
>
> so the 0ULL in FIELD_PREP is important. In __BF_FIELD_CHECK, the
> typeof(_reg) is unsigned long long (because 0ULL was passed). So we
> have a comparison between a u32 and a u64; indeed any u32 can never be
> greater than a u64 that we know has the value of ULLONG_MAX.
>
> I did send a series splitting these up, I wonder if they'd help here:
> https://lore.kernel.org/lkml/20200708230402.1644819-3-ndesaulni...@google.com/
> --
Thanks!
This indeed explains this. It seems there is nothing for me to do
about it in the driver code though, as it seems the issue is in the
macro and you have already posted a fix for it.
Thanks again,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
Re: [PATCH 2/2] crypto: ccree - add custom cache params from DT file
hmm... On Wed, Sep 16, 2020 at 4:48 PM kernel test robot wrote: > > url: > https://github.com/0day-ci/linux/commits/Gilad-Ben-Yossef/add-optional-cache-params-from-DT/20200916-152151 > base: > https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git > master > config: arm64-randconfig-r015-20200916 (attached as .config) > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project > 9e3842d60351f986d77dfe0a94f76e4fd895f188) > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # install arm64 cross compiling tool for clang build > # apt-get install binutils-aarch64-linux-gnu > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All warnings (new ones prefixed by >>): > > >> drivers/crypto/ccree/cc_driver.c:120:18: warning: result of comparison of > >> constant 18446744073709551615 with expression of type 'u32' (aka 'unsigned > >> int') is always false [-Wtautological-constant-out-of-range-compare] >cache_params |= FIELD_PREP(mask, val); >^ >include/linux/bitfield.h:94:3: note: expanded from macro 'FIELD_PREP' >__BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");\ >^~~ >include/linux/bitfield.h:52:28: note: expanded from macro > '__BF_FIELD_CHECK' >BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull, \ >~^~~~ >include/linux/build_bug.h:39:58: note: expanded from macro > 'BUILD_BUG_ON_MSG' >#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) >~^~~ >include/linux/compiler_types.h:319:22: note: expanded from macro > 'compiletime_assert' >_compiletime_assert(condition, msg, __compiletime_assert_, > __COUNTER__) > > ^~~ >include/linux/compiler_types.h:307:23: note: expanded from macro > '_compiletime_assert' >__compiletime_assert(condition, msg, prefix, suffix) >~^~~ >include/linux/compiler_types.h:299:9: note: expanded from macro > '__compiletime_assert' >if (!(condition)) \ > ^ I am unable to understand this warning. It looks like it is complaining about a FIELD_GET sanity check that is always false, which makes sense since we're using a constant. Anyone can enlighten me if I've missed something? Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH v6 7/8] X.509: support OSCCA sm2-with-sm3 certificate verification
On Mon, Sep 14, 2020 at 9:34 AM Tianjia Zhang
wrote:
>
> Hi Gilad,
>
> On 9/13/20 3:12 PM, Gilad Ben-Yossef wrote:
> > Hi,
> >
> >
> > On Thu, Sep 3, 2020 at 4:13 PM Tianjia Zhang
> > wrote:
> >>
> >> The digital certificate format based on SM2 crypto algorithm as
> >> specified in GM/T 0015-2012. It was published by State Encryption
> >> Management Bureau, China.
> >>
> >> The method of generating Other User Information is defined as
> >> ZA=H256(ENTLA || IDA || a || b || xG || yG || xA || yA), it also
> >> specified in https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02.
> >>
> >> The x509 certificate supports sm2-with-sm3 type certificate
> >> verification. Because certificate verification requires ZA
> >> in addition to tbs data, ZA also depends on elliptic curve
> >> parameters and public key data, so you need to access tbs in sig
> >> and calculate ZA. Finally calculate the digest of the
> >> signature and complete the verification work. The calculation
> >> process of ZA is declared in specifications GM/T 0009-2012
> >> and GM/T 0003.2-2012.
> >>
> >> Signed-off-by: Tianjia Zhang
> >> Tested-by: Xufeng Zhang
> >> ---
> >> crypto/asymmetric_keys/Makefile | 1 +
> >> crypto/asymmetric_keys/public_key.c | 6 +++
> >> crypto/asymmetric_keys/public_key_sm2.c | 61
> >> crypto/asymmetric_keys/x509_public_key.c | 3 ++
> >> include/crypto/public_key.h | 15 ++
> >> 5 files changed, 86 insertions(+)
> >> create mode 100644 crypto/asymmetric_keys/public_key_sm2.c
> >>
> >> diff --git a/crypto/asymmetric_keys/Makefile
> >> b/crypto/asymmetric_keys/Makefile
> >> index 28b91adba2ae..1a99ea5acb6b 100644
> >> --- a/crypto/asymmetric_keys/Makefile
> >> +++ b/crypto/asymmetric_keys/Makefile
> >> @@ -11,6 +11,7 @@ asymmetric_keys-y := \
> >> signature.o
> >>
> >> obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
> >> +obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key_sm2.o
> >> obj-$(CONFIG_ASYMMETRIC_TPM_KEY_SUBTYPE) += asym_tpm.o
> >>
> >> #
> >> diff --git a/crypto/asymmetric_keys/public_key.c
> >> b/crypto/asymmetric_keys/public_key.c
> >> index d8410ffd7f12..1d0492098bbd 100644
> >> --- a/crypto/asymmetric_keys/public_key.c
> >> +++ b/crypto/asymmetric_keys/public_key.c
> >> @@ -299,6 +299,12 @@ int public_key_verify_signature(const struct
> >> public_key *pkey,
> >> if (ret)
> >> goto error_free_key;
> >>
> >> + if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) {
> >> + ret = cert_sig_digest_update(sig, tfm);
> >> + if (ret)
> >> + goto error_free_key;
> >> + }
> >> +
> >> sg_init_table(src_sg, 2);
> >> sg_set_buf(_sg[0], sig->s, sig->s_size);
> >> sg_set_buf(_sg[1], sig->digest, sig->digest_size);
> >> diff --git a/crypto/asymmetric_keys/public_key_sm2.c
> >> b/crypto/asymmetric_keys/public_key_sm2.c
> >> new file mode 100644
> >> index ..7325cf21dbb4
> >> --- /dev/null
> >> +++ b/crypto/asymmetric_keys/public_key_sm2.c
> >> @@ -0,0 +1,61 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * asymmetric public-key algorithm for SM2-with-SM3 certificate
> >> + * as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012 SM2 and
> >> + * described at https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02
> >> + *
> >> + * Copyright (c) 2020, Alibaba Group.
> >> + * Authors: Tianjia Zhang
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#if IS_REACHABLE(CONFIG_CRYPTO_SM2)
> >> +
> >> +int cert_sig_digest_update(const struct public_key_signature *sig,
> >> + struct crypto_akcipher *tfm_pkey)
> >> +{
> >> + struct crypto_shash *tfm;
> >> + struct shash_desc *desc;
> >> + size_t desc_size;
> >> + unsigned char dgst[SM3_DIGEST_SIZE];
> >> + int ret;
> >> +
> >> + BUG_ON(!sig->data);
> >> +
> >> + ret = sm2_compute_z_digest(tfm_pkey,
[PATCH 0/2] add optional cache params from DT
Rework the setting of AXI bus cache parameters, including optionally allowing setting them from device tree Gilad Ben-Yossef (2): dt-bindings: crypto: update ccree optional params crypto: ccree - add custom cache params from DT file .../bindings/crypto/arm-cryptocell.txt| 4 + drivers/crypto/ccree/cc_driver.c | 89 +++ drivers/crypto/ccree/cc_driver.h | 4 +- drivers/crypto/ccree/cc_pm.c | 2 +- 4 files changed, 81 insertions(+), 18 deletions(-) -- 2.27.0
[PATCH 2/2] crypto: ccree - add custom cache params from DT file
Add optinal ability to customize DMA transactions cache parameters and
ACE bus sharability properties and set new defaults.
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_driver.c | 89 ++--
drivers/crypto/ccree/cc_driver.h | 4 +-
drivers/crypto/ccree/cc_pm.c | 2 +-
3 files changed, 77 insertions(+), 18 deletions(-)
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 6f519d3e896c..db497bc065d4 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -100,6 +100,71 @@ static const struct of_device_id arm_ccree_dev_of_match[]
= {
};
MODULE_DEVICE_TABLE(of, arm_ccree_dev_of_match);
+static void init_cc_dt_params(struct cc_drvdata *drvdata)
+{
+ struct device *dev = drvdata_to_dev(drvdata);
+ struct device_node *np = dev->of_node;
+ u32 cache_params, ace_const, val, mask;
+ int rc;
+
+ /* register CC_AXIM_CACHE_PARAMS */
+ cache_params = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
+ dev_dbg(dev, "Cache params previous: 0x%08X\n", cache_params);
+
+ rc = of_property_read_u32(np, "awcache", );
+ if (rc)
+ val = (drvdata->coherent ? 0xb : 0x2);
+
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_AWCACHE);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ /* write AWCACHE also to AWCACHE_LAST */
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_AWCACHE_LAST);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ rc = of_property_read_u32(np, "arcache", );
+ if (rc)
+ val = (drvdata->coherent ? 0xb : 0x2);
+
+ mask = CC_GENMASK(CC_AXIM_CACHE_PARAMS_ARCACHE);
+ cache_params &= ~mask;
+ cache_params |= FIELD_PREP(mask, val);
+
+ drvdata->cache_params = cache_params;
+
+ dev_dbg(dev, "Cache params current: 0x%08X\n", cache_params);
+
+ if (drvdata->hw_rev <= CC_HW_REV_710)
+ return;
+
+ /* register CC_AXIM_ACE_CONST */
+ ace_const = cc_ioread(drvdata, CC_REG(AXIM_ACE_CONST));
+ dev_dbg(dev, "ACE-const previous: 0x%08X\n", ace_const);
+
+ rc = of_property_read_u32(np, "ardomain", );
+ ace_const = cc_ioread(drvdata, CC_REG(AXIM_ACE_CONST));
+ if (rc)
+ val = (drvdata->coherent ? 0x2 : 0x3);
+
+ mask = CC_GENMASK(CC_AXIM_ACE_CONST_ARDOMAIN);
+ ace_const &= ~mask;
+ ace_const |= FIELD_PREP(mask, val);
+
+ rc = of_property_read_u32(np, "awdomain", );
+ if (rc)
+ val = (drvdata->coherent ? 0x2 : 0x3);
+
+ mask = CC_GENMASK(CC_AXIM_ACE_CONST_AWDOMAIN);
+ ace_const &= ~mask;
+ ace_const |= FIELD_PREP(mask, val);
+
+ dev_dbg(dev, "ACE-const current: 0x%08X\n", ace_const);
+
+ drvdata->ace_const = ace_const;
+}
+
static u32 cc_read_idr(struct cc_drvdata *drvdata, const u32 *idr_offsets)
{
int i;
@@ -218,9 +283,9 @@ bool cc_wait_for_reset_completion(struct cc_drvdata
*drvdata)
return false;
}
-int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe)
+int init_cc_regs(struct cc_drvdata *drvdata)
{
- unsigned int val, cache_params;
+ unsigned int val;
struct device *dev = drvdata_to_dev(drvdata);
/* Unmask all AXI interrupt sources AXI_CFG1 register */
@@ -245,19 +310,9 @@ int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe)
cc_iowrite(drvdata, CC_REG(HOST_IMR), ~val);
- cache_params = (drvdata->coherent ? CC_COHERENT_CACHE_PARAMS : 0x0);
-
- val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
-
- if (is_probe)
- dev_dbg(dev, "Cache params previous: 0x%08X\n", val);
-
- cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), cache_params);
- val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS));
-
- if (is_probe)
- dev_dbg(dev, "Cache params current: 0x%08X (expect: 0x%08X)\n",
- val, cache_params);
+ cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), drvdata->cache_params);
+ if (drvdata->hw_rev >= CC_HW_REV_712)
+ cc_iowrite(drvdata, CC_REG(AXIM_ACE_CONST), drvdata->ace_const);
return 0;
}
@@ -445,7 +500,9 @@ static int init_cc_resources(struct platform_device
*plat_dev)
}
dev_dbg(dev, "Registered to IRQ: %d\n", irq);
- rc = init_cc_regs(new_drvdata, true);
+ init_cc_dt_params(new_drvdata);
+
+ rc = init_cc_regs(new_drvdata);
if (rc) {
dev_err(dev, "init_cc_regs failed\n");
goto post_pm_err;
diff --git a/drivers/crypto/ccree/cc_driver.h b/drivers/crypto/ccree/cc_driver.h
index af77b2020350..cd5a51e8a281 100644
--- a/drivers/crypto/ccree/cc_driv
[PATCH 1/2] dt-bindings: crypto: update ccree optional params
Document ccree driver supporting new optional parameters allowing to customize the DMA transactions cache parameters and ACE bus sharability properties. Signed-off-by: Gilad Ben-Yossef --- Documentation/devicetree/bindings/crypto/arm-cryptocell.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt index 6130e6eb4af8..1a1603e457a8 100644 --- a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt +++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt @@ -13,6 +13,10 @@ Required properties: Optional properties: - clocks: Reference to the crypto engine clock. - dma-coherent: Present if dma operations are coherent. +- awcache: Set write transactions cache attributes +- arcache: Set read transactions cache attributes +- awdomain: Set write transactions ACE sharability domain (712, 703, 713 only) +- ardomain: Set read transactions ACE sharability domain (712, 703, 713 only) Examples: -- 2.27.0
Re: [PATCH v6 7/8] X.509: support OSCCA sm2-with-sm3 certificate verification
Hi,
On Thu, Sep 3, 2020 at 4:13 PM Tianjia Zhang
wrote:
>
> The digital certificate format based on SM2 crypto algorithm as
> specified in GM/T 0015-2012. It was published by State Encryption
> Management Bureau, China.
>
> The method of generating Other User Information is defined as
> ZA=H256(ENTLA || IDA || a || b || xG || yG || xA || yA), it also
> specified in https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02.
>
> The x509 certificate supports sm2-with-sm3 type certificate
> verification. Because certificate verification requires ZA
> in addition to tbs data, ZA also depends on elliptic curve
> parameters and public key data, so you need to access tbs in sig
> and calculate ZA. Finally calculate the digest of the
> signature and complete the verification work. The calculation
> process of ZA is declared in specifications GM/T 0009-2012
> and GM/T 0003.2-2012.
>
> Signed-off-by: Tianjia Zhang
> Tested-by: Xufeng Zhang
> ---
> crypto/asymmetric_keys/Makefile | 1 +
> crypto/asymmetric_keys/public_key.c | 6 +++
> crypto/asymmetric_keys/public_key_sm2.c | 61
> crypto/asymmetric_keys/x509_public_key.c | 3 ++
> include/crypto/public_key.h | 15 ++
> 5 files changed, 86 insertions(+)
> create mode 100644 crypto/asymmetric_keys/public_key_sm2.c
>
> diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
> index 28b91adba2ae..1a99ea5acb6b 100644
> --- a/crypto/asymmetric_keys/Makefile
> +++ b/crypto/asymmetric_keys/Makefile
> @@ -11,6 +11,7 @@ asymmetric_keys-y := \
> signature.o
>
> obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
> +obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key_sm2.o
> obj-$(CONFIG_ASYMMETRIC_TPM_KEY_SUBTYPE) += asym_tpm.o
>
> #
> diff --git a/crypto/asymmetric_keys/public_key.c
> b/crypto/asymmetric_keys/public_key.c
> index d8410ffd7f12..1d0492098bbd 100644
> --- a/crypto/asymmetric_keys/public_key.c
> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -299,6 +299,12 @@ int public_key_verify_signature(const struct public_key
> *pkey,
> if (ret)
> goto error_free_key;
>
> + if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) {
> + ret = cert_sig_digest_update(sig, tfm);
> + if (ret)
> + goto error_free_key;
> + }
> +
> sg_init_table(src_sg, 2);
> sg_set_buf(_sg[0], sig->s, sig->s_size);
> sg_set_buf(_sg[1], sig->digest, sig->digest_size);
> diff --git a/crypto/asymmetric_keys/public_key_sm2.c
> b/crypto/asymmetric_keys/public_key_sm2.c
> new file mode 100644
> index ..7325cf21dbb4
> --- /dev/null
> +++ b/crypto/asymmetric_keys/public_key_sm2.c
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * asymmetric public-key algorithm for SM2-with-SM3 certificate
> + * as specified by OSCCA GM/T 0003.1-2012 -- 0003.5-2012 SM2 and
> + * described at https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02
> + *
> + * Copyright (c) 2020, Alibaba Group.
> + * Authors: Tianjia Zhang
> + */
> +
> +#include
> +#include
> +#include
> +
> +#if IS_REACHABLE(CONFIG_CRYPTO_SM2)
> +
> +int cert_sig_digest_update(const struct public_key_signature *sig,
> + struct crypto_akcipher *tfm_pkey)
> +{
> + struct crypto_shash *tfm;
> + struct shash_desc *desc;
> + size_t desc_size;
> + unsigned char dgst[SM3_DIGEST_SIZE];
> + int ret;
> +
> + BUG_ON(!sig->data);
> +
> + ret = sm2_compute_z_digest(tfm_pkey, SM2_DEFAULT_USERID,
> + SM2_DEFAULT_USERID_LEN, dgst);
> + if (ret)
> + return ret;
> +
> + tfm = crypto_alloc_shash(sig->hash_algo, 0, 0);
> + if (IS_ERR(tfm))
> + return PTR_ERR(tfm);
> +
> + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> + desc = kzalloc(desc_size, GFP_KERNEL);
> + if (!desc)
> + goto error_free_tfm;
> +
> + desc->tfm = tfm;
> +
> + ret = crypto_shash_init(desc);
> + if (ret < 0)
> + goto error_free_desc;
> +
> + ret = crypto_shash_update(desc, dgst, SM3_DIGEST_SIZE);
> + if (ret < 0)
> + goto error_free_desc;
> +
> + ret = crypto_shash_finup(desc, sig->data, sig->data_size,
> sig->digest);
It looks like you are doing a separate init, update, finup every time
- I would consider using crypto_shash_digest() in one go.
In fact, considering the fact that you are allocating a tfm just for
this use and then releasing it, I would consider switching to
crypto_shash_tfm_digest() and dropping the kzalloc all together.
This should simplify the code a bit.
Other than that I don't have anything smart to say :-)
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
Re: [PATCH 16/22] crypto: ccree - add check for xts input length equal to zero
On Fri, Aug 7, 2020 at 7:22 PM Andrei Botila wrote:
>
> From: Andrei Botila
>
> Standardize the way input lengths equal to 0 are handled in all skcipher
> algorithms. All the algorithms return 0 for input lengths equal to zero.
> This change has implications not only for xts(aes) but also for cts(cbc(aes))
> and cts(cbc(paes)).
>
> Cc: Gilad Ben-Yossef
> Signed-off-by: Andrei Botila
> ---
> drivers/crypto/ccree/cc_cipher.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/crypto/ccree/cc_cipher.c
> b/drivers/crypto/ccree/cc_cipher.c
> index 076669dc1035..112bb8b4dce6 100644
> --- a/drivers/crypto/ccree/cc_cipher.c
> +++ b/drivers/crypto/ccree/cc_cipher.c
> @@ -912,17 +912,18 @@ static int cc_cipher_process(struct skcipher_request
> *req,
>
> /* STAT_PHASE_0: Init and sanity checks */
>
> - if (validate_data_size(ctx_p, nbytes)) {
> - dev_dbg(dev, "Unsupported data size %d.\n", nbytes);
> - rc = -EINVAL;
> - goto exit_process;
> - }
> if (nbytes == 0) {
> /* No data to process is valid */
> rc = 0;
> goto exit_process;
> }
>
> + if (validate_data_size(ctx_p, nbytes)) {
> + dev_dbg(dev, "Unsupported data size %d.\n", nbytes);
> + rc = -EINVAL;
> + goto exit_process;
> + }
> +
> if (ctx_p->fallback_on) {
> struct skcipher_request *subreq = skcipher_request_ctx(req);
>
> --
> 2.17.1
>
Acked-by: Gilad Ben-Yossef
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
[PATCH 0/2] Remove none supported ciphers
the CryptoCell HW has support for ciphers and modes not supported and used at this time by Linux. Remove the code supporting this in the ccree ddriver until such time support is added in the kernel. Gilad Ben-Yossef (2): crypto: ccree: remove data unit size support crypto: ccree: remove bitlocker cipher drivers/crypto/ccree/cc_cipher.c | 282 +-- drivers/crypto/ccree/cc_crypto_ctx.h | 1 - drivers/crypto/ccree/cc_driver.h | 1 - 3 files changed, 4 insertions(+), 280 deletions(-) -- 2.27.0
[PATCH 2/2] crypto: ccree: remove bitlocker cipher
Remove the bitlocker cipher which is not supported by
the kernel.
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_cipher.c | 49 ++--
drivers/crypto/ccree/cc_crypto_ctx.h | 1 -
2 files changed, 3 insertions(+), 47 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 0a74f3808510..50a0cd9abd49 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -96,8 +96,7 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p,
u32 size)
switch (size) {
case CC_AES_128_BIT_KEY_SIZE:
case CC_AES_192_BIT_KEY_SIZE:
- if (ctx_p->cipher_mode != DRV_CIPHER_XTS &&
- ctx_p->cipher_mode != DRV_CIPHER_BITLOCKER)
+ if (ctx_p->cipher_mode != DRV_CIPHER_XTS)
return 0;
break;
case CC_AES_256_BIT_KEY_SIZE:
@@ -105,8 +104,7 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p,
u32 size)
case (CC_AES_192_BIT_KEY_SIZE * 2):
case (CC_AES_256_BIT_KEY_SIZE * 2):
if (ctx_p->cipher_mode == DRV_CIPHER_XTS ||
- ctx_p->cipher_mode == DRV_CIPHER_ESSIV ||
- ctx_p->cipher_mode == DRV_CIPHER_BITLOCKER)
+ ctx_p->cipher_mode == DRV_CIPHER_ESSIV)
return 0;
break;
default:
@@ -143,7 +141,6 @@ static int validate_data_size(struct cc_cipher_ctx *ctx_p,
case DRV_CIPHER_ECB:
case DRV_CIPHER_CBC:
case DRV_CIPHER_ESSIV:
- case DRV_CIPHER_BITLOCKER:
if (IS_ALIGNED(size, AES_BLOCK_SIZE))
return 0;
break;
@@ -369,8 +366,7 @@ static int cc_cipher_sethkey(struct crypto_skcipher *sktfm,
const u8 *key,
}
if (ctx_p->cipher_mode == DRV_CIPHER_XTS ||
- ctx_p->cipher_mode == DRV_CIPHER_ESSIV ||
- ctx_p->cipher_mode == DRV_CIPHER_BITLOCKER) {
+ ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
if (hki.hw_key1 == hki.hw_key2) {
dev_err(dev, "Illegal hw key numbers (%d,%d)\n",
hki.hw_key1, hki.hw_key2);
@@ -568,7 +564,6 @@ static void cc_setup_readiv_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
- case DRV_CIPHER_BITLOCKER:
/* IV */
hw_desc_init([*seq_size]);
set_setup_mode([*seq_size], SETUP_WRITE_STATE1);
@@ -623,7 +618,6 @@ static void cc_setup_state_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
- case DRV_CIPHER_BITLOCKER:
break;
default:
dev_err(dev, "Unsupported cipher mode (%d)\n", cipher_mode);
@@ -657,7 +651,6 @@ static void cc_setup_xex_state_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
- case DRV_CIPHER_BITLOCKER:
if (cipher_mode == DRV_CIPHER_ESSIV)
key_len = SHA256_DIGEST_SIZE;
@@ -771,7 +764,6 @@ static void cc_setup_key_desc(struct crypto_tfm *tfm,
break;
case DRV_CIPHER_XTS:
case DRV_CIPHER_ESSIV:
- case DRV_CIPHER_BITLOCKER:
/* Load AES key */
hw_desc_init([*seq_size]);
set_cipher_mode([*seq_size], cipher_mode);
@@ -1069,24 +1061,6 @@ static const struct cc_alg_template skcipher_algs[] = {
.std_body = CC_STD_NIST,
.sec_func = true,
},
- {
- .name = "bitlocker(paes)",
- .driver_name = "bitlocker-paes-ccree",
- .blocksize = AES_BLOCK_SIZE,
- .template_skcipher = {
- .setkey = cc_cipher_sethkey,
- .encrypt = cc_cipher_encrypt,
- .decrypt = cc_cipher_decrypt,
- .min_keysize = CC_HW_KEY_SIZE,
- .max_keysize = CC_HW_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
- },
- .cipher_mode = DRV_CIPHER_BITLOCKER,
- .flow_mode = S_DIN_to_AES,
- .min_hw_rev = CC_HW_REV_712,
- .std_body = CC_STD_NIST,
- .sec_func = true,
- },
{
.name = "ecb(paes)",
.driver_name = "ecb-paes-ccree",
@@ -1215,23 +1189,6 @@ static const struct cc_alg_template skc
[PATCH 1/2] crypto: ccree: remove data unit size support
Remove the implementaion of automatic advancement of sector size in IV for
storage ciphers as its use is not supproted by the kernel.
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_cipher.c | 233 +--
drivers/crypto/ccree/cc_driver.h | 1 -
2 files changed, 1 insertion(+), 233 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 8e446a097086..0a74f3808510 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -645,16 +645,8 @@ static void cc_setup_xex_state_desc(struct crypto_tfm *tfm,
dma_addr_t key_dma_addr = ctx_p->user.key_dma_addr;
unsigned int key_len = (ctx_p->keylen / 2);
dma_addr_t iv_dma_addr = req_ctx->gen_ctx.iv_dma_addr;
- unsigned int du_size = nbytes;
unsigned int key_offset = key_len;
- struct cc_crypto_alg *cc_alg =
- container_of(tfm->__crt_alg, struct cc_crypto_alg,
-skcipher_alg.base);
-
- if (cc_alg->data_unit)
- du_size = cc_alg->data_unit;
-
switch (cipher_mode) {
case DRV_CIPHER_ECB:
break;
@@ -682,7 +674,7 @@ static void cc_setup_xex_state_desc(struct crypto_tfm *tfm,
(key_dma_addr + key_offset),
key_len, NS_BIT);
}
- set_xex_data_unit_size([*seq_size], du_size);
+ set_xex_data_unit_size([*seq_size], nbytes);
set_flow_mode([*seq_size], S_DIN_to_AES2);
set_key_size_aes([*seq_size], key_len);
set_setup_mode([*seq_size], SETUP_LOAD_XEX_KEY);
@@ -1059,44 +1051,6 @@ static const struct cc_alg_template skcipher_algs[] = {
.std_body = CC_STD_NIST,
.sec_func = true,
},
- {
- .name = "xts512(paes)",
- .driver_name = "xts-paes-du512-ccree",
- .blocksize = 1,
- .template_skcipher = {
- .setkey = cc_cipher_sethkey,
- .encrypt = cc_cipher_encrypt,
- .decrypt = cc_cipher_decrypt,
- .min_keysize = CC_HW_KEY_SIZE,
- .max_keysize = CC_HW_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
- },
- .cipher_mode = DRV_CIPHER_XTS,
- .flow_mode = S_DIN_to_AES,
- .data_unit = 512,
- .min_hw_rev = CC_HW_REV_712,
- .std_body = CC_STD_NIST,
- .sec_func = true,
- },
- {
- .name = "xts4096(paes)",
- .driver_name = "xts-paes-du4096-ccree",
- .blocksize = 1,
- .template_skcipher = {
- .setkey = cc_cipher_sethkey,
- .encrypt = cc_cipher_encrypt,
- .decrypt = cc_cipher_decrypt,
- .min_keysize = CC_HW_KEY_SIZE,
- .max_keysize = CC_HW_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
- },
- .cipher_mode = DRV_CIPHER_XTS,
- .flow_mode = S_DIN_to_AES,
- .data_unit = 4096,
- .min_hw_rev = CC_HW_REV_712,
- .std_body = CC_STD_NIST,
- .sec_func = true,
- },
{
.name = "essiv(cbc(paes),sha256)",
.driver_name = "essiv-paes-ccree",
@@ -1115,44 +1069,6 @@ static const struct cc_alg_template skcipher_algs[] = {
.std_body = CC_STD_NIST,
.sec_func = true,
},
- {
- .name = "essiv512(cbc(paes),sha256)",
- .driver_name = "essiv-paes-du512-ccree",
- .blocksize = AES_BLOCK_SIZE,
- .template_skcipher = {
- .setkey = cc_cipher_sethkey,
- .encrypt = cc_cipher_encrypt,
- .decrypt = cc_cipher_decrypt,
- .min_keysize = CC_HW_KEY_SIZE,
- .max_keysize = CC_HW_KEY_SIZE,
- .ivsize = AES_BLOCK_SIZE,
- },
- .cipher_mode = DRV_CIPHER_ESSIV,
- .flow_mode = S_DIN_to_AES,
- .data_unit = 512,
- .min_hw_rev = CC_HW_REV_712,
- .std_body = CC_STD_NIST,
- .sec_func = true,
- },
- {
- .name = "essiv4096(cbc(paes),sha256)",
- .driver_name = "essiv-paes-du4096-ccree",
- .blocksize = AES_BLOCK_SIZE,
- .template_skcipher = {
- .setkey = cc_cipher_sethkey,
- .encrypt = cc_cipher_encrypt,
- .de
Re: [PATCH v2 1/3] crypto: ccree: fix resource leak on error path
On Sun, Jun 21, 2020 at 4:07 PM Markus Elfring wrote: > > > Fix a small resource leak on the error path of cipher processing. > > I find it more appropriate to resend this patch series with a cover letter > together with all update steps. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=6469e8962c20b580b471790fe42367750599#n785 > Indeed and I did but for some reason I cannot fathom the cover letter did not make it to the list. I will try to resend it now. Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
[PATCH v2 0/3] fixes and update to essiv support
Small fixes and adapt essiv support to the new template format --- Changes from v1: - Incorporate coding style fixes suggested by Markus Elfring. Gilad Ben-Yossef (3): crypto: ccree: fix resource leak on error path crypto: ccree: adapt ccree essiv support to kcapi crypto: ccree: remove unused field drivers/crypto/ccree/cc_cipher.c | 149 ++- 1 file changed, 108 insertions(+), 41 deletions(-) -- 2.27.0
[PATCH v2 3/3] crypto: ccree: remove unused field
Remove yet another unused field left over from times gone by.
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_cipher.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 162871464438..442965b4cd9b 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -49,7 +49,6 @@ enum cc_key_type {
struct cc_cipher_ctx {
struct cc_drvdata *drvdata;
int keylen;
- int key_round_number;
int cipher_mode;
int flow_mode;
unsigned int flags;
--
2.27.0
[PATCH v2 2/3] crypto: ccree: adapt ccree essiv support to kcapi
The ESSIV support in ccree was added before the kernel
generic support and using a slightly different API.
Brings the ccree essiv interface into compliance with
kernel crypto api one.
Since CryptoCell only support 256 bit AES key for ESSIV,
also use a fallback if requested a smaller key size.
Signed-off-by: Gilad Ben-Yossef
Cc: Ard Biesheuvel
Cc: Libo Wang
Cc: Markus Elfring
---
drivers/crypto/ccree/cc_cipher.c | 124 +++
1 file changed, 93 insertions(+), 31 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index c2f759a04625..162871464438 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -60,6 +60,8 @@ struct cc_cipher_ctx {
struct cc_cpp_key_info cpp;
};
struct crypto_shash *shash_tfm;
+ struct crypto_skcipher *fallback_tfm;
+ bool fallback_on;
};
static void cc_cipher_complete(struct device *dev, void *cc_req, int err);
@@ -79,7 +81,6 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p,
u32 size)
case CC_AES_128_BIT_KEY_SIZE:
case CC_AES_192_BIT_KEY_SIZE:
if (ctx_p->cipher_mode != DRV_CIPHER_XTS &&
- ctx_p->cipher_mode != DRV_CIPHER_ESSIV &&
ctx_p->cipher_mode != DRV_CIPHER_BITLOCKER)
return 0;
break;
@@ -163,30 +164,49 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
skcipher_alg.base);
struct device *dev = drvdata_to_dev(cc_alg->drvdata);
unsigned int max_key_buf_size = cc_alg->skcipher_alg.max_keysize;
+ unsigned int fallback_req_size = 0;
dev_dbg(dev, "Initializing context @%p for %s\n", ctx_p,
crypto_tfm_alg_name(tfm));
- crypto_skcipher_set_reqsize(__crypto_skcipher_cast(tfm),
- sizeof(struct cipher_req_ctx));
-
ctx_p->cipher_mode = cc_alg->cipher_mode;
ctx_p->flow_mode = cc_alg->flow_mode;
ctx_p->drvdata = cc_alg->drvdata;
if (ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
+ const char *name = crypto_tfm_alg_name(tfm);
+
/* Alloc hash tfm for essiv */
- ctx_p->shash_tfm = crypto_alloc_shash("sha256-generic", 0, 0);
+ ctx_p->shash_tfm = crypto_alloc_shash("sha256", 0, 0);
if (IS_ERR(ctx_p->shash_tfm)) {
dev_err(dev, "Error allocating hash tfm for ESSIV.\n");
return PTR_ERR(ctx_p->shash_tfm);
}
+ max_key_buf_size <<= 1;
+
+ /* Alloc fallabck tfm or essiv when key size != 256 bit */
+ ctx_p->fallback_tfm =
+ crypto_alloc_skcipher(name, 0, CRYPTO_ALG_NEED_FALLBACK
| CRYPTO_ALG_ASYNC);
+
+ if (IS_ERR(ctx_p->fallback_tfm)) {
+ /* Note we're still allowing registration with no
fallback since it's
+* better to have most modes supported than none at all.
+*/
+ dev_warn(dev, "Error allocating fallback algo %s. Some
modes may be available.\n",
+ name);
+ ctx_p->fallback_tfm = NULL;
+ } else {
+ fallback_req_size =
crypto_skcipher_reqsize(ctx_p->fallback_tfm);
+ }
}
+ crypto_skcipher_set_reqsize(__crypto_skcipher_cast(tfm),
+ sizeof(struct cipher_req_ctx) +
fallback_req_size);
+
/* Allocate key buffer, cache line aligned */
- ctx_p->user.key = kmalloc(max_key_buf_size, GFP_KERNEL);
+ ctx_p->user.key = kzalloc(max_key_buf_size, GFP_KERNEL);
if (!ctx_p->user.key)
- goto free_shash;
+ goto free_fallback;
dev_dbg(dev, "Allocated key buffer in context. key=@%p\n",
ctx_p->user.key);
@@ -207,7 +227,8 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
free_key:
kfree(ctx_p->user.key);
-free_shash:
+free_fallback:
+ crypto_free_skcipher(ctx_p->fallback_tfm);
crypto_free_shash(ctx_p->shash_tfm);
return -ENOMEM;
@@ -230,6 +251,8 @@ static void cc_cipher_exit(struct crypto_tfm *tfm)
/* Free hash tfm for essiv */
crypto_free_shash(ctx_p->shash_tfm);
ctx_p->shash_tfm = NULL;
+ crypto_free_skcipher(ctx_p->fallback_tfm);
+ ctx_p->fallback_tfm = NULL;
}
/* Unmap key buffer */
@@ -313,6 +336,7 @@ static int cc_cipher_sethkey(struct crypto_skcipher *sktfm,
const u8 *key,
}
c
[PATCH v2 0/3] fixes and update to essiv support
Small fixes and adapt essiv support to the new template format --- Changes from v1: - Incorporate coding style fixes suggested by Markus Elfring. Gilad Ben-Yossef (3): crypto: ccree: fix resource leak on error path crypto: ccree: adapt ccree essiv support to kcapi crypto: ccree: remove unused field drivers/crypto/ccree/cc_cipher.c | 149 ++- 1 file changed, 108 insertions(+), 41 deletions(-) -- 2.27.0
[PATCH v2 1/3] crypto: ccree: fix resource leak on error path
Fix a small resource leak on the error path of cipher processing.
Signed-off-by: Gilad Ben-Yossef
Fixes: 63ee04c8b491e ("crypto: ccree - add skcipher support")
Cc: Markus Elfring
---
drivers/crypto/ccree/cc_cipher.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 42ec46be427d..c2f759a04625 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -163,7 +163,6 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
skcipher_alg.base);
struct device *dev = drvdata_to_dev(cc_alg->drvdata);
unsigned int max_key_buf_size = cc_alg->skcipher_alg.max_keysize;
- int rc = 0;
dev_dbg(dev, "Initializing context @%p for %s\n", ctx_p,
crypto_tfm_alg_name(tfm));
@@ -175,10 +174,19 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
ctx_p->flow_mode = cc_alg->flow_mode;
ctx_p->drvdata = cc_alg->drvdata;
+ if (ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
+ /* Alloc hash tfm for essiv */
+ ctx_p->shash_tfm = crypto_alloc_shash("sha256-generic", 0, 0);
+ if (IS_ERR(ctx_p->shash_tfm)) {
+ dev_err(dev, "Error allocating hash tfm for ESSIV.\n");
+ return PTR_ERR(ctx_p->shash_tfm);
+ }
+ }
+
/* Allocate key buffer, cache line aligned */
ctx_p->user.key = kmalloc(max_key_buf_size, GFP_KERNEL);
if (!ctx_p->user.key)
- return -ENOMEM;
+ goto free_shash;
dev_dbg(dev, "Allocated key buffer in context. key=@%p\n",
ctx_p->user.key);
@@ -190,21 +198,19 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
if (dma_mapping_error(dev, ctx_p->user.key_dma_addr)) {
dev_err(dev, "Mapping Key %u B at va=%pK for DMA failed\n",
max_key_buf_size, ctx_p->user.key);
- return -ENOMEM;
+ goto free_key;
}
dev_dbg(dev, "Mapped key %u B at va=%pK to dma=%pad\n",
max_key_buf_size, ctx_p->user.key, _p->user.key_dma_addr);
- if (ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
- /* Alloc hash tfm for essiv */
- ctx_p->shash_tfm = crypto_alloc_shash("sha256-generic", 0, 0);
- if (IS_ERR(ctx_p->shash_tfm)) {
- dev_err(dev, "Error allocating hash tfm for ESSIV.\n");
- return PTR_ERR(ctx_p->shash_tfm);
- }
- }
+ return 0;
- return rc;
+free_key:
+ kfree(ctx_p->user.key);
+free_shash:
+ crypto_free_shash(ctx_p->shash_tfm);
+
+ return -ENOMEM;
}
static void cc_cipher_exit(struct crypto_tfm *tfm)
--
2.27.0
Re: [PATCH 2/3] crypto: ccree: adapt ccree essiv support to kcapi
Hi Markus, On Sun, Jun 21, 2020 at 1:11 PM Markus Elfring wrote: > > I propose to avoid a typo in the previous patch subject. > > > > This patch brings the ccree essiv interface into > > compliance with kernel crypto api one. > > Can an imperative wording be nicer for the commit message? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=6469e8962c20b580b471790fe42367750599#n151 > Thank you for the review and comments for this and the previous patch. I have taken all your suggestions into consideration and will include them in the next revision of the patch set. Thanks again, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
[PATCH 3/3] crypto: ccree: remove unused field
Remove yet another unused field left over from times gone by.
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_cipher.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 6575e216262b..79cfb47c759c 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -49,7 +49,6 @@ enum cc_key_type {
struct cc_cipher_ctx {
struct cc_drvdata *drvdata;
int keylen;
- int key_round_number;
int cipher_mode;
int flow_mode;
unsigned int flags;
--
2.27.0
[PATCH 2/3] crypto: ccree: adapt ccree essiv supprot to kcapi
The ESSIV support in ccree was added before the kernel
generic support and using a slightly different API.
This patch brings the ccree essiv interface into
compliance with kernel crypto api one.
Since CryptoCell only support 256 bit AES key for ESSIV,
also use a fallback if requested a smaller key size.
Signed-off-by: Gilad Ben-Yossef
Cc: Ard Biesheuvel
Cc: Libo Wang
---
drivers/crypto/ccree/cc_cipher.c | 124 +++
1 file changed, 93 insertions(+), 31 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index e61f35ca9945..6575e216262b 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -60,6 +60,8 @@ struct cc_cipher_ctx {
struct cc_cpp_key_info cpp;
};
struct crypto_shash *shash_tfm;
+ struct crypto_skcipher *fallback_tfm;
+ bool fallback_on;
};
static void cc_cipher_complete(struct device *dev, void *cc_req, int err);
@@ -79,7 +81,6 @@ static int validate_keys_sizes(struct cc_cipher_ctx *ctx_p,
u32 size)
case CC_AES_128_BIT_KEY_SIZE:
case CC_AES_192_BIT_KEY_SIZE:
if (ctx_p->cipher_mode != DRV_CIPHER_XTS &&
- ctx_p->cipher_mode != DRV_CIPHER_ESSIV &&
ctx_p->cipher_mode != DRV_CIPHER_BITLOCKER)
return 0;
break;
@@ -163,30 +164,49 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
skcipher_alg.base);
struct device *dev = drvdata_to_dev(cc_alg->drvdata);
unsigned int max_key_buf_size = cc_alg->skcipher_alg.max_keysize;
+ unsigned int fallback_req_size = 0;
dev_dbg(dev, "Initializing context @%p for %s\n", ctx_p,
crypto_tfm_alg_name(tfm));
- crypto_skcipher_set_reqsize(__crypto_skcipher_cast(tfm),
- sizeof(struct cipher_req_ctx));
-
ctx_p->cipher_mode = cc_alg->cipher_mode;
ctx_p->flow_mode = cc_alg->flow_mode;
ctx_p->drvdata = cc_alg->drvdata;
if (ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
+ const char *name = crypto_tfm_alg_name(tfm);
+
/* Alloc hash tfm for essiv */
- ctx_p->shash_tfm = crypto_alloc_shash("sha256-generic", 0, 0);
+ ctx_p->shash_tfm = crypto_alloc_shash("sha256", 0, 0);
if (IS_ERR(ctx_p->shash_tfm)) {
dev_err(dev, "Error allocating hash tfm for ESSIV.\n");
return PTR_ERR(ctx_p->shash_tfm);
}
+ max_key_buf_size <<= 1;
+
+ /* Alloc fallabck tfm or essiv when key size != 256 bit */
+ ctx_p->fallback_tfm =
+ crypto_alloc_skcipher(name, 0, CRYPTO_ALG_NEED_FALLBACK
| CRYPTO_ALG_ASYNC);
+
+ if (IS_ERR(ctx_p->fallback_tfm)) {
+ /* Note we're still allowing registration with no
fallback since it's
+* better to have most modes supported than none at all.
+*/
+ dev_warn(dev, "Error allocating fallback algo %s. Some
modes may be available.\n",
+ name);
+ ctx_p->fallback_tfm = NULL;
+ } else {
+ fallback_req_size =
crypto_skcipher_reqsize(ctx_p->fallback_tfm);
+ }
}
+ crypto_skcipher_set_reqsize(__crypto_skcipher_cast(tfm),
+ sizeof(struct cipher_req_ctx) +
fallback_req_size);
+
/* Allocate key buffer, cache line aligned */
- ctx_p->user.key = kmalloc(max_key_buf_size, GFP_KERNEL);
+ ctx_p->user.key = kzalloc(max_key_buf_size, GFP_KERNEL);
if (!ctx_p->user.key)
- goto out_shash;
+ goto out_fallback;
dev_dbg(dev, "Allocated key buffer in context. key=@%p\n",
ctx_p->user.key);
@@ -207,7 +227,8 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
out_key:
kfree(ctx_p->user.key);
-out_shash:
+out_fallback:
+ crypto_free_skcipher(ctx_p->fallback_tfm);
crypto_free_shash(ctx_p->shash_tfm);
return -ENOMEM;
@@ -230,6 +251,8 @@ static void cc_cipher_exit(struct crypto_tfm *tfm)
/* Free hash tfm for essiv */
crypto_free_shash(ctx_p->shash_tfm);
ctx_p->shash_tfm = NULL;
+ crypto_free_skcipher(ctx_p->fallback_tfm);
+ ctx_p->fallback_tfm = NULL;
}
/* Unmap key buffer */
@@ -313,6 +336,7 @@ static int cc_cipher_sethkey(struct crypto_skcipher *sktfm,
const u8 *key,
}
ctx_p
[PATCH 0/3] fixes and update to essiv support
Small fixes and adapt essiv support to the new template format Gilad Ben-Yossef (3): crypto: ccree: fix resource leak on error path crypto: ccree: adapt ccree essiv supprot to kcapi crypto: ccree: remove unused field drivers/crypto/ccree/cc_cipher.c | 149 ++- 1 file changed, 108 insertions(+), 41 deletions(-) -- 2.27.0
[PATCH 1/3] crypto: ccree: fix resource leak on error path
Fix a small resource leak on the error path of cipher processing.
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_cipher.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 42ec46be427d..e61f35ca9945 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -163,7 +163,6 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
skcipher_alg.base);
struct device *dev = drvdata_to_dev(cc_alg->drvdata);
unsigned int max_key_buf_size = cc_alg->skcipher_alg.max_keysize;
- int rc = 0;
dev_dbg(dev, "Initializing context @%p for %s\n", ctx_p,
crypto_tfm_alg_name(tfm));
@@ -175,10 +174,19 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
ctx_p->flow_mode = cc_alg->flow_mode;
ctx_p->drvdata = cc_alg->drvdata;
+ if (ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
+ /* Alloc hash tfm for essiv */
+ ctx_p->shash_tfm = crypto_alloc_shash("sha256-generic", 0, 0);
+ if (IS_ERR(ctx_p->shash_tfm)) {
+ dev_err(dev, "Error allocating hash tfm for ESSIV.\n");
+ return PTR_ERR(ctx_p->shash_tfm);
+ }
+ }
+
/* Allocate key buffer, cache line aligned */
ctx_p->user.key = kmalloc(max_key_buf_size, GFP_KERNEL);
if (!ctx_p->user.key)
- return -ENOMEM;
+ goto out_shash;
dev_dbg(dev, "Allocated key buffer in context. key=@%p\n",
ctx_p->user.key);
@@ -190,21 +198,19 @@ static int cc_cipher_init(struct crypto_tfm *tfm)
if (dma_mapping_error(dev, ctx_p->user.key_dma_addr)) {
dev_err(dev, "Mapping Key %u B at va=%pK for DMA failed\n",
max_key_buf_size, ctx_p->user.key);
- return -ENOMEM;
+ goto out_key;
}
dev_dbg(dev, "Mapped key %u B at va=%pK to dma=%pad\n",
max_key_buf_size, ctx_p->user.key, _p->user.key_dma_addr);
- if (ctx_p->cipher_mode == DRV_CIPHER_ESSIV) {
- /* Alloc hash tfm for essiv */
- ctx_p->shash_tfm = crypto_alloc_shash("sha256-generic", 0, 0);
- if (IS_ERR(ctx_p->shash_tfm)) {
- dev_err(dev, "Error allocating hash tfm for ESSIV.\n");
- return PTR_ERR(ctx_p->shash_tfm);
- }
- }
+ return 0;
- return rc;
+out_key:
+ kfree(ctx_p->user.key);
+out_shash:
+ crypto_free_shash(ctx_p->shash_tfm);
+
+ return -ENOMEM;
}
static void cc_cipher_exit(struct crypto_tfm *tfm)
--
2.27.0
Re: [PATCH 5/9] crypto: ccree - Rename arrays to avoid conflict with crypto/sha256.h
On Sun, Sep 1, 2019 at 11:36 PM Hans de Goede wrote: > > Rename the algo_init arrays to cc_algo_init so that they do not conflict > with the functions declared in crypto/sha256.h. > > This is a preparation patch for folding crypto/sha256.h into crypto/sha.h. I'm fine with the renaming. Signed-off-by: Gilad Ben-Yossef Thanks, Gilad
[PATCH v2] checkpatch: add *_NOTIFIER_HEAD as var definition
Add *_NOTIFIER_HEAD as variable definition to avoid code like this:
ATOMIC_NOTIFIER_HEAD(foo);
EXPORT_SYMBOL_GPL(foo);
>From triggering the the following warning:
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
Signed-off-by: Gilad Ben-Yossef
Cc: John Hubbard
---
Changes from v1:
- Fixed misposition of braces.
- Tested on 1k last commits from Linux tree.
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 93a7edfe0f05..8bc0e753a329 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3864,6 +3864,7 @@ sub process {
^.DEFINE_$Ident\(\Q$name\E\)|
^.DECLARE_$Ident\(\Q$name\E\)|
^.LIST_HEAD\(\Q$name\E\)|
+ ^.${Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
\b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\()
)/x) {
--
2.23.0
[PATCH v2] checkpatch: add *_NOTIFIER_HEAD as var definition
Add *_NOTIFIER_HEAD as variable definition to avoid code like this:
ATOMIC_NOTIFIER_HEAD(foo);
EXPORT_SYMBOL_GPL(foo);
>From triggering the the following warning:
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
Signed-off-by: Gilad Ben-Yossef
---
Changes from v1:
- Better RegExp as suggested by Joe Perches.
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 342c7c781ba5..9cadda7024ae 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3864,6 +3864,7 @@ sub process {
^.DEFINE_$Ident\(\Q$name\E\)|
^.DECLARE_$Ident\(\Q$name\E\)|
^.LIST_HEAD\(\Q$name\E\)|
+ ^.{$Ident}_NOTIFIER_HEAD\(\Q$name\E\)|
^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
\b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\()
)/x) {
--
2.21.0
[PATCH] checkpatch: add *_NOTIFIER_HEAD as var definition
Add *_NOTIFIER_HEAD as variable definition to avoid code like this:
ATOMIC_NOTIFIER_HEAD(foo);
EXPORT_SYMBOL_GPL(foo);
>From triggering the the following warning:
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
Signed-off-by: Gilad Ben-Yossef
---
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 342c7c781ba5..2377707f4441 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3864,6 +3864,7 @@ sub process {
^.DEFINE_$Ident\(\Q$name\E\)|
^.DECLARE_$Ident\(\Q$name\E\)|
^.LIST_HEAD\(\Q$name\E\)|
+ ^.$Ident._NOTIFIER_HEAD\(\Q$name\E\)|
^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
\b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\()
)/x) {
--
2.21.0
[PATCH 0/4] crypto: ccree: cleanups, fixes and TEE FIPS support
Clean up unused ivgen support code and add support for notifiying Trusted Execution Enviornment of FIPS tests failures in FIPS mode. Gilad Ben-Yossef (4): crypto: ccree: drop legacy ivgen support crypto: ccree: account for TEE not ready to report crypto: fips: add FIPS test failure notification chain crypto: ccree: notify TEE on FIPS tests errors crypto/fips.c | 11 + crypto/testmgr.c | 4 +- drivers/crypto/ccree/Makefile | 2 +- drivers/crypto/ccree/cc_aead.c| 76 +-- drivers/crypto/ccree/cc_aead.h| 3 +- drivers/crypto/ccree/cc_driver.c | 12 +- drivers/crypto/ccree/cc_driver.h | 10 - drivers/crypto/ccree/cc_fips.c| 31 ++- drivers/crypto/ccree/cc_ivgen.c | 276 -- drivers/crypto/ccree/cc_ivgen.h | 55 - drivers/crypto/ccree/cc_pm.c | 2 - drivers/crypto/ccree/cc_request_mgr.c | 47 + include/linux/fips.h | 7 + 13 files changed, 68 insertions(+), 468 deletions(-) delete mode 100644 drivers/crypto/ccree/cc_ivgen.c delete mode 100644 drivers/crypto/ccree/cc_ivgen.h -- 2.21.0
[PATCH 2/4] crypto: ccree: account for TEE not ready to report
When ccree driver runs it checks the state of the Trusted Execution
Environment CryptoCell driver before proceeding. We did not account
for cases where the TEE side is not ready or not available at all.
Fix it by only considering TEE error state after sync with the TEE
side driver.
Signed-off-by: Gilad Ben-Yossef
Fixes: ab8ec9658f5a ("crypto: ccree - add FIPS support")
CC: sta...@vger.kernel.org # v4.17+
---
drivers/crypto/ccree/cc_fips.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_fips.c b/drivers/crypto/ccree/cc_fips.c
index 5ad3ffb7acaa..040e09c0e1af 100644
--- a/drivers/crypto/ccree/cc_fips.c
+++ b/drivers/crypto/ccree/cc_fips.c
@@ -21,7 +21,13 @@ static bool cc_get_tee_fips_status(struct cc_drvdata
*drvdata)
u32 reg;
reg = cc_ioread(drvdata, CC_REG(GPR_HOST));
- return (reg == (CC_FIPS_SYNC_TEE_STATUS | CC_FIPS_SYNC_MODULE_OK));
+ /* Did the TEE report status? */
+ if (reg & CC_FIPS_SYNC_TEE_STATUS)
+ /* Yes. Is it OK? */
+ return (reg & CC_FIPS_SYNC_MODULE_OK);
+
+ /* No. It's either not in use or will be reported later */
+ return true;
}
/*
--
2.21.0
[PATCH 3/4] crypto: fips: add FIPS test failure notification chain
Crypto test failures in FIPS mode cause an immediate panic, but
on some system the cryptographic boundary extends beyond just
the Linux controlled domain.
Add a simple atomic notification chain to allow interested parties
to register to receive notification prior to us kicking the bucket.
Signed-off-by: Gilad Ben-Yossef
---
crypto/fips.c| 11 +++
crypto/testmgr.c | 4 +++-
include/linux/fips.h | 7 +++
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/crypto/fips.c b/crypto/fips.c
index 9dfed122d6da..b30a67b6c441 100644
--- a/crypto/fips.c
+++ b/crypto/fips.c
@@ -16,10 +16,14 @@
#include
#include
#include
+#include
int fips_enabled;
EXPORT_SYMBOL_GPL(fips_enabled);
+ATOMIC_NOTIFIER_HEAD(fips_fail_notif_chain);
+EXPORT_SYMBOL_GPL(fips_fail_notif_chain);
+
/* Process kernel command-line parameter at boot time. fips=0 or fips=1 */
static int fips_enable(char *str)
{
@@ -63,6 +67,13 @@ static void crypto_proc_fips_exit(void)
unregister_sysctl_table(crypto_sysctls);
}
+void fips_fail_notify(void)
+{
+ if (fips_enabled)
+ atomic_notifier_call_chain(_fail_notif_chain, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(fips_fail_notify);
+
static int __init fips_init(void)
{
crypto_proc_fips_init();
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index d760f5cd35b2..fc2407d7a78f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -5245,9 +5245,11 @@ int alg_test(const char *driver, const char *alg, u32
type, u32 mask)
type, mask);
test_done:
- if (rc && (fips_enabled || panic_on_fail))
+ if (rc && (fips_enabled || panic_on_fail)) {
+ fips_fail_notify();
panic("alg: self-tests for %s (%s) failed in %s mode!\n",
driver, alg, fips_enabled ? "fips" : "panic_on_fail");
+ }
if (fips_enabled && !rc)
pr_info("alg: self-tests for %s (%s) passed\n", driver, alg);
diff --git a/include/linux/fips.h b/include/linux/fips.h
index afeeece92302..c6961e932fef 100644
--- a/include/linux/fips.h
+++ b/include/linux/fips.h
@@ -4,8 +4,15 @@
#ifdef CONFIG_CRYPTO_FIPS
extern int fips_enabled;
+extern struct atomic_notifier_head fips_fail_notif_chain;
+
+void fips_fail_notify(void);
+
#else
#define fips_enabled 0
+
+static inline void fips_fail_notify(void) {}
+
#endif
#endif
--
2.21.0
Re: [PATCH] crypto: ccree: fix spelling mistake "protedcted" -> "protected"
On Fri, Apr 26, 2019 at 4:18 PM Colin King wrote: > > From: Colin Ian King > > There is a spelling mistake in a dev_dbg message, fix it. > > Signed-off-by: Colin Ian King Acked-By: Gilad Ben-Yossef Thanks! Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
[PATCH 02/35] crypto: ccree: move key load desc. before flow desc.
Refactor the descriptor setup code in order to move the key loading
descriptor to one before last position. This has no effect on current
functionality but is needed for later support of Content Protection
Policy keys.
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_cipher.c | 107 +--
1 file changed, 73 insertions(+), 34 deletions(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 0abcdc224ab0..dcab96861d6f 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -399,7 +399,7 @@ static int cc_cipher_setkey(struct crypto_skcipher *sktfm,
const u8 *key,
return 0;
}
-static void cc_setup_cipher_desc(struct crypto_tfm *tfm,
+static void cc_setup_state_desc(struct crypto_tfm *tfm,
struct cipher_req_ctx *req_ctx,
unsigned int ivsize, unsigned int nbytes,
struct cc_hw_desc desc[],
@@ -423,11 +423,13 @@ static void cc_setup_cipher_desc(struct crypto_tfm *tfm,
du_size = cc_alg->data_unit;
switch (cipher_mode) {
+ case DRV_CIPHER_ECB:
+ break;
case DRV_CIPHER_CBC:
case DRV_CIPHER_CBC_CTS:
case DRV_CIPHER_CTR:
case DRV_CIPHER_OFB:
- /* Load cipher state */
+ /* Load IV */
hw_desc_init([*seq_size]);
set_din_type([*seq_size], DMA_DLLI, iv_dma_addr, ivsize,
NS_BIT);
@@ -441,7 +443,71 @@ static void cc_setup_cipher_desc(struct crypto_tfm *tfm,
set_setup_mode([*seq_size], SETUP_LOAD_STATE0);
}
(*seq_size)++;
- /*FALLTHROUGH*/
+ break;
+ case DRV_CIPHER_XTS:
+ case DRV_CIPHER_ESSIV:
+ case DRV_CIPHER_BITLOCKER:
+ /* load XEX key */
+ hw_desc_init([*seq_size]);
+ set_cipher_mode([*seq_size], cipher_mode);
+ set_cipher_config0([*seq_size], direction);
+ if (cc_is_hw_key(tfm)) {
+ set_hw_crypto_key([*seq_size],
+ ctx_p->hw.key2_slot);
+ } else {
+ set_din_type([*seq_size], DMA_DLLI,
+(key_dma_addr + (key_len / 2)),
+(key_len / 2), NS_BIT);
+ }
+ set_xex_data_unit_size([*seq_size], du_size);
+ set_flow_mode([*seq_size], S_DIN_to_AES2);
+ set_key_size_aes([*seq_size], (key_len / 2));
+ set_setup_mode([*seq_size], SETUP_LOAD_XEX_KEY);
+ (*seq_size)++;
+
+ /* Load IV */
+ hw_desc_init([*seq_size]);
+ set_setup_mode([*seq_size], SETUP_LOAD_STATE1);
+ set_cipher_mode([*seq_size], cipher_mode);
+ set_cipher_config0([*seq_size], direction);
+ set_key_size_aes([*seq_size], (key_len / 2));
+ set_flow_mode([*seq_size], flow_mode);
+ set_din_type([*seq_size], DMA_DLLI, iv_dma_addr,
+CC_AES_BLOCK_SIZE, NS_BIT);
+ (*seq_size)++;
+ break;
+ default:
+ dev_err(dev, "Unsupported cipher mode (%d)\n", cipher_mode);
+ }
+}
+
+
+static void cc_setup_key_desc(struct crypto_tfm *tfm,
+ struct cipher_req_ctx *req_ctx,
+ unsigned int nbytes, struct cc_hw_desc desc[],
+ unsigned int *seq_size)
+{
+ struct cc_cipher_ctx *ctx_p = crypto_tfm_ctx(tfm);
+ struct device *dev = drvdata_to_dev(ctx_p->drvdata);
+ int cipher_mode = ctx_p->cipher_mode;
+ int flow_mode = ctx_p->flow_mode;
+ int direction = req_ctx->gen_ctx.op_type;
+ dma_addr_t key_dma_addr = ctx_p->user.key_dma_addr;
+ unsigned int key_len = ctx_p->keylen;
+ unsigned int du_size = nbytes;
+
+ struct cc_crypto_alg *cc_alg =
+ container_of(tfm->__crt_alg, struct cc_crypto_alg,
+skcipher_alg.base);
+
+ if (cc_alg->data_unit)
+ du_size = cc_alg->data_unit;
+
+ switch (cipher_mode) {
+ case DRV_CIPHER_CBC:
+ case DRV_CIPHER_CBC_CTS:
+ case DRV_CIPHER_CTR:
+ case DRV_CIPHER_OFB:
case DRV_CIPHER_ECB:
/* Load key */
hw_desc_init([*seq_size]);
@@ -486,35 +552,6 @@ static void cc_setup_cipher_desc(struct crypto_tfm *tfm,
set_flow_mode([*seq_size], flow_mode);
set_setup_mode([*seq_size], SETUP_LOAD_KEY0);
(*seq_size)++;
-
- /* load XEX key */
- hw_desc_init([*seq_size]);
- set_cipher_mode([*seq_size], ciphe
[PATCH 00/35] crypto: ccree: features and bug fixes for 5.2
A set of new features, mostly support for CryptoCell 713 features including protected keys, security disable mode and new HW revision indetification interface alongside many bug fixes. Gilad Ben-Yossef (30): crypto: testmgr: add missing self test entries for protected keys crypto: ccree: move key load desc. before flow desc. crypto: ccree: move MLLI desc. before key load crypto: ccree: add support for sec disabled mode crypto: ccree: add CPP completion handling crypto: ccree: add remaining logic for CPP crypto: ccree: add SM4 protected keys support crypto: ccree: adapt CPP descriptor to new HW crypto: ccree: read next IV from HW crypto: ccree: add CID and PID support crypto: ccree: fix backlog notifications crypto: ccree: use proper callback completion api crypto: ccree: remove special handling of chained sg crypto: ccree: fix typo in debugfs error path crypto: ccree: fix mem leak on error path crypto: ccree: use devm_kzalloc for device data crypto: ccree: use std api when possible crypto: ccree: copyright header update crypto: ccree: zero out internal struct before use crypto: ccree: do not copy zero size MLLI table crypto: ccree: remove unused defines crypto: ccree: simplify fragment ICV detection crypto: ccree: simplify AEAD ICV addr calculation crypto: ccree: don't mangle the request assoclen crypto: ccree: make AEAD sgl iterator well behaved crypto: ccree: zap entire sg on aead request unmap crypto: ccree: use correct internal state sizes for export crypto: ccree: allow more AEAD assoc data fragments crypto: ccree: don't map MAC key on stack crypto: ccree: don't map AEAD key and IV on stack Ofir Drang (5): crypto: ccree: pm resume first enable the source clk crypto: ccree: remove cc7x3 obsoleted AXIM configs crypto: ccree: HOST_POWER_DOWN_EN should be the last CC access during suspend crypto: ccree: add function to handle cryptocell tee fips error crypto: ccree: handle tee fips error during power management resume crypto/testmgr.c| 20 + drivers/crypto/ccree/Makefile | 1 + drivers/crypto/ccree/cc_aead.c | 81 +++- drivers/crypto/ccree/cc_aead.h | 3 +- drivers/crypto/ccree/cc_buffer_mgr.c| 341 -- drivers/crypto/ccree/cc_buffer_mgr.h| 2 +- drivers/crypto/ccree/cc_cipher.c| 591 +++- drivers/crypto/ccree/cc_cipher.h| 3 +- drivers/crypto/ccree/cc_crypto_ctx.h| 10 +- drivers/crypto/ccree/cc_debugfs.c | 44 +- drivers/crypto/ccree/cc_debugfs.h | 2 +- drivers/crypto/ccree/cc_driver.c| 120 - drivers/crypto/ccree/cc_driver.h| 36 +- drivers/crypto/ccree/cc_fips.c | 29 +- drivers/crypto/ccree/cc_fips.h | 4 +- drivers/crypto/ccree/cc_hash.c | 64 ++- drivers/crypto/ccree/cc_hash.h | 2 +- drivers/crypto/ccree/cc_host_regs.h | 123 - drivers/crypto/ccree/cc_hw_queue_defs.h | 35 +- drivers/crypto/ccree/cc_ivgen.c | 11 +- drivers/crypto/ccree/cc_ivgen.h | 2 +- drivers/crypto/ccree/cc_kernel_regs.h | 2 +- drivers/crypto/ccree/cc_lli_defs.h | 4 +- drivers/crypto/ccree/cc_pm.c| 11 +- drivers/crypto/ccree/cc_pm.h| 2 +- drivers/crypto/ccree/cc_request_mgr.c | 116 +++-- drivers/crypto/ccree/cc_request_mgr.h | 2 +- drivers/crypto/ccree/cc_sram_mgr.c | 7 +- drivers/crypto/ccree/cc_sram_mgr.h | 2 +- 29 files changed, 1068 insertions(+), 602 deletions(-) -- 2.21.0
Re: [PATCH] crypto: ccree: add missing inline qualifier
Hi,
On Mon, Feb 18, 2019 at 10:48 AM Geert Uytterhoeven
wrote:
>
> On Mon, Feb 11, 2019 at 3:29 PM Gilad Ben-Yossef wrote:
> > Commit 1358c13a48c4 ("crypto: ccree - fix resume race condition on init")
> > was missing a "inline" qualifier for stub function used when CONFIG_PM
> > is not set causing a build warning.
> >
> > Fixes: 1358c13a48c4 ("crypto: ccree - fix resume race condition on init")
> > Cc: sta...@kernel.org # v4.20
>
> # v5.0
>
> However, I believe the version comment is not necessary, as the same
> version is indicated by the Fixes tag.
I've marked it v4.20 since I've marked the patch that this fixes for
inclusion in 4.20.
Although the underlying bug was there all along, it was only revealed
by a change that
went into 4.20.
>
> > Signed-off-by: Gilad Ben-Yossef
>
> Acked-by: Geert Uytterhoeven
>
Thanks!
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
[PATCH] crypto: ccree: add missing inline qualifier
Commit 1358c13a48c4 ("crypto: ccree - fix resume race condition on init")
was missing a "inline" qualifier for stub function used when CONFIG_PM
is not set causing a build warning.
Fixes: 1358c13a48c4 ("crypto: ccree - fix resume race condition on init")
Cc: sta...@kernel.org # v4.20
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_pm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_pm.h b/drivers/crypto/ccree/cc_pm.h
index f62624357020..907a6db4d6c0 100644
--- a/drivers/crypto/ccree/cc_pm.h
+++ b/drivers/crypto/ccree/cc_pm.h
@@ -30,7 +30,7 @@ static inline int cc_pm_init(struct cc_drvdata *drvdata)
return 0;
}
-static void cc_pm_go(struct cc_drvdata *drvdata) {}
+static inline void cc_pm_go(struct cc_drvdata *drvdata) {}
static inline void cc_pm_fini(struct cc_drvdata *drvdata) {}
--
2.20.1
[PATCH] MAINTAINERS: crypto: ccree: remove co-maintainer
The best-laid plans of mice and men often go awry. Remove Yael C. as co-maintainer as she moved on to other endeavours. Signed-off-by: Gilad Ben-Yossef --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 8c68de3cfd80..45ea92863a05 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3511,7 +3511,6 @@ F:include/linux/spi/cc2520.h F: Documentation/devicetree/bindings/net/ieee802154/cc2520.txt CCREE ARM TRUSTZONE CRYPTOCELL REE DRIVER -M: Yael Chemla M: Gilad Ben-Yossef L: linux-cry...@vger.kernel.org S: Supported -- 2.20.1
[BUGFIX PATCH] crypto: ccree: fix resume race condition on init
We were enabling autosuspend, which is using data set by the
hash module, prior to the hash module being inited, casuing
a crash on resume as part of the startup sequence if the race
was lost.
This was never a real problem because the PM infra was using low
res timers so we were always winning the race, until commit 8234f6734c5d
("PM-runtime: Switch autosuspend over to using hrtimers") changed that :-)
Fix this by seperating the PM setup and enablement and doing the
latter only at the end of the init sequence.
Signed-off-by: Gilad Ben-Yossef
Cc: Vincent Guittot
Cc: sta...@kernel.org # v4.20
---
Herbert, could you please take this for 5.0-rc6 ? thanks.
drivers/crypto/ccree/cc_driver.c | 7 ---
drivers/crypto/ccree/cc_pm.c | 13 ++---
drivers/crypto/ccree/cc_pm.h | 3 +++
3 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 210cc86605e9..3bcc6c76e090 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -380,7 +380,7 @@ static int init_cc_resources(struct platform_device
*plat_dev)
rc = cc_ivgen_init(new_drvdata);
if (rc) {
dev_err(dev, "cc_ivgen_init failed\n");
- goto post_power_mgr_err;
+ goto post_buf_mgr_err;
}
/* Allocate crypto algs */
@@ -403,6 +403,9 @@ static int init_cc_resources(struct platform_device
*plat_dev)
goto post_hash_err;
}
+ /* All set, we can allow autosuspend */
+ cc_pm_go(new_drvdata);
+
/* If we got here and FIPS mode is enabled
* it means all FIPS test passed, so let TEE
* know we're good.
@@ -417,8 +420,6 @@ static int init_cc_resources(struct platform_device
*plat_dev)
cc_cipher_free(new_drvdata);
post_ivgen_err:
cc_ivgen_fini(new_drvdata);
-post_power_mgr_err:
- cc_pm_fini(new_drvdata);
post_buf_mgr_err:
cc_buffer_mgr_fini(new_drvdata);
post_req_mgr_err:
diff --git a/drivers/crypto/ccree/cc_pm.c b/drivers/crypto/ccree/cc_pm.c
index d990f472e89f..6ff7e75ad90e 100644
--- a/drivers/crypto/ccree/cc_pm.c
+++ b/drivers/crypto/ccree/cc_pm.c
@@ -100,20 +100,19 @@ int cc_pm_put_suspend(struct device *dev)
int cc_pm_init(struct cc_drvdata *drvdata)
{
- int rc = 0;
struct device *dev = drvdata_to_dev(drvdata);
/* must be before the enabling to avoid resdundent suspending */
pm_runtime_set_autosuspend_delay(dev, CC_SUSPEND_TIMEOUT);
pm_runtime_use_autosuspend(dev);
/* activate the PM module */
- rc = pm_runtime_set_active(dev);
- if (rc)
- return rc;
- /* enable the PM module*/
- pm_runtime_enable(dev);
+ return pm_runtime_set_active(dev);
+}
- return rc;
+/* enable the PM module*/
+void cc_pm_go(struct cc_drvdata *drvdata)
+{
+ pm_runtime_enable(drvdata_to_dev(drvdata));
}
void cc_pm_fini(struct cc_drvdata *drvdata)
diff --git a/drivers/crypto/ccree/cc_pm.h b/drivers/crypto/ccree/cc_pm.h
index 020a5403c58b..f62624357020 100644
--- a/drivers/crypto/ccree/cc_pm.h
+++ b/drivers/crypto/ccree/cc_pm.h
@@ -16,6 +16,7 @@
extern const struct dev_pm_ops ccree_pm;
int cc_pm_init(struct cc_drvdata *drvdata);
+void cc_pm_go(struct cc_drvdata *drvdata);
void cc_pm_fini(struct cc_drvdata *drvdata);
int cc_pm_suspend(struct device *dev);
int cc_pm_resume(struct device *dev);
@@ -29,6 +30,8 @@ static inline int cc_pm_init(struct cc_drvdata *drvdata)
return 0;
}
+static void cc_pm_go(struct cc_drvdata *drvdata) {}
+
static inline void cc_pm_fini(struct cc_drvdata *drvdata) {}
static inline int cc_pm_suspend(struct device *dev)
--
2.20.1
Re: Regression due to "PM-runtime: Switch autosuspend over to using hrtimers"
On Thu, Feb 7, 2019 at 10:25 AM Gilad Ben-Yossef wrote:
> >
> > On Wed, 6 Feb 2019 at 17:40, Gilad Ben-Yossef wrote:
> > >
> > > Hi all,
> > >
> > > A regression was spotted in the ccree driver running on Arm 32 bit
> > > causing a kernel panic during the crypto API self test phase (panic
> > > messages included with this message) happening in the PM resume
> > > callback that was not happening before.
> > >
> > > I've bisected the change that caused this to commit 8234f6734c5d
> > > ("PM-runtime: Switch autosuspend over to using hrtimers").
> > >
> > > I'm still trying to figure out what is going on inside the callback,
> > > but as it was not happening before, I thought I'd give you a shout out
> > > to make you aware of this.
> >
> > Are you using autosuspend mode for this device ?
> Yes.
>
>
> > Also this happen in a platform specific function cc_init_hash_sram().
> > I can't see anything related to pm runtime and autosuspend in it.
>
> True. However, the function is called from the driver PM resume
> callback and before that commit it did not fail.
> My guess is that there is something related to the timing the callback
> is called, probably some race condition the change exposed.
>
OK, I've found it. It was indeed a race condition in the ccree driver.
We were doing something in the resume callback that relied on
initialization sequence that happens after autosuspend was enabled for
the device.
It was never a problem because with the lower res timers we always got
around to that initialization before auto suspend kicked in and we had
to resume but with your change we started losing that race :-)
Sorry for the noise and thanks for your help!
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
Re: Regression due to "PM-runtime: Switch autosuspend over to using hrtimers"
Hi,
Thank for the quick response.
On Wed, Feb 6, 2019 at 6:59 PM Vincent Guittot
wrote:
>
> Hi Gilad,
>
> On Wed, 6 Feb 2019 at 17:40, Gilad Ben-Yossef wrote:
> >
> > Hi all,
> >
> > A regression was spotted in the ccree driver running on Arm 32 bit
> > causing a kernel panic during the crypto API self test phase (panic
> > messages included with this message) happening in the PM resume
> > callback that was not happening before.
> >
> > I've bisected the change that caused this to commit 8234f6734c5d
> > ("PM-runtime: Switch autosuspend over to using hrtimers").
> >
> > I'm still trying to figure out what is going on inside the callback,
> > but as it was not happening before, I thought I'd give you a shout out
> > to make you aware of this.
>
> Are you using autosuspend mode for this device ?
Yes.
> Also this happen in a platform specific function cc_init_hash_sram().
> I can't see anything related to pm runtime and autosuspend in it.
True. However, the function is called from the driver PM resume
callback and before that commit it did not fail.
My guess is that there is something related to the timing the callback
is called, probably some race condition the change exposed.
There is probably nothing for you do until I figure out what is
failing in the internal function.
I just wanted to give the heads up in case someone else runs into a
similar issue with another driver.
> Do you have more details about where this panic happen in the function ?
I'm looking into this right now and will update.
Thanks!
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
Regression due to "PM-runtime: Switch autosuspend over to using hrtimers"
8 c0947d34 c093fc94 [ 11.157278] bce0: 000c000c ee54bcf0 ee54bcf0 ee513000 [ 11.165464] bd00: ee514000 ee515000 ee516000 ee517000 ee518000 ee519000 ee51a000 00013c00 [ 11.173649] bd20: 0002 ef009e80 0400 c3802d00 c3803c48 efdc2262 [ 11.181834] bd40: 0200 2e513000 0400 0071 0001 0001 [ 11.190019] bd60: ef03f380 ef03f2c0 0001 004f 0800 [ 11.198204] bd80: 0027 004f 004f 004f 004f 0400 0071 0001 [ 11.206389] bda0: 0001 [ 11.214575] bdc0: 0400 017c 0040 0002eae8 0ac0 c3806e04 [ 11.222761] bde0: c38568c0 c01155c4 c011947c ef03f2c0 ef7e5cc0 0ac0 efdcdd00 0040 [ 11.230946] be00: 0001 0040 ef14a600 eebd0080 eeae8f9c c0115610 c011947c c37dccc0 [ 11.239131] be20: eebd0080 efdcdd00 ef14a610 0ac0 ef14a610 c011571c ef0ce1c0 00ff [ 11.247316] be40: c081c544 ef0ce1c0 ef0ce180 00041400 [ 11.255501] be60: eebd0080 ef0ce180 0001 c0833e60 0005 c3803c48 1085 ef069cbc [ 11.263686] be80: eeae8f9c c032cd24 ef0ce180 c081daf4 00ae eebd1e00 [ 11.271872] bea0: c3803c48 c032cdec c032cdb4 00ae ee54a000 c3803c48 c032ddf8 [ 11.280057] bec0: ee54bf00 eebd1e00 0400 eebd1e80 ef7e6380 c3848d14 0001 [ 11.288242] bee0: 0001 0002 c3803c48 c37dccc0 ef086000 c013bcd8 [ 11.296428] bf00: c3846ee4 0002 ef086000 ef7e5cc0 ef247300 ef0832c0 c3803c48 eea8fc40 [ 11.304612] bf20: c080429c ee54bf74 c0797e74 c0152e80 [ 11.312798] bf40: ef069cc0 ef069cbc 00041400 e000 00041400 eeae8480 eebd1e00 [ 11.320983] bf60: eeae8480 ee54a000 eebd1e00 c0328d04 ef069cbc eeae8f9c c0328d44 [ 11.329168] bf80: eeae8f80 c013aac8 eeae8480 c013a980 [ 11.337353] bfa0: c01010e8 [ 11.345538] bfc0: [ 11.353723] bfe0: 0013 [ 11.361923] [] (cc_init_hash_sram) from [] (cc_pm_resume+0x78/0xf0) [ 11.369954] [] (cc_pm_resume) from [] (__rpm_callback+0xc0/0x1cc) [ 11.377800] [] (__rpm_callback) from [] (rpm_callback+0x20/0x80) [ 11.385557] [] (rpm_callback) from [] (rpm_resume+0x398/0x66c) [ 11.393144] [] (rpm_resume) from [] (__pm_runtime_resume+0x4c/0x64) [ 11.401164] [] (__pm_runtime_resume) from [] (cc_send_request+0x64/0x22c) [ 11.409703] [] (cc_send_request) from [] (cc_cipher_process+0x3b0/0x96c) [ 11.418153] [] (cc_cipher_process) from [] (__test_skcipher+0x21c/0xb30) [ 11.426605] [] (__test_skcipher) from [] (test_skcipher+0x28/0xb8) [ 11.434535] [] (test_skcipher) from [] (alg_test_skcipher+0x38/0x8c) [ 11.442635] [] (alg_test_skcipher) from [] (alg_test.part.5+0x11c/0x284) [ 11.451081] [] (alg_test.part.5) from [] (cryptomgr_test+0x40/0x48) [ 11.459100] [] (cryptomgr_test) from [] (kthread+0x148/0x150) [ 11.466594] [] (kthread) from [] (ret_from_fork+0x14/0x2c) [ 11.473823] Exception stack(0xee54bfb0 to 0xee54bff8) [ 11.478872] bfa0: [ 11.487057] bfc0: [ 11.495241] bfe0: 0013 [ 11.501863] Code: e24ddf6b e1a05000 e5941000 e28d6f6a (e1ca80d0) [ 11.508053] ---[ end trace 19720bed5fe13807 ]--- -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH 2/7] crypto: ccrree: no need to check return value of debugfs_create functions
On Wed, Jan 23, 2019 at 3:37 PM Greg Kroah-Hartman wrote: > > On Wed, Jan 23, 2019 at 02:58:22PM +0200, Gilad Ben-Yossef wrote: > > Hi, > > > > On Tue, Jan 22, 2019 at 5:14 PM Greg Kroah-Hartman > > wrote: > > > > > > When calling debugfs functions, there is no need to ever check the > > > return value. The function can work or not, but the code logic should > > > never do something different based on this. > > > > > > > > I get the part about not failing loading the driver just because some > > debugs entry isn't available, but wont it be weird if > > debugfs_create_dir() fails but debugfs_create_regset32() succeeds and > > we suddenly have weird files in the debugfs root dir? > > Not the end of the world of course but maybe it's better to avoid > > trying to create the files if the directory is not available? > > See this patch to handle that theoretical issue: > > > https://lore.kernel.org/lkml/20190123130917.gz4...@dhcp22.suse.cz/T/#me91cc3d16185be13d64f85c8477c543cbda9baf6 > Ah, sorry. I've missed that. Acked-By: Gilad Ben-Yossef Thanks! Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH 2/7] crypto: ccrree: no need to check return value of debugfs_create functions
Hi,
On Tue, Jan 22, 2019 at 5:14 PM Greg Kroah-Hartman
wrote:
>
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
I get the part about not failing loading the driver just because some
debugs entry isn't available, but wont it be weird if
debugfs_create_dir() fails but debugfs_create_regset32() succeeds and
we suddenly have weird files in the debugfs root dir?
Not the end of the world of course but maybe it's better to avoid
trying to create the files if the directory is not available?
Thanks,
Gilad
>
> Cc: Yael Chemla
> Cc: Gilad Ben-Yossef
> Cc: Herbert Xu
> Cc: "David S. Miller"
> Cc: linux-cry...@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman
> ---
> drivers/crypto/ccree/cc_debugfs.c | 22 +++---
> drivers/crypto/ccree/cc_debugfs.h | 8 ++--
> drivers/crypto/ccree/cc_driver.c | 7 +--
> 3 files changed, 6 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/crypto/ccree/cc_debugfs.c
> b/drivers/crypto/ccree/cc_debugfs.c
> index 5ca184e42483..5fa05a7bcf36 100644
> --- a/drivers/crypto/ccree/cc_debugfs.c
> +++ b/drivers/crypto/ccree/cc_debugfs.c
> @@ -39,11 +39,9 @@ static struct debugfs_reg32 debug_regs[] = {
> CC_DEBUG_REG(AXIM_MON_COMP),
> };
>
> -int __init cc_debugfs_global_init(void)
> +void __init cc_debugfs_global_init(void)
> {
> cc_debugfs_dir = debugfs_create_dir("ccree", NULL);
> -
> - return !cc_debugfs_dir;
> }
>
> void __exit cc_debugfs_global_fini(void)
> @@ -56,7 +54,6 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
> struct device *dev = drvdata_to_dev(drvdata);
> struct cc_debugfs_ctx *ctx;
> struct debugfs_regset32 *regset;
> - struct dentry *file;
>
> debug_regs[0].offset = drvdata->sig_offset;
> debug_regs[1].offset = drvdata->ver_offset;
> @@ -74,22 +71,9 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
> regset->base = drvdata->cc_base;
>
> ctx->dir = debugfs_create_dir(drvdata->plat_dev->name,
> cc_debugfs_dir);
> - if (!ctx->dir)
> - return -ENFILE;
> -
> - file = debugfs_create_regset32("regs", 0400, ctx->dir, regset);
> - if (!file) {
> - debugfs_remove(ctx->dir);
> - return -ENFILE;
> - }
>
> - file = debugfs_create_bool("coherent", 0400, ctx->dir,
> - >coherent);
> -
> - if (!file) {
> - debugfs_remove_recursive(ctx->dir);
> - return -ENFILE;
> - }
> + debugfs_create_regset32("regs", 0400, ctx->dir, regset);
> + debugfs_create_bool("coherent", 0400, ctx->dir, >coherent);
>
> drvdata->debugfs = ctx;
>
> diff --git a/drivers/crypto/ccree/cc_debugfs.h
> b/drivers/crypto/ccree/cc_debugfs.h
> index 5b5320eca7d2..01cbd9a95659 100644
> --- a/drivers/crypto/ccree/cc_debugfs.h
> +++ b/drivers/crypto/ccree/cc_debugfs.h
> @@ -5,7 +5,7 @@
> #define __CC_DEBUGFS_H__
>
> #ifdef CONFIG_DEBUG_FS
> -int cc_debugfs_global_init(void);
> +void cc_debugfs_global_init(void);
> void cc_debugfs_global_fini(void);
>
> int cc_debugfs_init(struct cc_drvdata *drvdata);
> @@ -13,11 +13,7 @@ void cc_debugfs_fini(struct cc_drvdata *drvdata);
>
> #else
>
> -static inline int cc_debugfs_global_init(void)
> -{
> - return 0;
> -}
> -
> +static inline void cc_debugfs_global_init(void) {}
> static inline void cc_debugfs_global_fini(void) {}
>
> static inline int cc_debugfs_init(struct cc_drvdata *drvdata)
> diff --git a/drivers/crypto/ccree/cc_driver.c
> b/drivers/crypto/ccree/cc_driver.c
> index 8ada308d72ee..662738e53ced 100644
> --- a/drivers/crypto/ccree/cc_driver.c
> +++ b/drivers/crypto/ccree/cc_driver.c
> @@ -538,13 +538,8 @@ static struct platform_driver ccree_driver = {
>
> static int __init ccree_init(void)
> {
> - int ret;
> -
> cc_hash_global_init();
> -
> - ret = cc_debugfs_global_init();
> - if (ret)
> - return ret;
> + cc_debugfs_global_init();
>
> return platform_driver_register(_driver);
> }
> --
> 2.20.1
>
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
[PATCH 0/7] crypto: ccree: bug fixes and cleanups
Assorted bug fixes and cleanups Gilad Ben-Yossef (3): crypto: ccree: unmap buffer before copying IV crypto: ccree: shared irq lines are not a bug crypto: ccree: don't copy zero size ciphertext Hadar Gat (4): crypto: ccree: improve error handling crypto: ccree: add error message crypto: ccree: fix free of unallocated mlli buffer crypto: ccree: remove legacy leftover drivers/crypto/ccree/cc_buffer_mgr.c | 87 ++-- drivers/crypto/ccree/cc_cipher.c | 6 +- drivers/crypto/ccree/cc_driver.c | 6 +- drivers/crypto/ccree/cc_driver.h | 2 - 4 files changed, 50 insertions(+), 51 deletions(-) -- 2.20.1
[PATCH 2/7] crypto: ccree: add error message
From: Hadar Gat
Add error message in case of too many mlli entries.
Signed-off-by: Hadar Gat
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_buffer_mgr.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_buffer_mgr.c
b/drivers/crypto/ccree/cc_buffer_mgr.c
index 32d2df36ced2..237a87a57830 100644
--- a/drivers/crypto/ccree/cc_buffer_mgr.c
+++ b/drivers/crypto/ccree/cc_buffer_mgr.c
@@ -156,8 +156,11 @@ static int cc_render_buff_to_mlli(struct device *dev,
dma_addr_t buff_dma,
/* Verify there is no memory overflow*/
new_nents = (*curr_nents + buff_size / CC_MAX_MLLI_ENTRY_SIZE + 1);
- if (new_nents > MAX_NUM_OF_TOTAL_MLLI_ENTRIES)
+ if (new_nents > MAX_NUM_OF_TOTAL_MLLI_ENTRIES) {
+ dev_err(dev, "Too many mlli entries. current %d max %d\n",
+ new_nents, MAX_NUM_OF_TOTAL_MLLI_ENTRIES);
return -ENOMEM;
+ }
/*handle buffer longer than 64 kbytes */
while (buff_size > CC_MAX_MLLI_ENTRY_SIZE) {
--
2.20.1
[PATCH 5/7] crypto: ccree: unmap buffer before copying IV
We were copying the last ciphertext block into the IV field
for CBC before removing the DMA mapping of the output buffer
with the result of the buffer sometime being out-of-sync cache
wise and were getting intermittent cases of bad output IV.
Fix it by moving the DMA buffer unmapping before the copy.
Signed-off-by: Gilad Ben-Yossef
Fixes: 00904aa0cd59 ("crypto: ccree - fix iv handling")
Cc:
---
drivers/crypto/ccree/cc_cipher.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index cc92b031fad1..98ea53524250 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -652,6 +652,8 @@ static void cc_cipher_complete(struct device *dev, void
*cc_req, int err)
unsigned int ivsize = crypto_skcipher_ivsize(sk_tfm);
unsigned int len;
+ cc_unmap_cipher_request(dev, req_ctx, ivsize, src, dst);
+
switch (ctx_p->cipher_mode) {
case DRV_CIPHER_CBC:
/*
@@ -681,7 +683,6 @@ static void cc_cipher_complete(struct device *dev, void
*cc_req, int err)
break;
}
- cc_unmap_cipher_request(dev, req_ctx, ivsize, src, dst);
kzfree(req_ctx->iv);
skcipher_request_complete(req, err);
--
2.20.1
[PATCH 3/7] crypto: ccree: fix free of unallocated mlli buffer
From: Hadar Gat
In cc_unmap_aead_request(), call dma_pool_free() for mlli buffer only
if an item is allocated from the pool and not always if there is a
pool allocated.
This fixes a kernel panic when trying to free a non-allocated item.
Cc: sta...@vger.kernel.org
Signed-off-by: Hadar Gat
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_buffer_mgr.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/ccree/cc_buffer_mgr.c
b/drivers/crypto/ccree/cc_buffer_mgr.c
index 237a87a57830..0ee1c52da0a4 100644
--- a/drivers/crypto/ccree/cc_buffer_mgr.c
+++ b/drivers/crypto/ccree/cc_buffer_mgr.c
@@ -614,10 +614,10 @@ void cc_unmap_aead_request(struct device *dev, struct
aead_request *req)
hw_iv_size, DMA_BIDIRECTIONAL);
}
- /*In case a pool was set, a table was
-*allocated and should be released
-*/
- if (areq_ctx->mlli_params.curr_pool) {
+ /* Release pool */
+ if ((areq_ctx->assoc_buff_type == CC_DMA_BUF_MLLI ||
+areq_ctx->data_buff_type == CC_DMA_BUF_MLLI) &&
+ (areq_ctx->mlli_params.mlli_virt_addr)) {
dev_dbg(dev, "free MLLI buffer: dma=%pad virt=%pK\n",
_ctx->mlli_params.mlli_dma_addr,
areq_ctx->mlli_params.mlli_virt_addr);
--
2.20.1
[PATCH 4/7] crypto: ccree: remove legacy leftover
From: Hadar Gat
Remove legacy code no longer in use.
Signed-off-by: Hadar Gat
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_driver.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/crypto/ccree/cc_driver.h b/drivers/crypto/ccree/cc_driver.h
index 5be7fd431b05..33dbf3e6d15d 100644
--- a/drivers/crypto/ccree/cc_driver.h
+++ b/drivers/crypto/ccree/cc_driver.h
@@ -111,13 +111,11 @@ struct cc_crypto_req {
* @cc_base: virt address of the CC registers
* @irq: device IRQ number
* @irq_mask: Interrupt mask shadow (1 for masked interrupts)
- * @fw_ver:SeP loaded firmware version
*/
struct cc_drvdata {
void __iomem *cc_base;
int irq;
u32 irq_mask;
- u32 fw_ver;
struct completion hw_queue_avail; /* wait for HW queue availability */
struct platform_device *plat_dev;
cc_sram_addr_t mlli_sram_addr;
--
2.20.1
[PATCH 7/7] crypto: ccree: don't copy zero size ciphertext
For decryption in CBC mode we need to save the last ciphertext block
for use as the next IV. However, we were trying to do this also with
zero sized ciphertext resulting in a panic.
Fix this by only doing the copy if the ciphertext length is at least
of IV size.
Signed-off-by: Gilad Ben-Yossef
Cc: sta...@vger.kernel.org
---
drivers/crypto/ccree/cc_cipher.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_cipher.c b/drivers/crypto/ccree/cc_cipher.c
index 98ea53524250..e202d7c7ea00 100644
--- a/drivers/crypto/ccree/cc_cipher.c
+++ b/drivers/crypto/ccree/cc_cipher.c
@@ -800,7 +800,8 @@ static int cc_cipher_decrypt(struct skcipher_request *req)
memset(req_ctx, 0, sizeof(*req_ctx));
- if (ctx_p->cipher_mode == DRV_CIPHER_CBC) {
+ if ((ctx_p->cipher_mode == DRV_CIPHER_CBC) &&
+ (req->cryptlen >= ivsize)) {
/* Allocate and save the last IV sized bytes of the source,
* which will be lost in case of in-place decryption.
--
2.20.1
[PATCH 6/7] crypto: ccree: shared irq lines are not a bug
The ccree driver was logging an error if it got an interrupt but
HW indicated nothing to do as might happen if sharing an irq line.
Remove the error as this is normal and we already have a debug
print for the IRR register value.
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 8ada308d72ee..e75fbe7a8f84 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -103,10 +103,10 @@ static irqreturn_t cc_isr(int irq, void *dev_id)
/* read the interrupt status */
irr = cc_ioread(drvdata, CC_REG(HOST_IRR));
dev_dbg(dev, "Got IRR=0x%08X\n", irr);
- if (irr == 0) { /* Probably shared interrupt line */
- dev_err(dev, "Got interrupt with empty IRR\n");
+
+ if (irr == 0) /* Probably shared interrupt line */
return IRQ_NONE;
- }
+
imr = cc_ioread(drvdata, CC_REG(HOST_IMR));
/* clear interrupt - must be before processing events */
--
2.20.1
[PATCH 0/7] bug fixes and cleanups
Assorted bug fixes and cleanups Gilad Ben-Yossef (3): crypto: ccree: unmap buffer before copying IV crypto: ccree: shared irq lines are not a bug crypto: ccree: don't copy zero size ciphertext Hadar Gat (4): crypto: ccree: improve error handling crypto: ccree: add error message crypto: ccree: fix free of unallocated mlli buffer crypto: ccree: remove legacy leftover drivers/crypto/ccree/cc_buffer_mgr.c | 87 ++-- drivers/crypto/ccree/cc_cipher.c | 6 +- drivers/crypto/ccree/cc_driver.c | 6 +- drivers/crypto/ccree/cc_driver.h | 2 - 4 files changed, 50 insertions(+), 51 deletions(-) -- 2.20.1
[PATCH 1/7] crypto: ccree: improve error handling
From: Hadar Gat
pass the returned error code to the higher level functions
Signed-off-by: Hadar Gat
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_buffer_mgr.c | 74 +---
1 file changed, 35 insertions(+), 39 deletions(-)
diff --git a/drivers/crypto/ccree/cc_buffer_mgr.c
b/drivers/crypto/ccree/cc_buffer_mgr.c
index dd948e1df9e5..32d2df36ced2 100644
--- a/drivers/crypto/ccree/cc_buffer_mgr.c
+++ b/drivers/crypto/ccree/cc_buffer_mgr.c
@@ -511,10 +511,8 @@ int cc_map_cipher_request(struct cc_drvdata *drvdata, void
*ctx,
/* Map the src SGL */
rc = cc_map_sg(dev, src, nbytes, DMA_BIDIRECTIONAL, _ctx->in_nents,
LLI_MAX_NUM_OF_DATA_ENTRIES, , _nents);
- if (rc) {
- rc = -ENOMEM;
+ if (rc)
goto cipher_exit;
- }
if (mapped_nents > 1)
req_ctx->dma_buf_type = CC_DMA_BUF_MLLI;
@@ -528,12 +526,11 @@ int cc_map_cipher_request(struct cc_drvdata *drvdata,
void *ctx,
}
} else {
/* Map the dst sg */
- if (cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
- _ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
- , _nents)) {
- rc = -ENOMEM;
+ rc = cc_map_sg(dev, dst, nbytes, DMA_BIDIRECTIONAL,
+ _ctx->out_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
+ , _nents);
+ if (rc)
goto cipher_exit;
- }
if (mapped_nents > 1)
req_ctx->dma_buf_type = CC_DMA_BUF_MLLI;
@@ -1078,10 +1075,8 @@ static int cc_aead_chain_data(struct cc_drvdata *drvdata,
_ctx->dst.nents,
LLI_MAX_NUM_OF_DATA_ENTRIES, _last_bytes,
_mapped_nents);
- if (rc) {
- rc = -ENOMEM;
+ if (rc)
goto chain_data_exit;
- }
}
dst_mapped_nents = cc_get_sgl_nents(dev, req->dst, size_for_map,
@@ -1235,11 +1230,10 @@ int cc_map_aead_request(struct cc_drvdata *drvdata,
struct aead_request *req)
}
areq_ctx->ccm_iv0_dma_addr = dma_addr;
- if (cc_set_aead_conf_buf(dev, areq_ctx, areq_ctx->ccm_config,
-_data, req->assoclen)) {
- rc = -ENOMEM;
+ rc = cc_set_aead_conf_buf(dev, areq_ctx, areq_ctx->ccm_config,
+ _data, req->assoclen);
+ if (rc)
goto aead_map_failure;
- }
}
if (areq_ctx->cipher_mode == DRV_CIPHER_GCTR) {
@@ -1299,10 +1293,8 @@ int cc_map_aead_request(struct cc_drvdata *drvdata,
struct aead_request *req)
(LLI_MAX_NUM_OF_ASSOC_DATA_ENTRIES +
LLI_MAX_NUM_OF_DATA_ENTRIES),
, _nents);
- if (rc) {
- rc = -ENOMEM;
+ if (rc)
goto aead_map_failure;
- }
if (areq_ctx->is_single_pass) {
/*
@@ -1386,6 +1378,7 @@ int cc_map_hash_request_final(struct cc_drvdata *drvdata,
void *ctx,
struct mlli_params *mlli_params = _ctx->mlli_params;
struct buffer_array sg_data;
struct buff_mgr_handle *buff_mgr = drvdata->buff_mgr_handle;
+ int rc = 0;
u32 dummy = 0;
u32 mapped_nents = 0;
@@ -1405,18 +1398,18 @@ int cc_map_hash_request_final(struct cc_drvdata
*drvdata, void *ctx,
/*TODO: copy data in case that buffer is enough for operation */
/* map the previous buffer */
if (*curr_buff_cnt) {
- if (cc_set_hash_buf(dev, areq_ctx, curr_buff, *curr_buff_cnt,
- _data)) {
- return -ENOMEM;
- }
+ rc = cc_set_hash_buf(dev, areq_ctx, curr_buff, *curr_buff_cnt,
+_data);
+ if (rc)
+ return rc;
}
if (src && nbytes > 0 && do_update) {
- if (cc_map_sg(dev, src, nbytes, DMA_TO_DEVICE,
- _ctx->in_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
- , _nents)) {
+ rc = cc_map_sg(dev, src, nbytes, DMA_TO_DEVICE,
+ _ctx->in_nents, LLI_MAX_NUM_OF_DATA_ENTRIES,
+ , _nents);
+ if (rc)
goto unmap_curr_buff;
- }
if (src && mapped_nents == 1 &&
areq_ctx->data_dma_buf_type == CC_DMA_BUF_NULL) {
memcpy(areq_ctx->buff_sg, src,
@@ -1435,7 +1
Re: [PATCH] crypto: ccree - avoid implicit enum conversion
Hi Nick, On Thu, Oct 11, 2018 at 2:49 AM Nick Desaulniers wrote: > > $ grep -r set_cipher_config0 -n > > shows a healthy mix of DESC_DIRECTION_ENCRYPT_ENCRYPT, > HASH_DIGEST_RESULT_LITTLE_ENDIAN, and DRV_CRYPTO_DIRECTION_ENCRYPT > (all enumeration values of different enums). > > > > > drivers/crypto/ccree/cc_aead.c:1643:30: warning: implicit conversion > > from enumeration type 'enum drv_hash_hw_mode' to different enumeration > > type 'enum drv_cipher_mode' [-Wenum-conversion] > > set_cipher_mode([idx], DRV_HASH_HW_GHASH); > > Looks like a lot fewer call sites using DRV_HASH_HW_GHASH; same value > as DRV_CIPHER_OFB (different enum), but can a block cipher mode and a > hash be valid values for pdesc->word[4]? > > > > > Since this fundamentally isn't a problem because these values just > > represent simple integers for a shift operation, make it clear to Clang > > that this is okay by making the mode parameter in both functions an int. > > This also looks like the simplest fix to me, assuming the values of > all of these enums are valid for whatever pdesc->word[4] is. Can the > maintainers explain what is CC7x-DESC what the comment makes reference > to? They indeed are. CC7x-DESC is a shorthand for "CryptoCell 7xx Descriptor" used in the hardware documents. These descriptors are comprised of fields with context sensitive meaning, hence this "overloading". Given this overloading of meaning an "int" type is indeed a better suited type here. Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH] crypto: ccree - avoid implicit enum conversion
Hi Nick, On Thu, Oct 11, 2018 at 2:49 AM Nick Desaulniers wrote: > > $ grep -r set_cipher_config0 -n > > shows a healthy mix of DESC_DIRECTION_ENCRYPT_ENCRYPT, > HASH_DIGEST_RESULT_LITTLE_ENDIAN, and DRV_CRYPTO_DIRECTION_ENCRYPT > (all enumeration values of different enums). > > > > > drivers/crypto/ccree/cc_aead.c:1643:30: warning: implicit conversion > > from enumeration type 'enum drv_hash_hw_mode' to different enumeration > > type 'enum drv_cipher_mode' [-Wenum-conversion] > > set_cipher_mode([idx], DRV_HASH_HW_GHASH); > > Looks like a lot fewer call sites using DRV_HASH_HW_GHASH; same value > as DRV_CIPHER_OFB (different enum), but can a block cipher mode and a > hash be valid values for pdesc->word[4]? > > > > > Since this fundamentally isn't a problem because these values just > > represent simple integers for a shift operation, make it clear to Clang > > that this is okay by making the mode parameter in both functions an int. > > This also looks like the simplest fix to me, assuming the values of > all of these enums are valid for whatever pdesc->word[4] is. Can the > maintainers explain what is CC7x-DESC what the comment makes reference > to? They indeed are. CC7x-DESC is a shorthand for "CryptoCell 7xx Descriptor" used in the hardware documents. These descriptors are comprised of fields with context sensitive meaning, hence this "overloading". Given this overloading of meaning an "int" type is indeed a better suited type here. Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom!
Re: [PATCH] crypto: ccree - avoid implicit enum conversion
Hi Nathan,
On Thu, Oct 11, 2018 at 12:44 AM Nathan Chancellor
wrote:
>
> Clang warns when one enumerated type is implicitly converted to another
> and this happens in several locations in this driver, ultimately related
> to the set_cipher_{mode,config0} functions. set_cipher_mode expects a mode
> of type drv_cipher_mode and set_cipher_config0 expects a mode of type
> drv_crypto_direction.
>
> drivers/crypto/ccree/cc_ivgen.c:58:35: warning: implicit conversion from
> enumeration type 'enum cc_desc_direction' to different enumeration type
> 'enum drv_crypto_direction' [-Wenum-conversion]
> set_cipher_config0(_seq[idx], DESC_DIRECTION_ENCRYPT_ENCRYPT);
>
> drivers/crypto/ccree/cc_hash.c:99:28: warning: implicit conversion from
> enumeration type 'enum cc_hash_conf_pad' to different enumeration type
> 'enum drv_crypto_direction' [-Wenum-conversion]
> set_cipher_config0(desc, HASH_DIGEST_RESULT_LITTLE_ENDIAN);
>
> drivers/crypto/ccree/cc_aead.c:1643:30: warning: implicit conversion
> from enumeration type 'enum drv_hash_hw_mode' to different enumeration
> type 'enum drv_cipher_mode' [-Wenum-conversion]
> set_cipher_mode([idx], DRV_HASH_HW_GHASH);
>
> Since this fundamentally isn't a problem because these values just
> represent simple integers for a shift operation, make it clear to Clang
> that this is okay by making the mode parameter in both functions an int.
Thank you. We actually had a patch fixing this in the pipeline but
your fix is simpler. :-)
Acked-by: Gilad Ben-Yossef
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
Re: [PATCH] crypto: ccree - avoid implicit enum conversion
Hi Nathan,
On Thu, Oct 11, 2018 at 12:44 AM Nathan Chancellor
wrote:
>
> Clang warns when one enumerated type is implicitly converted to another
> and this happens in several locations in this driver, ultimately related
> to the set_cipher_{mode,config0} functions. set_cipher_mode expects a mode
> of type drv_cipher_mode and set_cipher_config0 expects a mode of type
> drv_crypto_direction.
>
> drivers/crypto/ccree/cc_ivgen.c:58:35: warning: implicit conversion from
> enumeration type 'enum cc_desc_direction' to different enumeration type
> 'enum drv_crypto_direction' [-Wenum-conversion]
> set_cipher_config0(_seq[idx], DESC_DIRECTION_ENCRYPT_ENCRYPT);
>
> drivers/crypto/ccree/cc_hash.c:99:28: warning: implicit conversion from
> enumeration type 'enum cc_hash_conf_pad' to different enumeration type
> 'enum drv_crypto_direction' [-Wenum-conversion]
> set_cipher_config0(desc, HASH_DIGEST_RESULT_LITTLE_ENDIAN);
>
> drivers/crypto/ccree/cc_aead.c:1643:30: warning: implicit conversion
> from enumeration type 'enum drv_hash_hw_mode' to different enumeration
> type 'enum drv_cipher_mode' [-Wenum-conversion]
> set_cipher_mode([idx], DRV_HASH_HW_GHASH);
>
> Since this fundamentally isn't a problem because these values just
> represent simple integers for a shift operation, make it clear to Clang
> that this is okay by making the mode parameter in both functions an int.
Thank you. We actually had a patch fixing this in the pipeline but
your fix is simpler. :-)
Acked-by: Gilad Ben-Yossef
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
[PATCH v2 5/5] arm64: dts: renesas: r8a7795: add ccree binding
Add bindings for CryptoCell instance in the SoC.
Signed-off-by: Gilad Ben-Yossef <gi...@benyossef.com>
---
arch/arm64/boot/dts/renesas/r8a7795.dtsi | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index d842940..3ac75db 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -528,6 +528,15 @@
status = "disabled";
};
+ arm_cc630p: crypto@e6601000 {
+ compatible = "arm,cryptocell-630p-ree";
+ interrupts = ;
+ reg = <0x0 0xe6601000 0 0x1000>;
+ clocks = < CPG_MOD 229>;
+ resets = < 229>;
+ power-domains = < R8A7795_PD_ALWAYS_ON>;
+ };
+
i2c3: i2c@e66d {
#address-cells = <1>;
#size-cells = <0>;
--
2.7.4
[PATCH v2 5/5] arm64: dts: renesas: r8a7795: add ccree binding
Add bindings for CryptoCell instance in the SoC.
Signed-off-by: Gilad Ben-Yossef
---
arch/arm64/boot/dts/renesas/r8a7795.dtsi | 9 +
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index d842940..3ac75db 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -528,6 +528,15 @@
status = "disabled";
};
+ arm_cc630p: crypto@e6601000 {
+ compatible = "arm,cryptocell-630p-ree";
+ interrupts = ;
+ reg = <0x0 0xe6601000 0 0x1000>;
+ clocks = < CPG_MOD 229>;
+ resets = < 229>;
+ power-domains = < R8A7795_PD_ALWAYS_ON>;
+ };
+
i2c3: i2c@e66d {
#address-cells = <1>;
#size-cells = <0>;
--
2.7.4
[PATCH v2 3/5] crypto: ccree: silence debug prints
The cache parameter register configuration was being too verbose. Use dev_dbg() to only provide the information if needed. Signed-off-by: Gilad Ben-Yossef <gi...@benyossef.com> --- drivers/crypto/ccree/cc_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c index b266657..1e40e76 100644 --- a/drivers/crypto/ccree/cc_driver.c +++ b/drivers/crypto/ccree/cc_driver.c @@ -168,14 +168,14 @@ int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe) val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS)); if (is_probe) - dev_info(dev, "Cache params previous: 0x%08X\n", val); + dev_dbg(dev, "Cache params previous: 0x%08X\n", val); cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), cache_params); val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS)); if (is_probe) - dev_info(dev, "Cache params current: 0x%08X (expect: 0x%08X)\n", -val, cache_params); + dev_dbg(dev, "Cache params current: 0x%08X (expect: 0x%08X)\n", + val, cache_params); return 0; } -- 2.7.4
[PATCH v2 2/5] crypto: ccree: better clock handling
Use managed clock handling, differentiate between no clock (possibly OK)
and clock init failure (never OK) and correctly handle clock detection
being deferred.
Suggested-by: Geert Uytterhoeven <ge...@linux-m68k.org>
Signed-off-by: Gilad Ben-Yossef <gi...@benyossef.com>
---
drivers/crypto/ccree/cc_driver.c | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 6f93ce7..b266657 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -190,6 +190,7 @@ static int init_cc_resources(struct platform_device
*plat_dev)
u64 dma_mask;
const struct cc_hw_data *hw_rev;
const struct of_device_id *dev_id;
+ struct clk *clk;
int rc = 0;
new_drvdata = devm_kzalloc(dev, sizeof(*new_drvdata), GFP_KERNEL);
@@ -219,7 +220,24 @@ static int init_cc_resources(struct platform_device
*plat_dev)
platform_set_drvdata(plat_dev, new_drvdata);
new_drvdata->plat_dev = plat_dev;
- new_drvdata->clk = of_clk_get(np, 0);
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk))
+ switch (PTR_ERR(clk)) {
+ /* Clock is optional so this might be fine */
+ case -ENOENT:
+ break;
+
+ /* Clock not available, let's try again soon */
+ case -EPROBE_DEFER:
+ return -EPROBE_DEFER;
+
+ default:
+ dev_err(dev, "Error getting clock: %ld\n",
+ PTR_ERR(clk));
+ return PTR_ERR(clk);
+ }
+ new_drvdata->clk = clk;
+
new_drvdata->coherent = of_dma_is_coherent(np);
/* Get device resources */
--
2.7.4
[PATCH v2 4/5] clk: renesas: r8a7795: add ccree clock bindings
This patch adds the clock used by the CryptoCell 630p instance in the SoC.
Signed-off-by: Gilad Ben-Yossef <gi...@benyossef.com>
---
This patch depends upon the "clk: renesas: r8a7795: Add CR clock" patch
from Geert Uytterhoeven.
drivers/clk/renesas/r8a7795-cpg-mssr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c
b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index e5b1865..a85dd50 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -133,6 +133,7 @@ static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
DEF_MOD("sys-dmac2", 217, R8A7795_CLK_S0D3),
DEF_MOD("sys-dmac1", 218, R8A7795_CLK_S0D3),
DEF_MOD("sys-dmac0", 219, R8A7795_CLK_S0D3),
+ DEF_MOD("sceg-pub", 229, R8A7795_CLK_CR),
DEF_MOD("cmt3", 300, R8A7795_CLK_R),
DEF_MOD("cmt2", 301, R8A7795_CLK_R),
DEF_MOD("cmt1", 302, R8A7795_CLK_R),
--
2.7.4
[PATCH v2 3/5] crypto: ccree: silence debug prints
The cache parameter register configuration was being too verbose. Use dev_dbg() to only provide the information if needed. Signed-off-by: Gilad Ben-Yossef --- drivers/crypto/ccree/cc_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c index b266657..1e40e76 100644 --- a/drivers/crypto/ccree/cc_driver.c +++ b/drivers/crypto/ccree/cc_driver.c @@ -168,14 +168,14 @@ int init_cc_regs(struct cc_drvdata *drvdata, bool is_probe) val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS)); if (is_probe) - dev_info(dev, "Cache params previous: 0x%08X\n", val); + dev_dbg(dev, "Cache params previous: 0x%08X\n", val); cc_iowrite(drvdata, CC_REG(AXIM_CACHE_PARAMS), cache_params); val = cc_ioread(drvdata, CC_REG(AXIM_CACHE_PARAMS)); if (is_probe) - dev_info(dev, "Cache params current: 0x%08X (expect: 0x%08X)\n", -val, cache_params); + dev_dbg(dev, "Cache params current: 0x%08X (expect: 0x%08X)\n", + val, cache_params); return 0; } -- 2.7.4
[PATCH v2 2/5] crypto: ccree: better clock handling
Use managed clock handling, differentiate between no clock (possibly OK)
and clock init failure (never OK) and correctly handle clock detection
being deferred.
Suggested-by: Geert Uytterhoeven
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_driver.c | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 6f93ce7..b266657 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -190,6 +190,7 @@ static int init_cc_resources(struct platform_device
*plat_dev)
u64 dma_mask;
const struct cc_hw_data *hw_rev;
const struct of_device_id *dev_id;
+ struct clk *clk;
int rc = 0;
new_drvdata = devm_kzalloc(dev, sizeof(*new_drvdata), GFP_KERNEL);
@@ -219,7 +220,24 @@ static int init_cc_resources(struct platform_device
*plat_dev)
platform_set_drvdata(plat_dev, new_drvdata);
new_drvdata->plat_dev = plat_dev;
- new_drvdata->clk = of_clk_get(np, 0);
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk))
+ switch (PTR_ERR(clk)) {
+ /* Clock is optional so this might be fine */
+ case -ENOENT:
+ break;
+
+ /* Clock not available, let's try again soon */
+ case -EPROBE_DEFER:
+ return -EPROBE_DEFER;
+
+ default:
+ dev_err(dev, "Error getting clock: %ld\n",
+ PTR_ERR(clk));
+ return PTR_ERR(clk);
+ }
+ new_drvdata->clk = clk;
+
new_drvdata->coherent = of_dma_is_coherent(np);
/* Get device resources */
--
2.7.4
[PATCH v2 4/5] clk: renesas: r8a7795: add ccree clock bindings
This patch adds the clock used by the CryptoCell 630p instance in the SoC.
Signed-off-by: Gilad Ben-Yossef
---
This patch depends upon the "clk: renesas: r8a7795: Add CR clock" patch
from Geert Uytterhoeven.
drivers/clk/renesas/r8a7795-cpg-mssr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c
b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index e5b1865..a85dd50 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -133,6 +133,7 @@ static struct mssr_mod_clk r8a7795_mod_clks[] __initdata = {
DEF_MOD("sys-dmac2", 217, R8A7795_CLK_S0D3),
DEF_MOD("sys-dmac1", 218, R8A7795_CLK_S0D3),
DEF_MOD("sys-dmac0", 219, R8A7795_CLK_S0D3),
+ DEF_MOD("sceg-pub", 229, R8A7795_CLK_CR),
DEF_MOD("cmt3", 300, R8A7795_CLK_R),
DEF_MOD("cmt2", 301, R8A7795_CLK_R),
DEF_MOD("cmt1", 302, R8A7795_CLK_R),
--
2.7.4
[PATCH v2 0/5] crypto: ccree: cleanup, fixes and R-Car enabling
The patch set enables the use of CryptoCell found in some Renesas R-Car Salvator-X boards and fixes some driver issues uncovered that prevented to work properly. Changes from v1: - Properly fix the bug that caused us to read a bad signature register rather than dropping the check - Proper DT fields as indicated by Geert Uytterhoeven. - Better clock enabling as suggested by Geert Uytterhoeven. Note! the last two patches in the set depend on the "clk: renesas: r8a7795: Add CR clock" patch from Geert Uytterhoeven. Gilad Ben-Yossef (5): crypto: ccree: correct host regs offset crypto: ccree: better clock handling crypto: ccree: silence debug prints clk: renesas: r8a7795: add ccree clock bindings arm64: dts: renesas: r8a7795: add ccree binding arch/arm64/boot/dts/renesas/r8a7795.dtsi | 9 + drivers/clk/renesas/r8a7795-cpg-mssr.c | 1 + drivers/crypto/ccree/cc_debugfs.c| 7 +-- drivers/crypto/ccree/cc_driver.c | 34 ++-- drivers/crypto/ccree/cc_driver.h | 2 ++ drivers/crypto/ccree/cc_host_regs.h | 6 -- 6 files changed, 49 insertions(+), 10 deletions(-) -- 2.7.4
[PATCH v2 1/5] crypto: ccree: correct host regs offset
The product signature and HW revision register have different offset on the
older HW revisions.
This fixes the problem of the driver failing sanity check on silicon
despite working on the FPGA emulation systems.
Fixes: 27b3b22dd98c ("crypto: ccree - add support for older HW revs")
Cc: sta...@vger.kernel.org
Signed-off-by: Gilad Ben-Yossef <gi...@benyossef.com>
---
drivers/crypto/ccree/cc_debugfs.c | 7 +--
drivers/crypto/ccree/cc_driver.c| 8 ++--
drivers/crypto/ccree/cc_driver.h| 2 ++
drivers/crypto/ccree/cc_host_regs.h | 6 --
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/ccree/cc_debugfs.c
b/drivers/crypto/ccree/cc_debugfs.c
index 08f8db4..5ca184e 100644
--- a/drivers/crypto/ccree/cc_debugfs.c
+++ b/drivers/crypto/ccree/cc_debugfs.c
@@ -26,7 +26,8 @@ struct cc_debugfs_ctx {
static struct dentry *cc_debugfs_dir;
static struct debugfs_reg32 debug_regs[] = {
- CC_DEBUG_REG(HOST_SIGNATURE),
+ { .name = "SIGNATURE" }, /* Must be 0th */
+ { .name = "VERSION" }, /* Must be 1st */
CC_DEBUG_REG(HOST_IRR),
CC_DEBUG_REG(HOST_POWER_DOWN_EN),
CC_DEBUG_REG(AXIM_MON_ERR),
@@ -34,7 +35,6 @@ static struct debugfs_reg32 debug_regs[] = {
CC_DEBUG_REG(HOST_IMR),
CC_DEBUG_REG(AXIM_CFG),
CC_DEBUG_REG(AXIM_CACHE_PARAMS),
- CC_DEBUG_REG(HOST_VERSION),
CC_DEBUG_REG(GPR_HOST),
CC_DEBUG_REG(AXIM_MON_COMP),
};
@@ -58,6 +58,9 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
struct debugfs_regset32 *regset;
struct dentry *file;
+ debug_regs[0].offset = drvdata->sig_offset;
+ debug_regs[1].offset = drvdata->ver_offset;
+
ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
if (!ctx)
return -ENOMEM;
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 89ce013..6f93ce7 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -207,9 +207,13 @@ static int init_cc_resources(struct platform_device
*plat_dev)
if (hw_rev->rev >= CC_HW_REV_712) {
new_drvdata->hash_len_sz = HASH_LEN_SIZE_712;
new_drvdata->axim_mon_offset = CC_REG(AXIM_MON_COMP);
+ new_drvdata->sig_offset = CC_REG(HOST_SIGNATURE_712);
+ new_drvdata->ver_offset = CC_REG(HOST_VERSION_712);
} else {
new_drvdata->hash_len_sz = HASH_LEN_SIZE_630;
new_drvdata->axim_mon_offset = CC_REG(AXIM_MON_COMP8);
+ new_drvdata->sig_offset = CC_REG(HOST_SIGNATURE_630);
+ new_drvdata->ver_offset = CC_REG(HOST_VERSION_630);
}
platform_set_drvdata(plat_dev, new_drvdata);
@@ -276,7 +280,7 @@ static int init_cc_resources(struct platform_device
*plat_dev)
}
/* Verify correct mapping */
- signature_val = cc_ioread(new_drvdata, CC_REG(HOST_SIGNATURE));
+ signature_val = cc_ioread(new_drvdata, new_drvdata->sig_offset);
if (signature_val != hw_rev->sig) {
dev_err(dev, "Invalid CC signature: SIGNATURE=0x%08X !=
expected=0x%08X\n",
signature_val, hw_rev->sig);
@@ -287,7 +291,7 @@ static int init_cc_resources(struct platform_device
*plat_dev)
/* Display HW versions */
dev_info(dev, "ARM CryptoCell %s Driver: HW version 0x%08X, Driver
version %s\n",
-hw_rev->name, cc_ioread(new_drvdata, CC_REG(HOST_VERSION)),
+hw_rev->name, cc_ioread(new_drvdata, new_drvdata->ver_offset),
DRV_MODULE_VERSION);
rc = init_cc_regs(new_drvdata, true);
diff --git a/drivers/crypto/ccree/cc_driver.h b/drivers/crypto/ccree/cc_driver.h
index 2048fde..95f82b2 100644
--- a/drivers/crypto/ccree/cc_driver.h
+++ b/drivers/crypto/ccree/cc_driver.h
@@ -129,6 +129,8 @@ struct cc_drvdata {
enum cc_hw_rev hw_rev;
u32 hash_len_sz;
u32 axim_mon_offset;
+ u32 sig_offset;
+ u32 ver_offset;
};
struct cc_crypto_alg {
diff --git a/drivers/crypto/ccree/cc_host_regs.h
b/drivers/crypto/ccree/cc_host_regs.h
index f510018..616b2e1 100644
--- a/drivers/crypto/ccree/cc_host_regs.h
+++ b/drivers/crypto/ccree/cc_host_regs.h
@@ -45,7 +45,8 @@
#define CC_HOST_ICR_DSCRPTR_WATERMARK_QUEUE0_CLEAR_BIT_SIZE0x1UL
#define CC_HOST_ICR_AXIM_COMP_INT_CLEAR_BIT_SHIFT 0x17UL
#define CC_HOST_ICR_AXIM_COMP_INT_CLEAR_BIT_SIZE 0x1UL
-#define CC_HOST_SIGNATURE_REG_OFFSET 0xA24UL
+#define CC_HOST_SIGNATURE_712_REG_OFFSET 0xA24UL
+#define CC_HOST_SIGNATURE_630_REG_OFFSET 0xAC8UL
#define CC_HOST_SIGNATURE_VALUE_BIT_SHIFT 0x0UL
#define CC_HOST_SIGNATURE_VALUE_BIT_SIZE 0x20UL
#define CC_HOST_BOOT_REG_OFFSET0xA28UL
@@ -105,7 +106,8 @@
#define CC_HOST_BOOT_ONLY_ENCRYPT_LOCAL_BIT_SIZE 0x1UL
#defi
[PATCH v2 0/5] crypto: ccree: cleanup, fixes and R-Car enabling
The patch set enables the use of CryptoCell found in some Renesas R-Car Salvator-X boards and fixes some driver issues uncovered that prevented to work properly. Changes from v1: - Properly fix the bug that caused us to read a bad signature register rather than dropping the check - Proper DT fields as indicated by Geert Uytterhoeven. - Better clock enabling as suggested by Geert Uytterhoeven. Note! the last two patches in the set depend on the "clk: renesas: r8a7795: Add CR clock" patch from Geert Uytterhoeven. Gilad Ben-Yossef (5): crypto: ccree: correct host regs offset crypto: ccree: better clock handling crypto: ccree: silence debug prints clk: renesas: r8a7795: add ccree clock bindings arm64: dts: renesas: r8a7795: add ccree binding arch/arm64/boot/dts/renesas/r8a7795.dtsi | 9 + drivers/clk/renesas/r8a7795-cpg-mssr.c | 1 + drivers/crypto/ccree/cc_debugfs.c| 7 +-- drivers/crypto/ccree/cc_driver.c | 34 ++-- drivers/crypto/ccree/cc_driver.h | 2 ++ drivers/crypto/ccree/cc_host_regs.h | 6 -- 6 files changed, 49 insertions(+), 10 deletions(-) -- 2.7.4
[PATCH v2 1/5] crypto: ccree: correct host regs offset
The product signature and HW revision register have different offset on the
older HW revisions.
This fixes the problem of the driver failing sanity check on silicon
despite working on the FPGA emulation systems.
Fixes: 27b3b22dd98c ("crypto: ccree - add support for older HW revs")
Cc: sta...@vger.kernel.org
Signed-off-by: Gilad Ben-Yossef
---
drivers/crypto/ccree/cc_debugfs.c | 7 +--
drivers/crypto/ccree/cc_driver.c| 8 ++--
drivers/crypto/ccree/cc_driver.h| 2 ++
drivers/crypto/ccree/cc_host_regs.h | 6 --
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/ccree/cc_debugfs.c
b/drivers/crypto/ccree/cc_debugfs.c
index 08f8db4..5ca184e 100644
--- a/drivers/crypto/ccree/cc_debugfs.c
+++ b/drivers/crypto/ccree/cc_debugfs.c
@@ -26,7 +26,8 @@ struct cc_debugfs_ctx {
static struct dentry *cc_debugfs_dir;
static struct debugfs_reg32 debug_regs[] = {
- CC_DEBUG_REG(HOST_SIGNATURE),
+ { .name = "SIGNATURE" }, /* Must be 0th */
+ { .name = "VERSION" }, /* Must be 1st */
CC_DEBUG_REG(HOST_IRR),
CC_DEBUG_REG(HOST_POWER_DOWN_EN),
CC_DEBUG_REG(AXIM_MON_ERR),
@@ -34,7 +35,6 @@ static struct debugfs_reg32 debug_regs[] = {
CC_DEBUG_REG(HOST_IMR),
CC_DEBUG_REG(AXIM_CFG),
CC_DEBUG_REG(AXIM_CACHE_PARAMS),
- CC_DEBUG_REG(HOST_VERSION),
CC_DEBUG_REG(GPR_HOST),
CC_DEBUG_REG(AXIM_MON_COMP),
};
@@ -58,6 +58,9 @@ int cc_debugfs_init(struct cc_drvdata *drvdata)
struct debugfs_regset32 *regset;
struct dentry *file;
+ debug_regs[0].offset = drvdata->sig_offset;
+ debug_regs[1].offset = drvdata->ver_offset;
+
ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
if (!ctx)
return -ENOMEM;
diff --git a/drivers/crypto/ccree/cc_driver.c b/drivers/crypto/ccree/cc_driver.c
index 89ce013..6f93ce7 100644
--- a/drivers/crypto/ccree/cc_driver.c
+++ b/drivers/crypto/ccree/cc_driver.c
@@ -207,9 +207,13 @@ static int init_cc_resources(struct platform_device
*plat_dev)
if (hw_rev->rev >= CC_HW_REV_712) {
new_drvdata->hash_len_sz = HASH_LEN_SIZE_712;
new_drvdata->axim_mon_offset = CC_REG(AXIM_MON_COMP);
+ new_drvdata->sig_offset = CC_REG(HOST_SIGNATURE_712);
+ new_drvdata->ver_offset = CC_REG(HOST_VERSION_712);
} else {
new_drvdata->hash_len_sz = HASH_LEN_SIZE_630;
new_drvdata->axim_mon_offset = CC_REG(AXIM_MON_COMP8);
+ new_drvdata->sig_offset = CC_REG(HOST_SIGNATURE_630);
+ new_drvdata->ver_offset = CC_REG(HOST_VERSION_630);
}
platform_set_drvdata(plat_dev, new_drvdata);
@@ -276,7 +280,7 @@ static int init_cc_resources(struct platform_device
*plat_dev)
}
/* Verify correct mapping */
- signature_val = cc_ioread(new_drvdata, CC_REG(HOST_SIGNATURE));
+ signature_val = cc_ioread(new_drvdata, new_drvdata->sig_offset);
if (signature_val != hw_rev->sig) {
dev_err(dev, "Invalid CC signature: SIGNATURE=0x%08X !=
expected=0x%08X\n",
signature_val, hw_rev->sig);
@@ -287,7 +291,7 @@ static int init_cc_resources(struct platform_device
*plat_dev)
/* Display HW versions */
dev_info(dev, "ARM CryptoCell %s Driver: HW version 0x%08X, Driver
version %s\n",
-hw_rev->name, cc_ioread(new_drvdata, CC_REG(HOST_VERSION)),
+hw_rev->name, cc_ioread(new_drvdata, new_drvdata->ver_offset),
DRV_MODULE_VERSION);
rc = init_cc_regs(new_drvdata, true);
diff --git a/drivers/crypto/ccree/cc_driver.h b/drivers/crypto/ccree/cc_driver.h
index 2048fde..95f82b2 100644
--- a/drivers/crypto/ccree/cc_driver.h
+++ b/drivers/crypto/ccree/cc_driver.h
@@ -129,6 +129,8 @@ struct cc_drvdata {
enum cc_hw_rev hw_rev;
u32 hash_len_sz;
u32 axim_mon_offset;
+ u32 sig_offset;
+ u32 ver_offset;
};
struct cc_crypto_alg {
diff --git a/drivers/crypto/ccree/cc_host_regs.h
b/drivers/crypto/ccree/cc_host_regs.h
index f510018..616b2e1 100644
--- a/drivers/crypto/ccree/cc_host_regs.h
+++ b/drivers/crypto/ccree/cc_host_regs.h
@@ -45,7 +45,8 @@
#define CC_HOST_ICR_DSCRPTR_WATERMARK_QUEUE0_CLEAR_BIT_SIZE0x1UL
#define CC_HOST_ICR_AXIM_COMP_INT_CLEAR_BIT_SHIFT 0x17UL
#define CC_HOST_ICR_AXIM_COMP_INT_CLEAR_BIT_SIZE 0x1UL
-#define CC_HOST_SIGNATURE_REG_OFFSET 0xA24UL
+#define CC_HOST_SIGNATURE_712_REG_OFFSET 0xA24UL
+#define CC_HOST_SIGNATURE_630_REG_OFFSET 0xAC8UL
#define CC_HOST_SIGNATURE_VALUE_BIT_SHIFT 0x0UL
#define CC_HOST_SIGNATURE_VALUE_BIT_SIZE 0x20UL
#define CC_HOST_BOOT_REG_OFFSET0xA28UL
@@ -105,7 +106,8 @@
#define CC_HOST_BOOT_ONLY_ENCRYPT_LOCAL_BIT_SIZE 0x1UL
#define CC_HOST_BOOT
Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > Hi Gilad, > > On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef <gi...@benyossef.com> wrote: >> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven >> <ge...@linux-m68k.org> wrote: >>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c >>> does not distinguish between the absence of the clock property, and an >>> actual error in getting the clock, and never considers any error a failure >>> (incl. -PROBE_DEFER). >>> >>> As of_clk_get() returns -ENOENT for both a missing clock property and a >>> missing clock, you should use (devm_)clk_get() instead, and distinguish >>> between NULL (no clock property) and IS_ERR() (actual failure -> abort). >> >> I was trying to do as you suggested but I didn't quite get what is the >> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be. > > It's the (optional) name of the clock, helpful in case there is more than one. > In your case, NULL is fine. > I have assumed as much and tried it, it did not work and so I assumed I am missing something and asked you. It turns out I was missing the fact I was using the wrong device tree file... :-( So thanks, it works now :-) Having said that, while using devm)clk_get() is a better approach, it does not seems to distinguish between no "clocks" and a failure to clock information - it returns ENOENT in both cases as well. Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru
Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
On Tue, May 22, 2018 at 10:48 AM, Geert Uytterhoeven wrote: > Hi Gilad, > > On Mon, May 21, 2018 at 3:43 PM, Gilad Ben-Yossef wrote: >> On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven >> wrote: >>> Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c >>> does not distinguish between the absence of the clock property, and an >>> actual error in getting the clock, and never considers any error a failure >>> (incl. -PROBE_DEFER). >>> >>> As of_clk_get() returns -ENOENT for both a missing clock property and a >>> missing clock, you should use (devm_)clk_get() instead, and distinguish >>> between NULL (no clock property) and IS_ERR() (actual failure -> abort). >> >> I was trying to do as you suggested but I didn't quite get what is the >> dev_id (2nd) parameter to devm_clk_get parameter is supposed to be. > > It's the (optional) name of the clock, helpful in case there is more than one. > In your case, NULL is fine. > I have assumed as much and tried it, it did not work and so I assumed I am missing something and asked you. It turns out I was missing the fact I was using the wrong device tree file... :-( So thanks, it works now :-) Having said that, while using devm)clk_get() is a better approach, it does not seems to distinguish between no "clocks" and a failure to clock information - it returns ENOENT in both cases as well. Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru
Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > > Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c > does not distinguish between the absence of the clock property, and an > actual error in getting the clock, and never considers any error a failure > (incl. -PROBE_DEFER). > > As of_clk_get() returns -ENOENT for both a missing clock property and a > missing clock, you should use (devm_)clk_get() instead, and distinguish > between NULL (no clock property) and IS_ERR() (actual failure -> abort). > I was trying to do as you suggested but I didn't quite get what is the dev_id (2nd) parameter to devm_clk_get parameter is supposed to be. I see what of_clk_get() is doing, so can replicate that but it seems an over kill. Any ideas? Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru
Re: [PATCH 3/3] arm64: dts: renesas: r8a7795: add ccree binding
On Thu, May 17, 2018 at 1:16 PM, Geert Uytterhoeven wrote: > > Indeed. From a quick glance, it looks like drivers/crypto/ccree/cc_driver.c > does not distinguish between the absence of the clock property, and an > actual error in getting the clock, and never considers any error a failure > (incl. -PROBE_DEFER). > > As of_clk_get() returns -ENOENT for both a missing clock property and a > missing clock, you should use (devm_)clk_get() instead, and distinguish > between NULL (no clock property) and IS_ERR() (actual failure -> abort). > I was trying to do as you suggested but I didn't quite get what is the dev_id (2nd) parameter to devm_clk_get parameter is supposed to be. I see what of_clk_get() is doing, so can replicate that but it seems an over kill. Any ideas? Thanks, Gilad -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru
