2016-09-28 16:42:08

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: [PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

This series fixes the memory corruption found by Jan Stancek in 4.8-rc7. The
problem however also affects previous versions of the driver.

Marcelo Cerri (3):
crypto: ghash-generic - move common definitions to a new header file
crypto: vmx - Fix memory corruption caused by p8_ghash
crypto: vmx - Ensure ghash-generic is enabled

crypto/ghash-generic.c | 13 +------------
drivers/crypto/vmx/Kconfig | 2 +-
drivers/crypto/vmx/ghash.c | 31 ++++++++++++++++---------------
include/crypto/ghash.h | 23 +++++++++++++++++++++++
4 files changed, 41 insertions(+), 28 deletions(-)
create mode 100644 include/crypto/ghash.h

--
2.7.4


2016-09-28 16:42:54

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: [PATCH 2/3] crypto: vmx - Fix memory corruption caused by p8_ghash

This patch changes the p8_ghash driver to use ghash-generic as a fixed
fallback implementation. This allows the correct value of descsize to be
defined directly in its shash_alg structure and avoids problems with
incorrect buffer sizes when its state is exported or imported.

Reported-by: Jan Stancek <[email protected]>
Signed-off-by: Marcelo Cerri <[email protected]>
---
drivers/crypto/vmx/ghash.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/vmx/ghash.c b/drivers/crypto/vmx/ghash.c
index 6c999cb0..27a94a1 100644
--- a/drivers/crypto/vmx/ghash.c
+++ b/drivers/crypto/vmx/ghash.c
@@ -26,16 +26,13 @@
#include <linux/hardirq.h>
#include <asm/switch_to.h>
#include <crypto/aes.h>
+#include <crypto/ghash.h>
#include <crypto/scatterwalk.h>
#include <crypto/internal/hash.h>
#include <crypto/b128ops.h>

#define IN_INTERRUPT in_interrupt()

-#define GHASH_BLOCK_SIZE (16)
-#define GHASH_DIGEST_SIZE (16)
-#define GHASH_KEY_LEN (16)
-
void gcm_init_p8(u128 htable[16], const u64 Xi[2]);
void gcm_gmult_p8(u64 Xi[2], const u128 htable[16]);
void gcm_ghash_p8(u64 Xi[2], const u128 htable[16],
@@ -55,16 +52,11 @@ struct p8_ghash_desc_ctx {

static int p8_ghash_init_tfm(struct crypto_tfm *tfm)
{
- const char *alg;
+ const char *alg = "ghash-generic";
struct crypto_shash *fallback;
struct crypto_shash *shash_tfm = __crypto_shash_cast(tfm);
struct p8_ghash_ctx *ctx = crypto_tfm_ctx(tfm);

- if (!(alg = crypto_tfm_alg_name(tfm))) {
- printk(KERN_ERR "Failed to get algorithm name.\n");
- return -ENOENT;
- }
-
fallback = crypto_alloc_shash(alg, 0, CRYPTO_ALG_NEED_FALLBACK);
if (IS_ERR(fallback)) {
printk(KERN_ERR
@@ -78,10 +70,18 @@ static int p8_ghash_init_tfm(struct crypto_tfm *tfm)
crypto_shash_set_flags(fallback,
crypto_shash_get_flags((struct crypto_shash
*) tfm));
- ctx->fallback = fallback;

- shash_tfm->descsize = sizeof(struct p8_ghash_desc_ctx)
- + crypto_shash_descsize(fallback);
+ /* Check if the descsize defined in the algorithm is still enough. */
+ if (shash_tfm->descsize < sizeof(struct p8_ghash_desc_ctx)
+ + crypto_shash_descsize(fallback)) {
+ printk(KERN_ERR
+ "Desc size of the fallback implementation (%s) does not match the expected value: %lu vs %u\n",
+ alg,
+ shash_tfm->descsize - sizeof(struct p8_ghash_desc_ctx),
+ crypto_shash_descsize(fallback));
+ return -EINVAL;
+ }
+ ctx->fallback = fallback;

return 0;
}
@@ -113,7 +113,7 @@ static int p8_ghash_setkey(struct crypto_shash *tfm, const u8 *key,
{
struct p8_ghash_ctx *ctx = crypto_tfm_ctx(crypto_shash_tfm(tfm));

- if (keylen != GHASH_KEY_LEN)
+ if (keylen != GHASH_BLOCK_SIZE)
return -EINVAL;

preempt_disable();
@@ -211,7 +211,8 @@ struct shash_alg p8_ghash_alg = {
.update = p8_ghash_update,
.final = p8_ghash_final,
.setkey = p8_ghash_setkey,
- .descsize = sizeof(struct p8_ghash_desc_ctx),
+ .descsize = sizeof(struct p8_ghash_desc_ctx)
+ + sizeof(struct ghash_desc_ctx),
.base = {
.cra_name = "ghash",
.cra_driver_name = "p8_ghash",
--
2.7.4

2016-09-28 16:42:09

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: [PATCH 1/3] crypto: ghash-generic - move common definitions to a new header file

Move common values and types used by ghash-generic to a new header file
so drivers can directly use ghash-generic as a fallback implementation.

Signed-off-by: Marcelo Cerri <[email protected]>
---
crypto/ghash-generic.c | 13 +------------
include/crypto/ghash.h | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+), 12 deletions(-)
create mode 100644 include/crypto/ghash.h

diff --git a/crypto/ghash-generic.c b/crypto/ghash-generic.c
index bac7099..12ad3e3 100644
--- a/crypto/ghash-generic.c
+++ b/crypto/ghash-generic.c
@@ -14,24 +14,13 @@

#include <crypto/algapi.h>
#include <crypto/gf128mul.h>
+#include <crypto/ghash.h>
#include <crypto/internal/hash.h>
#include <linux/crypto.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>

-#define GHASH_BLOCK_SIZE 16
-#define GHASH_DIGEST_SIZE 16
-
-struct ghash_ctx {
- struct gf128mul_4k *gf128;
-};
-
-struct ghash_desc_ctx {
- u8 buffer[GHASH_BLOCK_SIZE];
- u32 bytes;
-};
-
static int ghash_init(struct shash_desc *desc)
{
struct ghash_desc_ctx *dctx = shash_desc_ctx(desc);
diff --git a/include/crypto/ghash.h b/include/crypto/ghash.h
new file mode 100644
index 0000000..2a61c9b
--- /dev/null
+++ b/include/crypto/ghash.h
@@ -0,0 +1,23 @@
+/*
+ * Common values for GHASH algorithms
+ */
+
+#ifndef __CRYPTO_GHASH_H__
+#define __CRYPTO_GHASH_H__
+
+#include <linux/types.h>
+#include <crypto/gf128mul.h>
+
+#define GHASH_BLOCK_SIZE 16
+#define GHASH_DIGEST_SIZE 16
+
+struct ghash_ctx {
+ struct gf128mul_4k *gf128;
+};
+
+struct ghash_desc_ctx {
+ u8 buffer[GHASH_BLOCK_SIZE];
+ u32 bytes;
+};
+
+#endif
--
2.7.4

2016-09-28 16:43:06

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: [PATCH 3/3] crypto: vmx - Ensure ghash-generic is enabled

Add CRYPTO_GHASH as a dependency for vmx_crypto since p8_ghash uses it
as the fallback implementation.

Signed-off-by: Marcelo Cerri <[email protected]>
---
drivers/crypto/vmx/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/vmx/Kconfig b/drivers/crypto/vmx/Kconfig
index a83ead1..c21b09a 100644
--- a/drivers/crypto/vmx/Kconfig
+++ b/drivers/crypto/vmx/Kconfig
@@ -1,6 +1,6 @@
config CRYPTO_DEV_VMX_ENCRYPT
tristate "Encryption acceleration support on P8 CPU"
- depends on CRYPTO_DEV_VMX
+ depends on CRYPTO_DEV_VMX || CRYPTO_GHASH
default m
help
Support for VMX cryptographic acceleration instructions on Power8 CPU.
--
2.7.4

2016-09-28 20:59:25

by Anton Blanchard

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

Hi Marcelo

> This series fixes the memory corruption found by Jan Stancek in
> 4.8-rc7. The problem however also affects previous versions of the
> driver.

If it affects previous versions, please add the lines in the sign off to
get it into the stable kernels.

Anton

2016-10-02 14:40:16

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

On Wed, Sep 28, 2016 at 01:42:08PM -0300, Marcelo Cerri wrote:
> This series fixes the memory corruption found by Jan Stancek in 4.8-rc7. The
> problem however also affects previous versions of the driver.
>
> Marcelo Cerri (3):
> crypto: ghash-generic - move common definitions to a new header file
> crypto: vmx - Fix memory corruption caused by p8_ghash
> crypto: vmx - Ensure ghash-generic is enabled

Patch 1/2 applied to crypto. I have changed patch 3 to use select
instead of depend and have applied it to cryptodev.

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

2016-10-02 14:40:50

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

On Thu, Sep 29, 2016 at 06:59:08AM +1000, Anton Blanchard wrote:
> Hi Marcelo
>
> > This series fixes the memory corruption found by Jan Stancek in
> > 4.8-rc7. The problem however also affects previous versions of the
> > driver.
>
> If it affects previous versions, please add the lines in the sign off to
> get it into the stable kernels.

I have added them to patches 1 and 2. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2016-10-03 12:08:35

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

Thank you.

--
Regards,
Marcelo

On Sun, Oct 02, 2016 at 10:40:47PM +0800, Herbert Xu wrote:
> On Thu, Sep 29, 2016 at 06:59:08AM +1000, Anton Blanchard wrote:
> > Hi Marcelo
> >
> > > This series fixes the memory corruption found by Jan Stancek in
> > > 4.8-rc7. The problem however also affects previous versions of the
> > > driver.
> >
> > If it affects previous versions, please add the lines in the sign off to
> > get it into the stable kernels.
>
> I have added them to patches 1 and 2. Thanks.
> --
> Email: Herbert Xu <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Attachments:
(No filename) (672.00 B)
signature.asc (473.00 B)
Download all attachments

2016-10-03 15:07:30

by Marcelo Henrique Cerri

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

Hi Herbert,

Sorry for bothering you. I noticed you included two of the patches in
the crypto-2.6 repository and the remaining one in cryptodev-2.6. Is
that right? I thought all 3 patches would be included in the cruptodev
repository.

--
Regards,
Marcelo

On Wed, Sep 28, 2016 at 01:42:08PM -0300, Marcelo Cerri wrote:
> This series fixes the memory corruption found by Jan Stancek in 4.8-rc7. The
> problem however also affects previous versions of the driver.
>
> Marcelo Cerri (3):
> crypto: ghash-generic - move common definitions to a new header file
> crypto: vmx - Fix memory corruption caused by p8_ghash
> crypto: vmx - Ensure ghash-generic is enabled
>
> crypto/ghash-generic.c | 13 +------------
> drivers/crypto/vmx/Kconfig | 2 +-
> drivers/crypto/vmx/ghash.c | 31 ++++++++++++++++---------------
> include/crypto/ghash.h | 23 +++++++++++++++++++++++
> 4 files changed, 41 insertions(+), 28 deletions(-)
> create mode 100644 include/crypto/ghash.h
>
> --
> 2.7.4
>


Attachments:
(No filename) (0.98 kB)
signature.asc (473.00 B)
Download all attachments

2016-10-10 02:16:52

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/3] Fix crypto/vmx/p8_ghash memory corruption

On Mon, Oct 03, 2016 at 12:07:25PM -0300, Marcelo Cerri wrote:
> Hi Herbert,
>
> Sorry for bothering you. I noticed you included two of the patches in
> the crypto-2.6 repository and the remaining one in cryptodev-2.6. Is
> that right? I thought all 3 patches would be included in the cruptodev
> repository.

I wanted the first two to go to stable as well so that's why
I split them up.

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