Re: [Part2 PATCH v6 00/38] x86: Secure Encrypted Virtualization (AMD)

[Brijesh Singh]

Hi Herbert,


On 10/24/2017 07:14 AM, Brijesh Singh wrote:

Hi Herbert and Paolo,








Since the PSP patches touches both the CCP and KVM driver, hence I was
wondering if you guys have any thought on how PSP patches will be
merged? I am talking about Patch 9 to 20 from this series. I have
ensured that patches apply cleanly on both kvm/master and
cryptodev-2.6/master. We can do this in one of two ways:

- Paolo can merge the PSP support through the KVM branch

or

- Herbert can create a topic branch with PSP changes and Paolo can use
that topic branch.

Any visibility will help my next submission. thank you.



Just checking, any thought on this?

Are you okay if the PSP changes are merged through the KVM tree?

-Brijesh

Re: [PATCH 0/2] hwrng: iproc-rng200: Add support for BCM7278

[Scott Branden]

Patch series looks fine.


On 17-11-01 04:20 PM, Florian Fainelli wrote:

Hi,

This patch series adds support for the RNG200 block found on the BCM7278 SoC.
This requires us to update the compatible string (and associated binding
document) as well as the Kconfig option to make that driver selectable with
ARCH_BRCMSTB gating the enabling of such SoCs.

Thank you

Florian Fainelli (2):
   dt-bindings: rng: Document BCM7278 RNG200 compatible
   hwrng: iproc-rng200: Add support for BCM7278

  Documentation/devicetree/bindings/rng/brcm,iproc-rng200.txt | 4 +++-
  drivers/char/hw_random/Kconfig  | 6 +++---
  drivers/char/hw_random/iproc-rng200.c   | 1 +
  3 files changed, 7 insertions(+), 4 deletions(-)


Acked-by: Scott Branden 

Re: [PATCH 00/12] bcm63xx-rng conversion to bcm2835-rng

[Florian Fainelli]

Hi Stefan,

On 11/02/2017 12:01 PM, Stefan Wahren wrote:
> Hi Florian,
> 
>> Florian Fainelli  hat am 2. November 2017 um 02:03 
>> geschrieben:
>>
>>
>> Hi,
>>
>> As it usually happens when there is a fair amount of HW IP block re-use,
>> competing implementations show up. In that case the BCM2835 HWRNG driver and
>> the BCM63xx RNG driver have exactly the same register offsets and this is
>> indeed the same piece of HW.
>>
>> This patch series first prepares the bcm2835-rng to be more future proof and
>> support newer platforms, and the last part brings in what is necessary to
>> migrate the bcm63xx-rng over to bcm2835-rng. Finally we delete bcm63xx-rng
>> completely.
>>
>> The reason why BCM2835 RNG was kept over BCM63xx RNG is because the former
>> deals correctly with a warm up count and the number of words available in the
>> FIFO size.
> 
> are these the same patches as in this branch [1]?
> 
> https://github.com/ffainelli/linux/commits/rng-consolidation

Yes, this branch contains these 12 patches.
-- 
Florian

Re: [PATCH 00/12] bcm63xx-rng conversion to bcm2835-rng

[Stefan Wahren]

Hi Florian,

> Florian Fainelli  hat am 2. November 2017 um 02:03 
> geschrieben:
> 
> 
> Hi,
> 
> As it usually happens when there is a fair amount of HW IP block re-use,
> competing implementations show up. In that case the BCM2835 HWRNG driver and
> the BCM63xx RNG driver have exactly the same register offsets and this is
> indeed the same piece of HW.
> 
> This patch series first prepares the bcm2835-rng to be more future proof and
> support newer platforms, and the last part brings in what is necessary to
> migrate the bcm63xx-rng over to bcm2835-rng. Finally we delete bcm63xx-rng
> completely.
> 
> The reason why BCM2835 RNG was kept over BCM63xx RNG is because the former
> deals correctly with a warm up count and the number of words available in the
> FIFO size.

are these the same patches as in this branch [1]?

https://github.com/ffainelli/linux/commits/rng-consolidation

Regards
Stefan

