* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).