| Message ID | 1521112738-13250-1-git-send-email-s.mesoraca16@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Mar 15, 2018 at 12:18:58PM +0100, Salvatore Mesoraca wrote: > > +#define MAX_BLOCKSIZE 16 > + > +#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS > +#define MAX_ALIGNMASK 15 > +#else > +#define MAX_ALIGNMASK 0 > +#endif > + Hmm, this won't work. Just because you have efficient unaligned access in general doesn't mean that every implementation can live with unaligned access. In particular, on x86 there are quite a few implementations that require alignment or they will fault. So please just make it 15 unconditionally. Thanks,
2018-03-23 16:36 GMT+01:00 Herbert Xu <herbert@gondor.apana.org.au>: > On Thu, Mar 15, 2018 at 12:18:58PM +0100, Salvatore Mesoraca wrote: >> >> +#define MAX_BLOCKSIZE 16 >> + >> +#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS >> +#define MAX_ALIGNMASK 15 >> +#else >> +#define MAX_ALIGNMASK 0 >> +#endif >> + > > Hmm, this won't work. Just because you have efficient unaligned > access in general doesn't mean that every implementation can live > with unaligned access. In particular, on x86 there are quite a > few implementations that require alignment or they will fault. > > So please just make it 15 unconditionally. Oh, thank you for pointing it out. I'll fix this in v3. Salvatore
diff --git a/crypto/ctr.c b/crypto/ctr.c index 854d924..2c9f80f 100644 --- a/crypto/ctr.c +++ b/crypto/ctr.c @@ -21,6 +21,14 @@ #include <linux/scatterlist.h> #include <linux/slab.h> +#define MAX_BLOCKSIZE 16 + +#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS +#define MAX_ALIGNMASK 15 +#else +#define MAX_ALIGNMASK 0 +#endif + struct crypto_ctr_ctx { struct crypto_cipher *child; }; @@ -58,7 +66,7 @@ static void crypto_ctr_crypt_final(struct blkcipher_walk *walk, unsigned int bsize = crypto_cipher_blocksize(tfm); unsigned long alignmask = crypto_cipher_alignmask(tfm); u8 *ctrblk = walk->iv; - u8 tmp[bsize + alignmask]; + u8 tmp[MAX_BLOCKSIZE + MAX_ALIGNMASK]; u8 *keystream = PTR_ALIGN(tmp + 0, alignmask + 1); u8 *src = walk->src.virt.addr; u8 *dst = walk->dst.virt.addr; @@ -106,7 +114,7 @@ static int crypto_ctr_crypt_inplace(struct blkcipher_walk *walk, unsigned int nbytes = walk->nbytes; u8 *ctrblk = walk->iv; u8 *src = walk->src.virt.addr; - u8 tmp[bsize + alignmask]; + u8 tmp[MAX_BLOCKSIZE + MAX_ALIGNMASK]; u8 *keystream = PTR_ALIGN(tmp + 0, alignmask + 1); do { @@ -206,6 +214,14 @@ static struct crypto_instance *crypto_ctr_alloc(struct rtattr **tb) if (alg->cra_blocksize < 4) goto out_put_alg; + /* Block size must be <= MAX_BLOCKSIZE. */ + if (alg->cra_blocksize > MAX_BLOCKSIZE) + goto out_put_alg; + + /* Alignmask must be <= MAX_ALIGNMASK. */ + if (alg->cra_alignmask > MAX_ALIGNMASK) + goto out_put_alg; + /* If this is false we'd fail the alignment of crypto_inc. */ if (alg->cra_blocksize % 4) goto out_put_alg;
All ciphers implemented in Linux have a block size less than or equal to 16 bytes and the most demanding hw require 16 bytes alignment for the block buffer. We avoid 2 VLAs[1] by always allocating 16 bytes with 16 bytes alignment, unless the architecture supports efficient unaligned accesses. We also check the selected cipher at instance creation time, if it doesn't comply with these limits, we fail the creation. [1] https://lkml.org/lkml/2018/3/7/621 Signed-off-by: Salvatore Mesoraca <s.mesoraca16@gmail.com> --- crypto/ctr.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)