> 
> Thanks!
> 
> Florian Fainelli (12):
>   hwrng: bcm2835-rng: Obtain base register via resource
>   hwrng: bcm2835-rng: Define a driver private context
>   hwrng: bcm2835-rng: Move enabling to hwrng::init
>   hwrng: bcm2835-rng: Implementation cleanup callback
>   hwrng: bcm2835-rng: Use device managed helpers
>   hwrng: bcm2835-rng: Rework interrupt masking
>   hwrng: bcm2835-rng: Manage an optional clock
>   hwrng: bcm2835-rng: Abstract I/O accessors
>   hwrng: bcm2835-rng: Add Broadcom MIPS I/O accessors
>   dt-bindings: rng: Incorporate brcm,bcm6368.txt binding
>   hwrng: bcm2835-rng: Enable BCM2835 RNG to work on BCM63xx platforms
>   hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over
> 
>  .../devicetree/bindings/rng/brcm,bcm2835.txt   |  22 ++-
>  .../devicetree/bindings/rng/brcm,bcm6368.txt   |  17 ---
>  drivers/char/hw_random/Kconfig |  20 +--
>  drivers/char/hw_random/Makefile|   1 -
>  drivers/char/hw_random/bcm2835-rng.c   | 166 
> ++---
>  drivers/char/hw_random/bcm63xx-rng.c   | 154 ---
>  6 files changed, 139 insertions(+), 241 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
>  delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
> 
> -- 
> 2.9.3
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 00/12] bcm63xx-rng conversion to bcm2835-rng

[Stefan Wahren]

Hi Florian,

> Florian Fainelli  hat am 2. November 2017 um 02:03 
> geschrieben:
> 
> 
> Hi,
> 
> As it usually happens when there is a fair amount of HW IP block re-use,
> competing implementations show up. In that case the BCM2835 HWRNG driver and
> the BCM63xx RNG driver have exactly the same register offsets and this is
> indeed the same piece of HW.
> 
> This patch series first prepares the bcm2835-rng to be more future proof and
> support newer platforms, and the last part brings in what is necessary to
> migrate the bcm63xx-rng over to bcm2835-rng. Finally we delete bcm63xx-rng
> completely.
> 
> The reason why BCM2835 RNG was kept over BCM63xx RNG is because the former
> deals correctly with a warm up count and the number of words available in the
> FIFO size.

are these the same patches as in this branch [1]?

https://github.com/ffainelli/linux/commits/rng-consolidation

Regards
Stefan

> 
> Thanks!
> 
> Florian Fainelli (12):
>   hwrng: bcm2835-rng: Obtain base register via resource
>   hwrng: bcm2835-rng: Define a driver private context
>   hwrng: bcm2835-rng: Move enabling to hwrng::init
>   hwrng: bcm2835-rng: Implementation cleanup callback
>   hwrng: bcm2835-rng: Use device managed helpers
>   hwrng: bcm2835-rng: Rework interrupt masking
>   hwrng: bcm2835-rng: Manage an optional clock
>   hwrng: bcm2835-rng: Abstract I/O accessors
>   hwrng: bcm2835-rng: Add Broadcom MIPS I/O accessors
>   dt-bindings: rng: Incorporate brcm,bcm6368.txt binding
>   hwrng: bcm2835-rng: Enable BCM2835 RNG to work on BCM63xx platforms
>   hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over
> 
>  .../devicetree/bindings/rng/brcm,bcm2835.txt   |  22 ++-
>  .../devicetree/bindings/rng/brcm,bcm6368.txt   |  17 ---
>  drivers/char/hw_random/Kconfig |  20 +--
>  drivers/char/hw_random/Makefile|   1 -
>  drivers/char/hw_random/bcm2835-rng.c   | 166 
> ++---
>  drivers/char/hw_random/bcm63xx-rng.c   | 154 ---
>  6 files changed, 139 insertions(+), 241 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/rng/brcm,bcm6368.txt
>  delete mode 100644 drivers/char/hw_random/bcm63xx-rng.c
> 
> -- 
> 2.9.3
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] crypto: dh - don't permit 'p' to be 0

[Eric Biggers]

On Thu, Nov 02, 2017 at 01:40:51PM +0200, Tudor Ambarus wrote:
> Hi, Eric,
> 
> On 11/02/2017 12:25 AM, Eric Biggers wrote:
> >If 'p' is 0 for the software Diffie-Hellman implementation, then
> >dh_max_size() returns 0.
> 
> dh_set_secret() returns -EINVAL if p_len < 1536, see
> dh_check_params_length(). What am I missing?
> 
> Cheers,
> ta

