Message ID | 1522146713-25175-2-git-send-email-paolo.pisati@canonical.com |
---|---|
State | New |
Headers | show |
Series | SAUCE: crypto: thunderx_zip: Fix fallout from CONFIG_VMAP_STACK | expand |
Applied to bionic master-next branch.
Thanks.
Cascardo.
Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Applied-to: bionic/master-next
On 27.03.2018 12:31, Paolo Pisati wrote: > From: Jan Glauber <jglauber@cavium.com> > > BugLink: http://bugs.launchpad.net/bugs/1755073 > > Enabling virtual mapped kernel stacks breaks the thunderx_zip > driver. On compression or decompression the executing CPU hangs > in an endless loop. The reason for this is the usage of __pa() > by the driver that does not work for an address that is > not part of the 1:1 mapping. > > The zip driver allocates a result struct on the stack and needs > to tell the hardware the pysical address within this struct > that is used to signal the completion of the request. > With CONFIG_VMAP_STACK __pa() is no longer usable for a stack address. > > As the hardware gets the wrong address it writes the result byte > to an arbitrary address. The zip driver then waits forever for the > completion byte to contain a non-zero value. > > Allocating the result struct from 1:1 mapped memory resolves this > bug. > > Note that there are more outstanding issues with the used completion > mechanism: > - volatile is used for a bitfield > - no barriers at all re used > - the completion loop should have a retry counter and not run forever > - polling for the result should use a delay > - interrupts for completion and error notification should be used > > Signed-off-by: Jan Glauber <jglauber@cavium.com> > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > drivers/crypto/cavium/zip/zip_crypto.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/crypto/cavium/zip/zip_crypto.c b/drivers/crypto/cavium/zip/zip_crypto.c > index 8df4d26..2fc9b03 100644 > --- a/drivers/crypto/cavium/zip/zip_crypto.c > +++ b/drivers/crypto/cavium/zip/zip_crypto.c > @@ -124,7 +124,7 @@ int zip_compress(const u8 *src, unsigned int slen, > struct zip_kernel_ctx *zip_ctx) > { > struct zip_operation *zip_ops = NULL; > - struct zip_state zip_state; > + struct zip_state *zip_state; > struct zip_device *zip = NULL; > int ret; > > @@ -135,20 +135,23 @@ int zip_compress(const u8 *src, unsigned int slen, > if (!zip) > return -ENODEV; > > - memset(&zip_state, 0, sizeof(struct zip_state)); > + zip_state = kzalloc(sizeof(*zip_state), GFP_KERNEL); > + if (!zip_state) > + return -ENOMEM; > + > zip_ops = &zip_ctx->zip_comp; > > zip_ops->input_len = slen; > zip_ops->output_len = *dlen; > memcpy(zip_ops->input, src, slen); > > - ret = zip_deflate(zip_ops, &zip_state, zip); > + ret = zip_deflate(zip_ops, zip_state, zip); > > if (!ret) { > *dlen = zip_ops->output_len; > memcpy(dst, zip_ops->output, *dlen); > } > - > + kfree(zip_state); > return ret; > } > > @@ -157,7 +160,7 @@ int zip_decompress(const u8 *src, unsigned int slen, > struct zip_kernel_ctx *zip_ctx) > { > struct zip_operation *zip_ops = NULL; > - struct zip_state zip_state; > + struct zip_state *zip_state; > struct zip_device *zip = NULL; > int ret; > > @@ -168,7 +171,10 @@ int zip_decompress(const u8 *src, unsigned int slen, > if (!zip) > return -ENODEV; > > - memset(&zip_state, 0, sizeof(struct zip_state)); > + zip_state = kzalloc(sizeof(*zip_state), GFP_KERNEL); > + if (!zip_state) > + return -ENOMEM; > + > zip_ops = &zip_ctx->zip_decomp; > memcpy(zip_ops->input, src, slen); > > @@ -179,13 +185,13 @@ int zip_decompress(const u8 *src, unsigned int slen, > zip_ops->input_len = slen; > zip_ops->output_len = *dlen; > > - ret = zip_inflate(zip_ops, &zip_state, zip); > + ret = zip_inflate(zip_ops, zip_state, zip); > > if (!ret) { > *dlen = zip_ops->output_len; > memcpy(dst, zip_ops->output, *dlen); > } > - > + kfree(zip_state); > return ret; > } > >
On 03/27/18 12:31, Paolo Pisati wrote: > From: Jan Glauber <jglauber@cavium.com> > > BugLink: http://bugs.launchpad.net/bugs/1755073 > > Enabling virtual mapped kernel stacks breaks the thunderx_zip > driver. On compression or decompression the executing CPU hangs > in an endless loop. The reason for this is the usage of __pa() > by the driver that does not work for an address that is > not part of the 1:1 mapping. > > The zip driver allocates a result struct on the stack and needs > to tell the hardware the pysical address within this struct > that is used to signal the completion of the request. > With CONFIG_VMAP_STACK __pa() is no longer usable for a stack address. > > As the hardware gets the wrong address it writes the result byte > to an arbitrary address. The zip driver then waits forever for the > completion byte to contain a non-zero value. > > Allocating the result struct from 1:1 mapped memory resolves this > bug. > > Note that there are more outstanding issues with the used completion > mechanism: > - volatile is used for a bitfield > - no barriers at all re used > - the completion loop should have a retry counter and not run forever > - polling for the result should use a delay > - interrupts for completion and error notification should be used > > Signed-off-by: Jan Glauber <jglauber@cavium.com> > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> > --- > drivers/crypto/cavium/zip/zip_crypto.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/crypto/cavium/zip/zip_crypto.c b/drivers/crypto/cavium/zip/zip_crypto.c > index 8df4d26..2fc9b03 100644 > --- a/drivers/crypto/cavium/zip/zip_crypto.c > +++ b/drivers/crypto/cavium/zip/zip_crypto.c > @@ -124,7 +124,7 @@ int zip_compress(const u8 *src, unsigned int slen, > struct zip_kernel_ctx *zip_ctx) > { > struct zip_operation *zip_ops = NULL; > - struct zip_state zip_state; > + struct zip_state *zip_state; > struct zip_device *zip = NULL; > int ret; > > @@ -135,20 +135,23 @@ int zip_compress(const u8 *src, unsigned int slen, > if (!zip) > return -ENODEV; > > - memset(&zip_state, 0, sizeof(struct zip_state)); > + zip_state = kzalloc(sizeof(*zip_state), GFP_KERNEL); > + if (!zip_state) > + return -ENOMEM; > + > zip_ops = &zip_ctx->zip_comp; > > zip_ops->input_len = slen; > zip_ops->output_len = *dlen; > memcpy(zip_ops->input, src, slen); > > - ret = zip_deflate(zip_ops, &zip_state, zip); > + ret = zip_deflate(zip_ops, zip_state, zip); > > if (!ret) { > *dlen = zip_ops->output_len; > memcpy(dst, zip_ops->output, *dlen); > } > - > + kfree(zip_state); > return ret; > } > > @@ -157,7 +160,7 @@ int zip_decompress(const u8 *src, unsigned int slen, > struct zip_kernel_ctx *zip_ctx) > { > struct zip_operation *zip_ops = NULL; > - struct zip_state zip_state; > + struct zip_state *zip_state; > struct zip_device *zip = NULL; > int ret; > > @@ -168,7 +171,10 @@ int zip_decompress(const u8 *src, unsigned int slen, > if (!zip) > return -ENODEV; > > - memset(&zip_state, 0, sizeof(struct zip_state)); > + zip_state = kzalloc(sizeof(*zip_state), GFP_KERNEL); > + if (!zip_state) > + return -ENOMEM; > + > zip_ops = &zip_ctx->zip_decomp; > memcpy(zip_ops->input, src, slen); > > @@ -179,13 +185,13 @@ int zip_decompress(const u8 *src, unsigned int slen, > zip_ops->input_len = slen; > zip_ops->output_len = *dlen; > > - ret = zip_inflate(zip_ops, &zip_state, zip); > + ret = zip_inflate(zip_ops, zip_state, zip); > > if (!ret) { > *dlen = zip_ops->output_len; > memcpy(dst, zip_ops->output, *dlen); > } > - > + kfree(zip_state); > return ret; > } > > Applied to artful/master-next branch. Thanks, Kleber
diff --git a/drivers/crypto/cavium/zip/zip_crypto.c b/drivers/crypto/cavium/zip/zip_crypto.c index 8df4d26..2fc9b03 100644 --- a/drivers/crypto/cavium/zip/zip_crypto.c +++ b/drivers/crypto/cavium/zip/zip_crypto.c @@ -124,7 +124,7 @@ int zip_compress(const u8 *src, unsigned int slen, struct zip_kernel_ctx *zip_ctx) { struct zip_operation *zip_ops = NULL; - struct zip_state zip_state; + struct zip_state *zip_state; struct zip_device *zip = NULL; int ret; @@ -135,20 +135,23 @@ int zip_compress(const u8 *src, unsigned int slen, if (!zip) return -ENODEV; - memset(&zip_state, 0, sizeof(struct zip_state)); + zip_state = kzalloc(sizeof(*zip_state), GFP_KERNEL); + if (!zip_state) + return -ENOMEM; + zip_ops = &zip_ctx->zip_comp; zip_ops->input_len = slen; zip_ops->output_len = *dlen; memcpy(zip_ops->input, src, slen); - ret = zip_deflate(zip_ops, &zip_state, zip); + ret = zip_deflate(zip_ops, zip_state, zip); if (!ret) { *dlen = zip_ops->output_len; memcpy(dst, zip_ops->output, *dlen); } - + kfree(zip_state); return ret; } @@ -157,7 +160,7 @@ int zip_decompress(const u8 *src, unsigned int slen, struct zip_kernel_ctx *zip_ctx) { struct zip_operation *zip_ops = NULL; - struct zip_state zip_state; + struct zip_state *zip_state; struct zip_device *zip = NULL; int ret; @@ -168,7 +171,10 @@ int zip_decompress(const u8 *src, unsigned int slen, if (!zip) return -ENODEV; - memset(&zip_state, 0, sizeof(struct zip_state)); + zip_state = kzalloc(sizeof(*zip_state), GFP_KERNEL); + if (!zip_state) + return -ENOMEM; + zip_ops = &zip_ctx->zip_decomp; memcpy(zip_ops->input, src, slen); @@ -179,13 +185,13 @@ int zip_decompress(const u8 *src, unsigned int slen, zip_ops->input_len = slen; zip_ops->output_len = *dlen; - ret = zip_inflate(zip_ops, &zip_state, zip); + ret = zip_inflate(zip_ops, zip_state, zip); if (!ret) { *dlen = zip_ops->output_len; memcpy(dst, zip_ops->output, *dlen); } - + kfree(zip_state); return ret; }