* [PATCH 0/2] drivers/crypto - s5p fixes
@ 2019-08-19 14:22 Ard Biesheuvel
2019-08-19 14:22 ` [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes Ard Biesheuvel
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2019-08-19 14:22 UTC (permalink / raw)
To: linux-crypto
Cc: herbert, Ard Biesheuvel, Krzysztof Kozlowski, Vladimir Zapolskiy,
Kamil Konieczny, linux-samsung-soc
Fix a couple of issues in the s5p crypto driver that were caught in fuzz
testing.
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Vladimir Zapolskiy <vz@mleia.com>
Cc: Kamil Konieczny <k.konieczny@partner.samsung.com>
Cc: linux-samsung-soc@vger.kernel.org
Ard Biesheuvel (2):
crypto: s5p - deal gracefully with bogus input sizes
crypto: s5p - use correct block size of 1 for ctr(aes)
drivers/crypto/s5p-sss.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes 2019-08-19 14:22 [PATCH 0/2] drivers/crypto - s5p fixes Ard Biesheuvel @ 2019-08-19 14:22 ` Ard Biesheuvel 2019-08-20 9:51 ` Kamil Konieczny 2019-08-20 10:07 ` Krzysztof Kozlowski 2019-08-19 14:22 ` [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) Ard Biesheuvel 2019-08-30 8:19 ` [PATCH 0/2] drivers/crypto - s5p fixes Herbert Xu 2 siblings, 2 replies; 13+ messages in thread From: Ard Biesheuvel @ 2019-08-19 14:22 UTC (permalink / raw) To: linux-crypto Cc: herbert, Ard Biesheuvel, Krzysztof Kozlowski, Vladimir Zapolskiy, Kamil Konieczny, linux-samsung-soc The s5p skcipher driver returns -EINVAL for zero length inputs, which deviates from the behavior of the generic ECB template, and causes fuzz tests to fail. In cases where the input is not a multiple of the AES block size (and the chaining mode is not CTR), it prints an error to the kernel log, which is a thing we usually try to avoid in response to situations that can be triggered by unprivileged users. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- drivers/crypto/s5p-sss.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 9ef25230c199..ef90c58edb1f 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -2056,9 +2056,12 @@ static int s5p_aes_crypt(struct ablkcipher_request *req, unsigned long mode) struct s5p_aes_ctx *ctx = crypto_ablkcipher_ctx(tfm); struct s5p_aes_dev *dev = ctx->dev; + if (!req->nbytes) + return 0; + if (!IS_ALIGNED(req->nbytes, AES_BLOCK_SIZE) && ((mode & FLAGS_AES_MODE_MASK) != FLAGS_AES_CTR)) { - dev_err(dev->dev, "request size is not exact amount of AES blocks\n"); + dev_dbg(dev->dev, "request size is not exact amount of AES blocks\n"); return -EINVAL; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes 2019-08-19 14:22 ` [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes Ard Biesheuvel @ 2019-08-20 9:51 ` Kamil Konieczny 2019-08-20 10:07 ` Krzysztof Kozlowski 1 sibling, 0 replies; 13+ messages in thread From: Kamil Konieczny @ 2019-08-20 9:51 UTC (permalink / raw) To: Ard Biesheuvel, linux-crypto Cc: herbert, Krzysztof Kozlowski, Vladimir Zapolskiy, linux-samsung-soc On 19.08.2019 16:22, Ard Biesheuvel wrote: > The s5p skcipher driver returns -EINVAL for zero length inputs, which > deviates from the behavior of the generic ECB template, and causes fuzz > tests to fail. In cases where the input is not a multiple of the AES > block size (and the chaining mode is not CTR), it prints an error to > the kernel log, which is a thing we usually try to avoid in response > to situations that can be triggered by unprivileged users. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > drivers/crypto/s5p-sss.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c > index 9ef25230c199..ef90c58edb1f 100644 > --- a/drivers/crypto/s5p-sss.c > +++ b/drivers/crypto/s5p-sss.c > @@ -2056,9 +2056,12 @@ static int s5p_aes_crypt(struct ablkcipher_request *req, unsigned long mode) > struct s5p_aes_ctx *ctx = crypto_ablkcipher_ctx(tfm); > struct s5p_aes_dev *dev = ctx->dev; > > + if (!req->nbytes) > + return 0; > + > if (!IS_ALIGNED(req->nbytes, AES_BLOCK_SIZE) && > ((mode & FLAGS_AES_MODE_MASK) != FLAGS_AES_CTR)) { > - dev_err(dev->dev, "request size is not exact amount of AES blocks\n"); > + dev_dbg(dev->dev, "request size is not exact amount of AES blocks\n"); > return -EINVAL; > } Acked-by: Kamil Konieczny <k.konieczny@partner.samsung.com> -- Best regards, Kamil Konieczny Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes 2019-08-19 14:22 ` [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes Ard Biesheuvel 2019-08-20 9:51 ` Kamil Konieczny @ 2019-08-20 10:07 ` Krzysztof Kozlowski 1 sibling, 0 replies; 13+ messages in thread From: Krzysztof Kozlowski @ 2019-08-20 10:07 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-crypto, herbert, Vladimir Zapolskiy, Kamil Konieczny, linux-samsung-soc On Mon, 19 Aug 2019 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > The s5p skcipher driver returns -EINVAL for zero length inputs, which > deviates from the behavior of the generic ECB template, and causes fuzz > tests to fail. In cases where the input is not a multiple of the AES > block size (and the chaining mode is not CTR), it prints an error to > the kernel log, which is a thing we usually try to avoid in response > to situations that can be triggered by unprivileged users. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > drivers/crypto/s5p-sss.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) 2019-08-19 14:22 [PATCH 0/2] drivers/crypto - s5p fixes Ard Biesheuvel 2019-08-19 14:22 ` [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes Ard Biesheuvel @ 2019-08-19 14:22 ` Ard Biesheuvel 2019-08-20 9:51 ` Kamil Konieczny 2019-08-20 10:24 ` Krzysztof Kozlowski 2019-08-30 8:19 ` [PATCH 0/2] drivers/crypto - s5p fixes Herbert Xu 2 siblings, 2 replies; 13+ messages in thread From: Ard Biesheuvel @ 2019-08-19 14:22 UTC (permalink / raw) To: linux-crypto Cc: herbert, Ard Biesheuvel, Krzysztof Kozlowski, Vladimir Zapolskiy, Kamil Konieczny, linux-samsung-soc Align the s5p ctr(aes) implementation with other implementations of the same mode, by setting the block size to 1. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- drivers/crypto/s5p-sss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index ef90c58edb1f..010f1bb20dad 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -2173,7 +2173,7 @@ static struct crypto_alg algs[] = { .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC | CRYPTO_ALG_KERN_DRIVER_ONLY, - .cra_blocksize = AES_BLOCK_SIZE, + .cra_blocksize = 1, .cra_ctxsize = sizeof(struct s5p_aes_ctx), .cra_alignmask = 0x0f, .cra_type = &crypto_ablkcipher_type, -- 2.17.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) 2019-08-19 14:22 ` [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) Ard Biesheuvel @ 2019-08-20 9:51 ` Kamil Konieczny 2019-08-20 10:24 ` Krzysztof Kozlowski 1 sibling, 0 replies; 13+ messages in thread From: Kamil Konieczny @ 2019-08-20 9:51 UTC (permalink / raw) To: Ard Biesheuvel, linux-crypto Cc: herbert, Krzysztof Kozlowski, Vladimir Zapolskiy, linux-samsung-soc On 19.08.2019 16:22, Ard Biesheuvel wrote: > Align the s5p ctr(aes) implementation with other implementations > of the same mode, by setting the block size to 1. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > drivers/crypto/s5p-sss.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c > index ef90c58edb1f..010f1bb20dad 100644 > --- a/drivers/crypto/s5p-sss.c > +++ b/drivers/crypto/s5p-sss.c > @@ -2173,7 +2173,7 @@ static struct crypto_alg algs[] = { > .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | > CRYPTO_ALG_ASYNC | > CRYPTO_ALG_KERN_DRIVER_ONLY, > - .cra_blocksize = AES_BLOCK_SIZE, > + .cra_blocksize = 1, > .cra_ctxsize = sizeof(struct s5p_aes_ctx), > .cra_alignmask = 0x0f, > .cra_type = &crypto_ablkcipher_type, > Acked-by: Kamil Konieczny <k.konieczny@partner.samsung.com> -- Best regards, Kamil Konieczny Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) 2019-08-19 14:22 ` [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) Ard Biesheuvel 2019-08-20 9:51 ` Kamil Konieczny @ 2019-08-20 10:24 ` Krzysztof Kozlowski 2019-08-20 10:57 ` Ard Biesheuvel 2019-08-20 11:39 ` Kamil Konieczny 1 sibling, 2 replies; 13+ messages in thread From: Krzysztof Kozlowski @ 2019-08-20 10:24 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-crypto, herbert, Vladimir Zapolskiy, Kamil Konieczny, linux-samsung-soc On Mon, 19 Aug 2019 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > Align the s5p ctr(aes) implementation with other implementations > of the same mode, by setting the block size to 1. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > drivers/crypto/s5p-sss.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c > index ef90c58edb1f..010f1bb20dad 100644 > --- a/drivers/crypto/s5p-sss.c > +++ b/drivers/crypto/s5p-sss.c > @@ -2173,7 +2173,7 @@ static struct crypto_alg algs[] = { > .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | > CRYPTO_ALG_ASYNC | > CRYPTO_ALG_KERN_DRIVER_ONLY, > - .cra_blocksize = AES_BLOCK_SIZE, > + .cra_blocksize = 1, This makes sense but I wonder how does it work later with s5p_aes_crypt() and its check for request length alignment (AES_BLOCK_SIZE). With block size of 1 byte, I understand that req->nbytes can be for example 4 bytes which is not AES block aligned... If my reasoning is correct, then the CTR mode in s5p-sss is not fully working. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) 2019-08-20 10:24 ` Krzysztof Kozlowski @ 2019-08-20 10:57 ` Ard Biesheuvel 2019-08-20 11:23 ` Krzysztof Kozlowski 2019-08-20 11:39 ` Kamil Konieczny 1 sibling, 1 reply; 13+ messages in thread From: Ard Biesheuvel @ 2019-08-20 10:57 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu, Vladimir Zapolskiy, Kamil Konieczny, linux-samsung-soc On Tue, 20 Aug 2019 at 13:24, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Mon, 19 Aug 2019 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > > > Align the s5p ctr(aes) implementation with other implementations > > of the same mode, by setting the block size to 1. > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > --- > > drivers/crypto/s5p-sss.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c > > index ef90c58edb1f..010f1bb20dad 100644 > > --- a/drivers/crypto/s5p-sss.c > > +++ b/drivers/crypto/s5p-sss.c > > @@ -2173,7 +2173,7 @@ static struct crypto_alg algs[] = { > > .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | > > CRYPTO_ALG_ASYNC | > > CRYPTO_ALG_KERN_DRIVER_ONLY, > > - .cra_blocksize = AES_BLOCK_SIZE, > > + .cra_blocksize = 1, > > This makes sense but I wonder how does it work later with > s5p_aes_crypt() and its check for request length alignment > (AES_BLOCK_SIZE). With block size of 1 byte, I understand that > req->nbytes can be for example 4 bytes which is not AES block > aligned... If my reasoning is correct, then the CTR mode in s5p-sss is > not fully working. > I re-ran the kernelci.org tests with this change, and I saw no more failures. https://kernelci.org/boot/all/job/ardb/branch/for-kernelci/kernel/v5.3-rc1-197-gc8809c50be4f/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) 2019-08-20 10:57 ` Ard Biesheuvel @ 2019-08-20 11:23 ` Krzysztof Kozlowski 0 siblings, 0 replies; 13+ messages in thread From: Krzysztof Kozlowski @ 2019-08-20 11:23 UTC (permalink / raw) To: Ard Biesheuvel Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu, Vladimir Zapolskiy, Kamil Konieczny, linux-samsung-soc On Tue, 20 Aug 2019 at 12:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Tue, 20 Aug 2019 at 13:24, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > On Mon, 19 Aug 2019 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > > > > > Align the s5p ctr(aes) implementation with other implementations > > > of the same mode, by setting the block size to 1. > > > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > --- > > > drivers/crypto/s5p-sss.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c > > > index ef90c58edb1f..010f1bb20dad 100644 > > > --- a/drivers/crypto/s5p-sss.c > > > +++ b/drivers/crypto/s5p-sss.c > > > @@ -2173,7 +2173,7 @@ static struct crypto_alg algs[] = { > > > .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | > > > CRYPTO_ALG_ASYNC | > > > CRYPTO_ALG_KERN_DRIVER_ONLY, > > > - .cra_blocksize = AES_BLOCK_SIZE, > > > + .cra_blocksize = 1, > > > > This makes sense but I wonder how does it work later with > > s5p_aes_crypt() and its check for request length alignment > > (AES_BLOCK_SIZE). With block size of 1 byte, I understand that > > req->nbytes can be for example 4 bytes which is not AES block > > aligned... If my reasoning is correct, then the CTR mode in s5p-sss is > > not fully working. > > > > > I re-ran the kernelci.org tests with this change, and I saw no more failures. > > https://kernelci.org/boot/all/job/ardb/branch/for-kernelci/kernel/v5.3-rc1-197-gc8809c50be4f/ Indeed, self tests are passing. Anyway the change is correct so: Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) 2019-08-20 10:24 ` Krzysztof Kozlowski 2019-08-20 10:57 ` Ard Biesheuvel @ 2019-08-20 11:39 ` Kamil Konieczny 2019-08-20 11:48 ` Krzysztof Kozlowski 1 sibling, 1 reply; 13+ messages in thread From: Kamil Konieczny @ 2019-08-20 11:39 UTC (permalink / raw) To: Krzysztof Kozlowski, Ard Biesheuvel Cc: linux-crypto, herbert, Vladimir Zapolskiy, linux-samsung-soc On 20.08.2019 12:24, Krzysztof Kozlowski wrote: > On Mon, 19 Aug 2019 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> >> Align the s5p ctr(aes) implementation with other implementations >> of the same mode, by setting the block size to 1. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> drivers/crypto/s5p-sss.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c >> index ef90c58edb1f..010f1bb20dad 100644 >> --- a/drivers/crypto/s5p-sss.c >> +++ b/drivers/crypto/s5p-sss.c >> @@ -2173,7 +2173,7 @@ static struct crypto_alg algs[] = { >> .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | >> CRYPTO_ALG_ASYNC | >> CRYPTO_ALG_KERN_DRIVER_ONLY, >> - .cra_blocksize = AES_BLOCK_SIZE, >> + .cra_blocksize = 1, > > This makes sense but I wonder how does it work later with > s5p_aes_crypt() and its check for request length alignment > (AES_BLOCK_SIZE). With block size of 1 byte, I understand that > req->nbytes can be for example 4 bytes which is not AES block > aligned... If my reasoning is correct, then the CTR mode in s5p-sss is > not fully working. As I remember this case there are allocated buffers with len aligned up AES_BLOCK_SIZE, source data copy to one buf, hw encrypts full block, then nbytes are copy back. -- Best regards, Kamil Konieczny Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) 2019-08-20 11:39 ` Kamil Konieczny @ 2019-08-20 11:48 ` Krzysztof Kozlowski 2019-08-20 12:28 ` Kamil Konieczny 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2019-08-20 11:48 UTC (permalink / raw) To: Kamil Konieczny Cc: Ard Biesheuvel, linux-crypto, herbert, Vladimir Zapolskiy, linux-samsung-soc On Tue, 20 Aug 2019 at 13:39, Kamil Konieczny <k.konieczny@partner.samsung.com> wrote: > > > > On 20.08.2019 12:24, Krzysztof Kozlowski wrote: > > On Mon, 19 Aug 2019 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> > >> Align the s5p ctr(aes) implementation with other implementations > >> of the same mode, by setting the block size to 1. > >> > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > >> drivers/crypto/s5p-sss.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c > >> index ef90c58edb1f..010f1bb20dad 100644 > >> --- a/drivers/crypto/s5p-sss.c > >> +++ b/drivers/crypto/s5p-sss.c > >> @@ -2173,7 +2173,7 @@ static struct crypto_alg algs[] = { > >> .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | > >> CRYPTO_ALG_ASYNC | > >> CRYPTO_ALG_KERN_DRIVER_ONLY, > >> - .cra_blocksize = AES_BLOCK_SIZE, > >> + .cra_blocksize = 1, > > > > This makes sense but I wonder how does it work later with > > s5p_aes_crypt() and its check for request length alignment > > (AES_BLOCK_SIZE). With block size of 1 byte, I understand that > > req->nbytes can be for example 4 bytes which is not AES block > > aligned... If my reasoning is correct, then the CTR mode in s5p-sss is > > not fully working. > > As I remember this case there are allocated buffers with len aligned up > AES_BLOCK_SIZE, source data copy to one buf, hw encrypts full block, > then nbytes are copy back. Buffer alignment is different thing and it is defined in cra_alignmask. I am talking about req->nbytes which should be aligned according to s5p_aes_crypt(). But if blocksize is 1 byte, then what possible values for req->nbytes? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) 2019-08-20 11:48 ` Krzysztof Kozlowski @ 2019-08-20 12:28 ` Kamil Konieczny 0 siblings, 0 replies; 13+ messages in thread From: Kamil Konieczny @ 2019-08-20 12:28 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Ard Biesheuvel, linux-crypto, herbert, Vladimir Zapolskiy, linux-samsung-soc On 20.08.2019 13:48, Krzysztof Kozlowski wrote: > On Tue, 20 Aug 2019 at 13:39, Kamil Konieczny > <k.konieczny@partner.samsung.com> wrote: >> >> >> >> On 20.08.2019 12:24, Krzysztof Kozlowski wrote: >>> On Mon, 19 Aug 2019 at 16:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >>>> >>>> Align the s5p ctr(aes) implementation with other implementations >>>> of the same mode, by setting the block size to 1. >>>> >>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>>> --- >>>> drivers/crypto/s5p-sss.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c >>>> index ef90c58edb1f..010f1bb20dad 100644 >>>> --- a/drivers/crypto/s5p-sss.c >>>> +++ b/drivers/crypto/s5p-sss.c >>>> @@ -2173,7 +2173,7 @@ static struct crypto_alg algs[] = { >>>> .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | >>>> CRYPTO_ALG_ASYNC | >>>> CRYPTO_ALG_KERN_DRIVER_ONLY, >>>> - .cra_blocksize = AES_BLOCK_SIZE, >>>> + .cra_blocksize = 1, >>> >>> This makes sense but I wonder how does it work later with >>> s5p_aes_crypt() and its check for request length alignment >>> (AES_BLOCK_SIZE). With block size of 1 byte, I understand that >>> req->nbytes can be for example 4 bytes which is not AES block >>> aligned... If my reasoning is correct, then the CTR mode in s5p-sss is >>> not fully working. >> >> As I remember this case there are allocated buffers with len aligned up >> AES_BLOCK_SIZE, source data copy to one buf, hw encrypts full block, >> then nbytes are copy back. to be precise, hw encrypts align_up(req->nbytes, AES_BLOCK_SIZE) > Buffer alignment is different thing and it is defined in cra_alignmask. > I am talking about req->nbytes which should be aligned according to > s5p_aes_crypt(). But if blocksize is 1 byte, then what possible values > for req->nbytes? For AES-CTR, there are blocks of size multiply of AES_BLOCK_SIZE, with last one which can be of any size so for req->nbytes valid values are n*AES_BLOCK_SIZE + 0...AES_BLOCK_SIZE-1 -- Best regards, Kamil Konieczny Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] drivers/crypto - s5p fixes 2019-08-19 14:22 [PATCH 0/2] drivers/crypto - s5p fixes Ard Biesheuvel 2019-08-19 14:22 ` [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes Ard Biesheuvel 2019-08-19 14:22 ` [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) Ard Biesheuvel @ 2019-08-30 8:19 ` Herbert Xu 2 siblings, 0 replies; 13+ messages in thread From: Herbert Xu @ 2019-08-30 8:19 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-crypto, Krzysztof Kozlowski, Vladimir Zapolskiy, Kamil Konieczny, linux-samsung-soc On Mon, Aug 19, 2019 at 05:22:24PM +0300, Ard Biesheuvel wrote: > Fix a couple of issues in the s5p crypto driver that were caught in fuzz > testing. > > Cc: Krzysztof Kozlowski <krzk@kernel.org> > Cc: Vladimir Zapolskiy <vz@mleia.com> > Cc: Kamil Konieczny <k.konieczny@partner.samsung.com> > Cc: linux-samsung-soc@vger.kernel.org > > Ard Biesheuvel (2): > crypto: s5p - deal gracefully with bogus input sizes > crypto: s5p - use correct block size of 1 for ctr(aes) > > drivers/crypto/s5p-sss.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) All applied. Thanks. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-08-30 8:19 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-19 14:22 [PATCH 0/2] drivers/crypto - s5p fixes Ard Biesheuvel 2019-08-19 14:22 ` [PATCH 1/2] crypto: s5p - deal gracefully with bogus input sizes Ard Biesheuvel 2019-08-20 9:51 ` Kamil Konieczny 2019-08-20 10:07 ` Krzysztof Kozlowski 2019-08-19 14:22 ` [PATCH 2/2] crypto: s5p - use correct block size of 1 for ctr(aes) Ard Biesheuvel 2019-08-20 9:51 ` Kamil Konieczny 2019-08-20 10:24 ` Krzysztof Kozlowski 2019-08-20 10:57 ` Ard Biesheuvel 2019-08-20 11:23 ` Krzysztof Kozlowski 2019-08-20 11:39 ` Kamil Konieczny 2019-08-20 11:48 ` Krzysztof Kozlowski 2019-08-20 12:28 ` Kamil Konieczny 2019-08-30 8:19 ` [PATCH 0/2] drivers/crypto - s5p fixes Herbert Xu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.