You pass a buffer containing 0's, not a buffer of length 0.  The buffer is
interpreted as an arbitrary precision integer, so any length buffer filled with
0's has the mathematical value 0.

Eric

Re: [PATCH 1/4] crypto: dh - fix double free of ctx->p

[Eric Biggers]

Hi Tudor,

On Thu, Nov 02, 2017 at 12:55:56PM +0200, Tudor Ambarus wrote:
> Hi, Eric,
> 
> On 11/02/2017 12:25 AM, Eric Biggers wrote:
> >When setting the secret with the software Diffie-Hellman implementation,
> >if allocating 'g' failed (e.g. if it was longer than
> >MAX_EXTERN_MPI_BITS), then 'p' was freed twice: once immediately, and
> >once later when the crypto_kpp tfm was destroyed.  Fix it by using
> >dh_free_ctx() in the error paths, as that sets the pointers to NULL.
> 
> Other solution would be to have just an one-line patch that sets p to
> NULL after freeing it. The benefit of just setting p to NULL and not
> using dh_free_ctx() is that you'll spare some cpu cycles. If g fails,
> g and a will already be NULL, so freeing them and set them to NULL is
> redundant.
> 
> However, if you decide to always use dh_free_ctx(), you'll have to get
> rid of dh_clear_params(), because it will be used just in dh_free_ctx().
> You can copy dh_clear_params() body to dh_free_ctx().
> 

This is on an error path, so a few cycles don't matter.  I would much rather
have the simpler code, with less chance to introduce exploitable bugs.

Yes, I should inline dh_clear_params() into dh_free_ctx().

Eric

Re: [Part2 PATCH v7 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support

[Borislav Petkov]

On Wed, Nov 01, 2017 at 04:15:58PM -0500, Brijesh Singh wrote:
> AMD's new Secure Encrypted Virtualization (SEV) feature allows the
> memory contents of virtual machines to be transparently encrypted with a
> key unique to the VM. The programming and management of the encryption
> keys are handled by the AMD Secure Processor (AMD-SP) which exposes the
> commands for these tasks. The complete spec is available at:
> 
> http://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf
> 
> Extend the AMD-SP driver to provide the following support:
> 
>  - an in-kernel API to communicate with the SEV firmware. The API can be
>used by the hypervisor to create encryption context for a SEV guest.
> 
>  - a userspace IOCTL to manage the platform certificates.
> 
> Cc: Paolo Bonzini 
> Cc: "Radim Krčmář" 
> Cc: Borislav Petkov 
> Cc: Herbert Xu 
> Cc: Gary Hook 
> Cc: Tom Lendacky 
> Cc: linux-crypto@vger.kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Improvements-by: Borislav Petkov 
> Signed-off-by: Brijesh Singh 
> ---
>  drivers/crypto/ccp/psp-dev.c | 350 
> +++
>  drivers/crypto/ccp/psp-dev.h |  24 +++
>  drivers/crypto/ccp/sp-dev.c  |   9 ++
>  drivers/crypto/ccp/sp-dev.h  |   4 +
>  include/linux/psp-sev.h  | 143 ++
>  5 files changed, 530 insertions(+)

Some more cleanups:

* If sev_data_init is per psp_device, you can simply embed it in the
struct psp_device and save yourself the complete allocation.

* s/sev_ops_init/sev_misc_init/ because it doesn't do anything ops-like

* save some header lines.

---
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index c61ca16096ca..81c70f9ce23c 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -186,7 +186,7 @@ static int __sev_platform_init_locked(struct sev_data_init 
*data, int *error)
return 0;
 
if (!data)
-   data = psp->sev_init;
+   data = >cmd_buf;
 
rc = __sev_do_cmd_locked(SEV_CMD_INIT, data, error);
if (rc)
@@ -282,7 +282,7 @@ static void sev_exit(struct kref *ref)
misc_deregister(_dev->misc);
 }
 
-static int sev_ops_init(struct psp_device *psp)
+static int sev_misc_init(struct psp_device *psp)
 {
struct device *dev = psp->dev;
int ret;
@@ -319,10 +319,6 @@ static int sev_ops_init(struct psp_device *psp)
if (!psp->sev_status)
return -ENOMEM;
 
-   psp->sev_init = devm_kzalloc(dev, sizeof(*psp->sev_init), GFP_KERNEL);
-   if (!psp->sev_init)
-   return -ENOMEM;
-
init_waitqueue_head(>sev_int_queue);
psp->sev_misc = misc_dev;
dev_dbg(dev, "registered SEV device\n");
@@ -338,7 +334,7 @@ static int sev_init(struct psp_device *psp)
return 1;
}
 
-   return sev_ops_init(psp);
+   return sev_misc_init(psp);
 }
 
 int psp_dev_init(struct sp_device *sp)
diff --git a/drivers/crypto/ccp/psp-dev.h b/drivers/crypto/ccp/psp-dev.h
index 2236d6749d2e..fa62ff50912b 100644
--- a/drivers/crypto/ccp/psp-dev.h
+++ b/drivers/crypto/ccp/psp-dev.h
@@ -77,7 +77,7 @@ struct psp_device {
wait_queue_head_t sev_int_queue;
struct sev_misc_dev *sev_misc;
struct sev_user_data_status *sev_status;
-   struct sev_data_init *sev_init;
+   struct sev_data_init cmd_buf;
 };
 
 #endif /* __PSP_DEV_H */
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index fb563248d9a9..d57cd7625a9f 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -626,11 +626,7 @@ sev_guest_activate(struct sev_data_activate *data, int 
*error) { return -ENODEV;
 static inline int sev_guest_df_flush(int *error) { return -ENODEV; }
 
 static inline int
-sev_issue_cmd_external_user(struct file *filep,
-   unsigned int id, void *data, int *error)
-{
-   return -ENODEV;
-}
+sev_issue_cmd_external_user(struct file *filep, unsigned int id, void *data, 
int *error) { return -ENODEV; }
 
 #endif /* CONFIG_CRYPTO_DEV_SP_PSP */
 

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[v3 PATCH 2/3] crypto: atmel-aes/tdes - remove empty functions

[Tudor Ambarus]

Pointer members of an object with static storage duration, if not
explicitly initialized, will be initialized to a NULL pointer.
The crypto API checks if these pointers are not NULL before using them,
therefore we can safely remove these empty functions.

Signed-off-by: Tudor Ambarus 
---
changes in v3:
- update the commit message
changes in v2:
- remove empty atmel_aes_gcm_exit()

 drivers/crypto/atmel-aes.c  | 20 
 drivers/crypto/atmel-tdes.c | 18 --
 2 files changed, 38 deletions(-)

diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 9659759..c2f0a12 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -1237,10 +1237,6 @@ static int atmel_aes_ctr_cra_init(struct crypto_tfm *tfm)
return 0;
 }
 
-static void atmel_aes_cra_exit(struct crypto_tfm *tfm)
-{
-}
-
 static struct crypto_alg aes_algs[] = {
 {
.cra_name   = "ecb(aes)",
@@ -1253,7 +1249,6 @@ static struct crypto_alg aes_algs[] = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1273,7 +1268,6 @@ static struct crypto_alg aes_algs[] = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1294,7 +1288,6 @@ static struct crypto_alg aes_algs[] = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1315,7 +1308,6 @@ static struct crypto_alg aes_algs[] = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1336,7 +1328,6 @@ static struct crypto_alg aes_algs[] = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1357,7 +1348,6 @@ static struct crypto_alg aes_algs[] = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1378,7 +1368,6 @@ static struct crypto_alg aes_algs[] = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1399,7 +1388,6 @@ static struct crypto_alg aes_algs[] = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_ctr_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1422,7 +1410,6 @@ static struct crypto_alg aes_cfb64_alg = {
.cra_type   = _ablkcipher_type,
.cra_module = THIS_MODULE,
.cra_init   = atmel_aes_cra_init,
-   .cra_exit   = atmel_aes_cra_exit,
.cra_u.ablkcipher = {
.min_keysize= AES_MIN_KEY_SIZE,
.max_keysize= AES_MAX_KEY_SIZE,
@@ -1809,18 +1796,12 @@ static int atmel_aes_gcm_init(struct crypto_aead *tfm)
return 0;
 }
 
-static void atmel_aes_gcm_exit(struct crypto_aead *tfm)
-{
-
-}
-
 static struct aead_alg aes_gcm_alg = {
.setkey = atmel_aes_gcm_setkey,
.setauthsize= atmel_aes_gcm_setauthsize,
.encrypt= atmel_aes_gcm_encrypt,
.decrypt= 

[PATCH] crypto: ecdh - remove empty exit()

[Tudor Ambarus]

Pointer members of an object with static storage duration, if not
explicitly initialized, will be initialized to a NULL pointer. The crypto
API checks if this pointer is not NULL before using it, we are safe to
remove the function.

Signed-off-by: Tudor Ambarus 
---
 crypto/ecdh.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/crypto/ecdh.c b/crypto/ecdh.c
index 4271fc7..3aca093 100644
--- a/crypto/ecdh.c
+++ b/crypto/ecdh.c
@@ -131,17 +131,11 @@ static unsigned int ecdh_max_size(struct crypto_kpp *tfm)
return ctx->ndigits << (ECC_DIGITS_TO_BYTES_SHIFT + 1);
 }
 
-static void no_exit_tfm(struct crypto_kpp *tfm)
-{
-   return;
-}
-
 static struct kpp_alg ecdh = {
.set_secret = ecdh_set_secret,
.generate_public_key = ecdh_compute_value,
.compute_shared_secret = ecdh_compute_value,
.max_size = ecdh_max_size,
-   .exit = no_exit_tfm,
.base = {
.cra_name = "ecdh",
.cra_driver_name = "ecdh-generic",
-- 
2.9.4

Re: [PATCH] crypto: ccm - preserve the IV buffer

[Tudor Ambarus]

On 10/31/2017 04:42 PM, Romain Izard wrote:

The IV buffer used during CCM operations is used twice, during both the
hashing step and the ciphering step.

When using a hardware accelerator that updates the contents of the IV
buffer at the end of ciphering operations, the value will be modified.
In the decryption case, the subsequent setup of the hashing algorithm
will interpret the updated IV instead of the original value, which can
lead to out-of-bounds writes.

Reuse the idata buffer, only used in the hashing step, to preserve the
IV's value during the ciphering step in the decryption case.

Signed-off-by: Romain Izard 
---
  crypto/ccm.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/crypto/ccm.c b/crypto/ccm.c
index 1ce37ae0ce56..0a083342ec8c 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -363,7 +363,7 @@ static int crypto_ccm_decrypt(struct aead_request *req)
unsigned int cryptlen = req->cryptlen;
u8 *authtag = pctx->auth_tag;
u8 *odata = pctx->odata;
-   u8 *iv = req->iv;
+   u8 *iv = pctx->idata;
int err;
  
  	cryptlen -= authsize;

@@ -379,6 +379,8 @@ static int crypto_ccm_decrypt(struct aead_request *req)
if (req->src != req->dst)
dst = pctx->dst;
  
+	memcpy(iv, req->iv, 16);

+
skcipher_request_set_tfm(skreq, ctx->ctr);
skcipher_request_set_callback(skreq, pctx->flags,
  crypto_ccm_decrypt_done, req);



Reviewed-by: Tudor Ambarus 

Re: [PATCH 2/4] crypto: dh - don't permit 'p' to be 0

[Tudor Ambarus]

Hi, Eric,

On 11/02/2017 12:25 AM, Eric Biggers wrote:

If 'p' is 0 for the software Diffie-Hellman implementation, then
dh_max_size() returns 0.


dh_set_secret() returns -EINVAL if p_len < 1536, see
dh_check_params_length(). What am I missing?

Cheers,
ta

Re: [PATCH 1/4] crypto: dh - fix double free of ctx->p

[Tudor Ambarus]

Hi, Eric,

On 11/02/2017 12:25 AM, Eric Biggers wrote:

When setting the secret with the software Diffie-Hellman implementation,
if allocating 'g' failed (e.g. if it was longer than
MAX_EXTERN_MPI_BITS), then 'p' was freed twice: once immediately, and
once later when the crypto_kpp tfm was destroyed.  Fix it by using
dh_free_ctx() in the error paths, as that sets the pointers to NULL.


Other solution would be to have just an one-line patch that sets p to
NULL after freeing it. The benefit of just setting p to NULL and not
using dh_free_ctx() is that you'll spare some cpu cycles. If g fails,
g and a will already be NULL, so freeing them and set them to NULL is
redundant.

However, if you decide to always use dh_free_ctx(), you'll have to get
rid of dh_clear_params(), because it will be used just in dh_free_ctx().
You can copy dh_clear_params() body to dh_free_ctx().

Cheers,
ta

Re: [PATCH RESEND 4/4] dma: caam: add dma memcpy driver

[kbuild test robot]

Hi Radu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on cryptodev/master]
[also build test WARNING on v4.14-rc7 next-20171018]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Horia-Geant/add-CAAM-DMA-memcpy-driver/20171102-081734
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

>> WARNING: drivers/dma/caam_dma.o(.data+0x8): Section mismatch in reference 
>> from the variable caam_dma_driver to the function .init.text:caam_dma_probe()
   The variable caam_dma_driver references
   the function __init caam_dma_probe()
   If the reference is valid then annotate the
   variable with or __refdata (see linux/init.h) or name the variable:
   

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH v2 1/3] staging: ccree: copy IV to DMAable memory

[Gilad Ben-Yossef]

We are being passed an IV buffer from unknown origin, which may be
stack allocated and thus not safe for DMA. Allocate a DMA safe
buffer for the IV and use that instead.

Signed-off-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_cipher.c | 20 ++--
 drivers/staging/ccree/ssi_cipher.h |  1 +
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index 78706f5..0b69103 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -695,6 +695,7 @@ static int ssi_blkcipher_complete(struct device *dev,
struct ablkcipher_request *req = (struct ablkcipher_request *)areq;
 
ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
+   kfree(req_ctx->iv);
 
/*Decrease the inflight counter*/
if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
@@ -757,6 +758,17 @@ static int ssi_blkcipher_process(
rc = 0;
goto exit_process;
}
+
+   /* The IV we are handed may be allocted from the stack so
+* we must copy it to a DMAable buffer before use.
+*/
+   req_ctx->iv = kmalloc(ivsize, GFP_KERNEL);
+   if (!req_ctx->iv) {
+   rc = -ENOMEM;
+   goto exit_process;
+   }
+   memcpy(req_ctx->iv, info, ivsize);
+
/*For CTS in case of data size aligned to 16 use CBC mode*/
if (((nbytes % AES_BLOCK_SIZE) == 0) && (ctx_p->cipher_mode == 
DRV_CIPHER_CBC_CTS)) {
ctx_p->cipher_mode = DRV_CIPHER_CBC;
@@ -778,7 +790,9 @@ static int ssi_blkcipher_process(
 
/* STAT_PHASE_1: Map buffers */
 
-   rc = ssi_buffer_mgr_map_blkcipher_request(ctx_p->drvdata, req_ctx, 
ivsize, nbytes, info, src, dst);
+   rc = ssi_buffer_mgr_map_blkcipher_request(ctx_p->drvdata, req_ctx,
+ ivsize, nbytes, req_ctx->iv,
+ src, dst);
if (unlikely(rc != 0)) {
dev_err(dev, "map_request() failed\n");
goto exit_process;
@@ -830,8 +844,10 @@ static int ssi_blkcipher_process(
if (cts_restore_flag != 0)
ctx_p->cipher_mode = DRV_CIPHER_CBC_CTS;
 
-   if (rc != -EINPROGRESS)
+   if (rc != -EINPROGRESS) {
kfree(req_ctx->backup_info);
+   kfree(req_ctx->iv);
+   }
 
return rc;
 }
diff --git a/drivers/staging/ccree/ssi_cipher.h 
b/drivers/staging/ccree/ssi_cipher.h
index f499962..25e6335 100644
--- a/drivers/staging/ccree/ssi_cipher.h
+++ b/drivers/staging/ccree/ssi_cipher.h
@@ -43,6 +43,7 @@ struct blkcipher_req_ctx {
u32 out_nents;
u32 out_mlli_nents;
u8 *backup_info; /*store iv for generated IV flow*/
+   u8 *iv;
bool is_giv;
struct mlli_params mlli_params;
 };
-- 
2.7.4

[PATCH v2 3/3] staging: ccree: remove dead code

[Gilad Ben-Yossef]

The inflight_counter field is updated in a single location and
never used. Remove it.

Signed-off-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_cipher.c | 4 
 drivers/staging/ccree/ssi_driver.h | 1 -
 2 files changed, 5 deletions(-)

diff --git a/drivers/staging/ccree/ssi_cipher.c 
b/drivers/staging/ccree/ssi_cipher.c
index 0b69103..ee85cbf 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -697,10 +697,6 @@ static int ssi_blkcipher_complete(struct device *dev,
ssi_buffer_mgr_unmap_blkcipher_request(dev, req_ctx, ivsize, src, dst);
kfree(req_ctx->iv);
 
-   /*Decrease the inflight counter*/
-   if (ctx_p->flow_mode == BYPASS && ctx_p->drvdata->inflight_counter > 0)
-   ctx_p->drvdata->inflight_counter--;
-
if (areq) {
/*
 * The crypto API expects us to set the req->info to the last
diff --git a/drivers/staging/ccree/ssi_driver.h 
b/drivers/staging/ccree/ssi_driver.h
index 488f665..e01c880 100644
--- a/drivers/staging/ccree/ssi_driver.h
+++ b/drivers/staging/ccree/ssi_driver.h
@@ -134,7 +134,6 @@ struct ssi_drvdata {
void *fips_handle;
void *ivgen_handle;
void *sram_mgr_handle;
-   u32 inflight_counter;
struct clk *clk;
bool coherent;
 };
-- 
2.7.4

[PATCH v2 2/3] staging: ccree: handle limiting of DMA masks

[Gilad Ben-Yossef]

Properly handle limiting of DMA masks based on device and bus
capabilities.

Signed-off-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/ssi_driver.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index 5f03c25..1d4c7bb 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -208,6 +208,7 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
struct device *dev = _dev->dev;
struct device_node *np = dev->of_node;
u32 signature_val;
+   dma_addr_t dma_mask;
int rc = 0;
 
new_drvdata = devm_kzalloc(dev, sizeof(*new_drvdata), GFP_KERNEL);
@@ -256,15 +257,29 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
}
dev_dbg(dev, "Registered to IRQ: %d\n", new_drvdata->irq);
 
+   if (!plat_dev->dev.dma_mask)
+   plat_dev->dev.dma_mask = _dev->dev.coherent_dma_mask;
+
+   dma_mask = (dma_addr_t)(DMA_BIT_MASK(DMA_BIT_MASK_LEN));
+   while (dma_mask > 0x7fffUL) {
+   if (dma_supported(_dev->dev, dma_mask)) {
+   rc = dma_set_coherent_mask(_dev->dev, dma_mask);
+   if (!rc)
+   break;
+   }
+   dma_mask >>= 1;
+   }
+
+   if (rc) {
+   dev_err(dev, "Error: failed in dma_set_mask, mask=%par\n",
+   _mask);
+   goto post_drvdata_err;
+   }
+
rc = cc_clk_on(new_drvdata);
if (rc)
goto post_drvdata_err;
 
-   if (!dev->dma_mask)
-   dev->dma_mask = >coherent_dma_mask;
-
-   if (!dev->coherent_dma_mask)
-   dev->coherent_dma_mask = DMA_BIT_MASK(DMA_BIT_MASK_LEN);
 
/* Verify correct mapping */
signature_val = CC_HAL_READ_REGISTER(CC_REG_OFFSET(HOST_RGF, 
HOST_SIGNATURE));
-- 
2.7.4

[PATCH v2 0/3] staging: ccree: Fixes and cleanups

[Gilad Ben-Yossef]

Fixes and cleanups for 4.15

Changes from v1:
- Move DMA mask code to before turning on clocks, based
  on feedback from Dan Carpenter.
- Add missing kmalloc success check, as spotted by Dan
  Carpenter.

Gilad Ben-Yossef (3):
  staging: ccree: copy IV to DMAable memory
  staging: ccree: handle limiting of DMA masks
  staging: ccree: remove dead code

 drivers/staging/ccree/ssi_cipher.c | 24 ++--
 drivers/staging/ccree/ssi_cipher.h |  1 +
 drivers/staging/ccree/ssi_driver.c | 25 -
 drivers/staging/ccree/ssi_driver.h |  1 -
 4 files changed, 39 insertions(+), 12 deletions(-)

-- 
2.7.4