| Message ID | 1394241960-1764-1-git-send-email-behanw@converseincode.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 2014-03-07 at 17:26 -0800, behanw@converseincode.com wrote: > From: Jan-Simon Möller <dl9pf@gmx.de> > > Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99 > compliant equivalent. This is the original VLAIS struct. [] > diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c [] > @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, > u8 *data, size_t data_len, u8 *mic) > { > struct scatterlist assoc, pt, ct[2]; > - struct { > - struct aead_request req; > - u8 priv[crypto_aead_reqsize(tfm)]; > - } aead_req; > > - memset(&aead_req, 0, sizeof(aead_req)); > + char aead_req_data[sizeof(struct aead_request) + > + crypto_aead_reqsize(tfm) + > + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; Can this be a too large amount of stack? Is crypto_aead_reqsize() limited to < ~1k? Perhaps it'd be better to use kzalloc for this or another reserved pool -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/07/14 17:56, Joe Perches wrote: > On Fri, 2014-03-07 at 17:26 -0800, behanw@converseincode.com wrote: >> From: Jan-Simon Möller <dl9pf@gmx.de> >> >> Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99 >> compliant equivalent. This is the original VLAIS struct. > [] >> diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c > [] >> @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, >> u8 *data, size_t data_len, u8 *mic) >> { >> struct scatterlist assoc, pt, ct[2]; >> - struct { >> - struct aead_request req; >> - u8 priv[crypto_aead_reqsize(tfm)]; >> - } aead_req; >> >> - memset(&aead_req, 0, sizeof(aead_req)); >> + char aead_req_data[sizeof(struct aead_request) + >> + crypto_aead_reqsize(tfm) + >> + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; > Can this be a too large amount of stack? > > Is crypto_aead_reqsize() limited to < ~1k? > > Perhaps it'd be better to use kzalloc for this > or another reserved pool No more stack being used than with the the original code. The stack memory use is identical. Behan
On Fri, 2014-03-07 at 18:15 -0800, Behan Webster wrote: > On 03/07/14 17:56, Joe Perches wrote: > > On Fri, 2014-03-07 at 17:26 -0800, behanw@converseincode.com wrote: > >> From: Jan-Simon Möller <dl9pf@gmx.de> > >> > >> Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99 > >> compliant equivalent. This is the original VLAIS struct. > > [] > >> diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c > > [] > >> @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, > >> u8 *data, size_t data_len, u8 *mic) > >> { > >> struct scatterlist assoc, pt, ct[2]; > >> - struct { > >> - struct aead_request req; > >> - u8 priv[crypto_aead_reqsize(tfm)]; > >> - } aead_req; > >> > >> - memset(&aead_req, 0, sizeof(aead_req)); > >> + char aead_req_data[sizeof(struct aead_request) + > >> + crypto_aead_reqsize(tfm) + > >> + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; > > Can this be a too large amount of stack? > > > > Is crypto_aead_reqsize() limited to < ~1k? > > > > Perhaps it'd be better to use kzalloc for this > > or another reserved pool > No more stack being used than with the the original code. The stack > memory use is identical. I do understand that, but that's not my question. I appreciate you're getting this to compile for llvm. Any idea of the max value of crypto_aead_reqsize? include/linux/crypto.h:static inline unsigned int crypto_aead_reqsize(struct crypto_aead *tfm) include/linux/crypto.h-{ include/linux/crypto.h- return crypto_aead_crt(tfm)->reqsize; include/linux/crypto.h-} $ grep-2.5.4 -rP --include=*.[ch] "(?:->|\.)\s*\breqsize\s*=[^=][^;]+;" * arch/x86/crypto/aesni-intel_glue.c: tfm->crt_aead.reqsize = sizeof(struct aead_request) + crypto_aead_reqsize(&cryptd_tfm->base); crypto/chainiv.c: tfm->crt_ablkcipher.reqsize = sizeof(struct ablkcipher_request); crypto/authenc.c: tfm->crt_aead.reqsize = sizeof(struct authenc_request_ctx) + ctx->reqoff + max_t(unsigned int, crypto_ahash_reqsize(auth) + sizeof(struct ahash_request), sizeof(struct skcipher_givcrypt_request) + crypto_ablkcipher_reqsize(enc)); crypto/authencesn.c: tfm->crt_aead.reqsize = sizeof(struct authenc_esn_request_ctx) + ctx->reqoff + max_t(unsigned int, crypto_ahash_reqsize(auth) + sizeof(struct ahash_request), sizeof(struct skcipher_givcrypt_request) + crypto_ablkcipher_reqsize(enc)); crypto/shash.c: crt->reqsize = sizeof(struct shash_desc) + crypto_shash_descsize(shash); crypto/cryptd.c: tfm->crt_ablkcipher.reqsize = sizeof(struct cryptd_blkcipher_request_ctx); crypto/cryptd.c: tfm->crt_aead.reqsize = sizeof(struct cryptd_aead_request_ctx); crypto/seqiv.c: tfm->crt_ablkcipher.reqsize = sizeof(struct ablkcipher_request); crypto/seqiv.c: tfm->crt_aead.reqsize = sizeof(struct aead_request); crypto/ablk_helper.c: tfm->crt_ablkcipher.reqsize = sizeof(struct ablkcipher_request) + crypto_ablkcipher_reqsize(&cryptd_tfm->base); crypto/ctr.c: tfm->crt_ablkcipher.reqsize = align + sizeof(struct crypto_rfc3686_req_ctx) + crypto_ablkcipher_reqsize(cipher); crypto/eseqiv.c: tfm->crt_ablkcipher.reqsize = reqsize + sizeof(struct ablkcipher_request); crypto/ccm.c: tfm->crt_aead.reqsize = align + sizeof(struct crypto_ccm_req_priv_ctx) + crypto_ablkcipher_reqsize(ctr); crypto/ccm.c: tfm->crt_aead.reqsize = sizeof(struct aead_request) + ALIGN(crypto_aead_reqsize(aead), crypto_tfm_ctx_alignment()) + align + 16; crypto/gcm.c: tfm->crt_aead.reqsize = align + offsetof(struct crypto_gcm_req_priv_ctx, u) + max(sizeof(struct ablkcipher_request) + crypto_ablkcipher_reqsize(ctr), sizeof(struct ahash_request) + crypto_ahash_reqsize(ghash)); crypto/gcm.c: tfm->crt_aead.reqsize = sizeof(struct aead_request) + ALIGN(crypto_aead_reqsize(aead), crypto_tfm_ctx_alignment()) + align + 16; crypto/gcm.c: tfm->crt_aead.reqsize = sizeof(struct crypto_rfc4543_req_ctx) + ALIGN(crypto_aead_reqsize(aead), crypto_tfm_ctx_alignment()) + align + 16; crypto/pcrypt.c: tfm->crt_aead.reqsize = sizeof(struct pcrypt_request) + sizeof(struct aead_givcrypt_request) + crypto_aead_reqsize(cipher); drivers/staging/sep/sep_crypto.c: tfm->crt_ablkcipher.reqsize = sizeof(struct this_task_ctx); drivers/crypto/n2_core.c: tfm->crt_ablkcipher.reqsize = sizeof(struct n2_request_context); drivers/crypto/mxs-dcp.c: tfm->crt_ablkcipher.reqsize = sizeof(struct dcp_aes_req_ctx); drivers/crypto/sahara.c: tfm->crt_ablkcipher.reqsize = sizeof(struct sahara_aes_reqctx); drivers/crypto/ccp/ccp-crypto-aes.c: tfm->crt_ablkcipher.reqsize = sizeof(struct ccp_aes_req_ctx); drivers/crypto/ccp/ccp-crypto-aes.c: tfm->crt_ablkcipher.reqsize = sizeof(struct ccp_aes_req_ctx); drivers/crypto/ccp/ccp-crypto-aes-xts.c: tfm->crt_ablkcipher.reqsize = sizeof(struct ccp_aes_req_ctx) + fallback_tfm->base.crt_ablkcipher.reqsize; drivers/crypto/s5p-sss.c: tfm->crt_ablkcipher.reqsize = sizeof(struct s5p_aes_reqctx); drivers/crypto/atmel-aes.c: tfm->crt_ablkcipher.reqsize = sizeof(struct atmel_aes_reqctx); drivers/crypto/ixp4xx_crypto.c: tfm->crt_ablkcipher.reqsize = sizeof(struct ablk_ctx); drivers/crypto/ixp4xx_crypto.c: tfm->crt_aead.reqsize = sizeof(struct aead_ctx); drivers/crypto/omap-des.c: tfm->crt_ablkcipher.reqsize = sizeof(struct omap_des_reqctx); drivers/crypto/mv_cesa.c: tfm->crt_ablkcipher.reqsize = sizeof(struct mv_req_ctx); drivers/crypto/omap-aes.c: tfm->crt_ablkcipher.reqsize = sizeof(struct omap_aes_reqctx); drivers/crypto/hifn_795x.c: tfm->crt_ablkcipher.reqsize = sizeof(struct hifn_request_context); drivers/crypto/amcc/crypto4xx_core.c: tfm->crt_ablkcipher.reqsize = sizeof(struct crypto4xx_ctx); drivers/crypto/atmel-tdes.c: tfm->crt_ablkcipher.reqsize = sizeof(struct atmel_tdes_reqctx); drivers/crypto/picoxcell_crypto.c: tfm->crt_aead.reqsize = sizeof(struct spacc_req); drivers/crypto/picoxcell_crypto.c: tfm->crt_ablkcipher.reqsize = sizeof(struct spacc_req); include/crypto/internal/hash.h: tfm->reqsize = reqsize; -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 07, 2014 at 06:15:43PM -0800, Behan Webster wrote: > On 03/07/14 17:56, Joe Perches wrote: > >On Fri, 2014-03-07 at 17:26 -0800, behanw@converseincode.com wrote: > >>From: Jan-Simon Möller <dl9pf@gmx.de> > >> > >>Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99 > >>compliant equivalent. This is the original VLAIS struct. > >[] > >>diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c > >[] > >>@@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, > >> u8 *data, size_t data_len, u8 *mic) > >> { > >> struct scatterlist assoc, pt, ct[2]; > >>- struct { > >>- struct aead_request req; > >>- u8 priv[crypto_aead_reqsize(tfm)]; > >>- } aead_req; > >>- memset(&aead_req, 0, sizeof(aead_req)); > >>+ char aead_req_data[sizeof(struct aead_request) + > >>+ crypto_aead_reqsize(tfm) + > >>+ CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; > >Can this be a too large amount of stack? > > > >Is crypto_aead_reqsize() limited to < ~1k? > > > >Perhaps it'd be better to use kzalloc for this > >or another reserved pool > No more stack being used than with the the original code. The stack > memory use is identical. Could you explain that? It looks like aead_req_data can be bigger than original struct aead_req up to CRYPTO_MINALIGN bytes. IOW adding CRYPTO_MINALIGN to size seems unneeded as aead_request->__ctx alignment requirement should by assured by proper sizeof(struct aead_request). Besides, why not add VLAIS feature to llvm instead of fixing all programs that use it? Stanislaw -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 03/07/14 18:27, Joe Perches wrote: > On Fri, 2014-03-07 at 18:15 -0800, Behan Webster wrote: >> On 03/07/14 17:56, Joe Perches wrote: >>> On Fri, 2014-03-07 at 17:26 -0800, behanw@converseincode.com wrote: >>>> From: Jan-Simon Möller <dl9pf@gmx.de> >>>> >>>> Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99 >>>> compliant equivalent. This is the original VLAIS struct. >>> [] >>>> diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c >>> [] >>>> @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, >>>> u8 *data, size_t data_len, u8 *mic) >>>> { >>>> struct scatterlist assoc, pt, ct[2]; >>>> - struct { >>>> - struct aead_request req; >>>> - u8 priv[crypto_aead_reqsize(tfm)]; >>>> - } aead_req; >>>> >>>> - memset(&aead_req, 0, sizeof(aead_req)); >>>> + char aead_req_data[sizeof(struct aead_request) + >>>> + crypto_aead_reqsize(tfm) + >>>> + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; >>> Can this be a too large amount of stack? >>> >>> Is crypto_aead_reqsize() limited to < ~1k? >>> >>> Perhaps it'd be better to use kzalloc for this >>> or another reserved pool >> No more stack being used than with the the original code. The stack >> memory use is identical. > I do understand that, but that's not my question. > > I appreciate you're getting this to compile for llvm. > > Any idea of the max value of crypto_aead_reqsize? And I understand your question, as well as why it is important. Moving it from being stack based to alloacted memory may or may not be a good thing, but is orthogonal to the intention of this particular patch (which is just to remove the use of VLAIS from this code). > $ grep-2.5.4 -rP --include=*.[ch] "(?:->|\.)\s*\breqsize\s*=[^=][^;]+;" * Very clever. I'm going to use this. :) Behan
On 8 March 2014 02:26, <behanw@converseincode.com> wrote: > diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c > index 7c7df47..3317578 100644 > --- a/net/mac80211/aes_ccm.c > +++ b/net/mac80211/aes_ccm.c > @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, > u8 *data, size_t data_len, u8 *mic) > { > struct scatterlist assoc, pt, ct[2]; > - struct { > - struct aead_request req; > - u8 priv[crypto_aead_reqsize(tfm)]; > - } aead_req; > > - memset(&aead_req, 0, sizeof(aead_req)); > + char aead_req_data[sizeof(struct aead_request) + > + crypto_aead_reqsize(tfm) + > + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; > + > + struct aead_request *aead_req = (void *) aead_req_data; Bad trick with regards to alignment. The alignment requirement for struct aead_request is stronger than what an array of chars may have. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 03/08/14 12:29, Sergei Antonov wrote: > On 8 March 2014 02:26, <behanw@converseincode.com> wrote: >> diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c >> index 7c7df47..3317578 100644 >> --- a/net/mac80211/aes_ccm.c >> +++ b/net/mac80211/aes_ccm.c >> @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, >> u8 *data, size_t data_len, u8 *mic) >> { >> struct scatterlist assoc, pt, ct[2]; >> - struct { >> - struct aead_request req; >> - u8 priv[crypto_aead_reqsize(tfm)]; >> - } aead_req; >> >> - memset(&aead_req, 0, sizeof(aead_req)); >> + char aead_req_data[sizeof(struct aead_request) + >> + crypto_aead_reqsize(tfm) + >> + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; >> + >> + struct aead_request *aead_req = (void *) aead_req_data; > Bad trick with regards to alignment. > The alignment requirement for struct aead_request is stronger than > what an array of chars may have. Damn it. I should have seen that too. We had to do that in our related netfilter patch. Good catch. Will fix. Thanks, Behan
On 03/08/14 06:53, Stanislaw Gruszka wrote: > On Fri, Mar 07, 2014 at 06:15:43PM -0800, Behan Webster wrote: >> On 03/07/14 17:56, Joe Perches wrote: >>> On Fri, 2014-03-07 at 17:26 -0800, behanw@converseincode.com wrote: >>>> From: Jan-Simon Möller <dl9pf@gmx.de> >>>> >>>> Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99 >>>> compliant equivalent. This is the original VLAIS struct. >>> [] >>>> diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c >>> [] >>>> @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, >>>> u8 *data, size_t data_len, u8 *mic) >>>> { >>>> struct scatterlist assoc, pt, ct[2]; >>>> - struct { >>>> - struct aead_request req; >>>> - u8 priv[crypto_aead_reqsize(tfm)]; >>>> - } aead_req; >>>> - memset(&aead_req, 0, sizeof(aead_req)); >>>> + char aead_req_data[sizeof(struct aead_request) + >>>> + crypto_aead_reqsize(tfm) + >>>> + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; >>> Can this be a too large amount of stack? >>> >>> Is crypto_aead_reqsize() limited to < ~1k? >>> >>> Perhaps it'd be better to use kzalloc for this >>> or another reserved pool >> No more stack being used than with the the original code. The stack >> memory use is identical. > Could you explain that? It looks like aead_req_data can be bigger than > original struct aead_req up to CRYPTO_MINALIGN bytes. IOW adding > CRYPTO_MINALIGN to size seems unneeded as aead_request->__ctx alignment > requirement should by assured by proper sizeof(struct aead_request). True, possibly a few more bytes. Nothing significant. However, I will fix this too. > Besides, why not add VLAIS feature to llvm instead of fixing all > programs that use it? You aren't the first to ask this (I certainly get asked this regularly). :) VLAIS is specifically disallowed by the C89, C99, and later C standards. VLAIS is also not an officially documented feature of gcc (It is a side effect from the support of other languages supported by the gcc toolchain). The LLVM project won't add VLAIS support for these reasons, and because it would unnecessarily complicate their compiler architecture with almost no appreciable gain. VLAIS is only used in a handful of places in the kernel (almost exclusively in crypto related code), so changing it in the kernel not only is easier, but also makes the kernel code C99 (and beyond) compliant. Being standards compliant is important not only for clang, but also for newer versions of gcc (though not yet in the case of VLAIS). VLAIS should not be confused with Variable Length Arrays (VLA) and flexible members (undefined or zero length arrays at the end of a non embedded struct) which are both explicitly in the C standards and supported by both gcc and clang. Behan
On 8 Mar 2014 at 21:29, Sergei Antonov wrote: > > - memset(&aead_req, 0, sizeof(aead_req)); > > + char aead_req_data[sizeof(struct aead_request) + > > + crypto_aead_reqsize(tfm) + > > + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; ^^^^^^^^^^^^^^^^^^^^ wouldn't the underlined attribute ensure the required alignment? > Bad trick with regards to alignment. > The alignment requirement for struct aead_request is stronger than > what an array of chars may have. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 8 March 2014 23:01, PaX Team <pageexec@freemail.hu> wrote: > On 8 Mar 2014 at 21:29, Sergei Antonov wrote: > >> > - memset(&aead_req, 0, sizeof(aead_req)); >> > + char aead_req_data[sizeof(struct aead_request) + >> > + crypto_aead_reqsize(tfm) + >> > + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; > ^^^^^^^^^^^^^^^^^^^^ > wouldn't the underlined attribute ensure the required alignment? Yes. Sorry, I overlooked it. I would remove unneeded CRYPTO_MINALIGN and get the alignment from the target structure: char aead_req_data[sizeof(struct aead_request) + crypto_aead_reqsize(tfm)] __attribute__((__aligned__(__alignof__(struct aead_request)))); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 9. März 2014 00:00:19 MEZ, Sergei Antonov <saproj@gmail.com> wrote: >On 8 March 2014 23:01, PaX Team <pageexec@freemail.hu> wrote: >> On 8 Mar 2014 at 21:29, Sergei Antonov wrote: >> >>> > - memset(&aead_req, 0, sizeof(aead_req)); >>> > + char aead_req_data[sizeof(struct aead_request) + >>> > + crypto_aead_reqsize(tfm) + >>> > + CRYPTO_MINALIGN] >CRYPTO_MINALIGN_ATTR; >> >^^^^^^^^^^^^^^^^^^^^ >> wouldn't the underlined attribute ensure the required alignment? >Yes. Sorry, I overlooked it. > >I would remove unneeded CRYPTO_MINALIGN and get the alignment from the >target structure: > char aead_req_data[sizeof(struct aead_request) + > crypto_aead_reqsize(tfm)] > __attribute__((__aligned__(__alignof__(struct aead_request)))); LGTM
From: Sergei Antonov <saproj@gmail.com> Date: Sat, 8 Mar 2014 21:29:57 +0100 > On 8 March 2014 02:26, <behanw@converseincode.com> wrote: >> diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c >> index 7c7df47..3317578 100644 >> --- a/net/mac80211/aes_ccm.c >> +++ b/net/mac80211/aes_ccm.c >> @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, >> u8 *data, size_t data_len, u8 *mic) >> { >> struct scatterlist assoc, pt, ct[2]; >> - struct { >> - struct aead_request req; >> - u8 priv[crypto_aead_reqsize(tfm)]; >> - } aead_req; >> >> - memset(&aead_req, 0, sizeof(aead_req)); >> + char aead_req_data[sizeof(struct aead_request) + >> + crypto_aead_reqsize(tfm) + >> + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; >> + >> + struct aead_request *aead_req = (void *) aead_req_data; > > Bad trick with regards to alignment. > The alignment requirement for struct aead_request is stronger than > what an array of chars may have. Indeed, this will crash on platforms such as sparc64 that trap on unaligned accesses. You have to tell the compiler the alignment using proper types. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 03/08/14 15:00, Sergei Antonov wrote: > On 8 March 2014 23:01, PaX Team <pageexec@freemail.hu> wrote: >> On 8 Mar 2014 at 21:29, Sergei Antonov wrote: >> >>>> - memset(&aead_req, 0, sizeof(aead_req)); >>>> + char aead_req_data[sizeof(struct aead_request) + >>>> + crypto_aead_reqsize(tfm) + >>>> + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; >> ^^^^^^^^^^^^^^^^^^^^ >> wouldn't the underlined attribute ensure the required alignment? > Yes. Sorry, I overlooked it. > > I would remove unneeded CRYPTO_MINALIGN and get the alignment from the > target structure: > char aead_req_data[sizeof(struct aead_request) + > crypto_aead_reqsize(tfm)] > __attribute__((__aligned__(__alignof__(struct aead_request)))); Even better. Will resubmit. Behan
diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c index 7c7df47..3317578 100644 --- a/net/mac80211/aes_ccm.c +++ b/net/mac80211/aes_ccm.c @@ -23,12 +23,14 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, u8 *data, size_t data_len, u8 *mic) { struct scatterlist assoc, pt, ct[2]; - struct { - struct aead_request req; - u8 priv[crypto_aead_reqsize(tfm)]; - } aead_req; - memset(&aead_req, 0, sizeof(aead_req)); + char aead_req_data[sizeof(struct aead_request) + + crypto_aead_reqsize(tfm) + + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; + + struct aead_request *aead_req = (void *) aead_req_data; + + memset(aead_req, 0, sizeof(aead_req_data)); sg_init_one(&pt, data, data_len); sg_init_one(&assoc, &aad[2], be16_to_cpup((__be16 *)aad)); @@ -36,23 +38,25 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, sg_set_buf(&ct[0], data, data_len); sg_set_buf(&ct[1], mic, IEEE80211_CCMP_MIC_LEN); - aead_request_set_tfm(&aead_req.req, tfm); - aead_request_set_assoc(&aead_req.req, &assoc, assoc.length); - aead_request_set_crypt(&aead_req.req, &pt, ct, data_len, b_0); + aead_request_set_tfm(aead_req, tfm); + aead_request_set_assoc(aead_req, &assoc, assoc.length); + aead_request_set_crypt(aead_req, &pt, ct, data_len, b_0); - crypto_aead_encrypt(&aead_req.req); + crypto_aead_encrypt(aead_req); } int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, u8 *data, size_t data_len, u8 *mic) { struct scatterlist assoc, pt, ct[2]; - struct { - struct aead_request req; - u8 priv[crypto_aead_reqsize(tfm)]; - } aead_req; - memset(&aead_req, 0, sizeof(aead_req)); + char aead_req_data[sizeof(struct aead_request) + + crypto_aead_reqsize(tfm) + + CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR; + + struct aead_request *aead_req = (void *) aead_req_data; + + memset(aead_req, 0, sizeof(aead_req_data)); sg_init_one(&pt, data, data_len); sg_init_one(&assoc, &aad[2], be16_to_cpup((__be16 *)aad)); @@ -60,12 +64,12 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad, sg_set_buf(&ct[0], data, data_len); sg_set_buf(&ct[1], mic, IEEE80211_CCMP_MIC_LEN); - aead_request_set_tfm(&aead_req.req, tfm); - aead_request_set_assoc(&aead_req.req, &assoc, assoc.length); - aead_request_set_crypt(&aead_req.req, ct, &pt, + aead_request_set_tfm(aead_req, tfm); + aead_request_set_assoc(aead_req, &assoc, assoc.length); + aead_request_set_crypt(aead_req, ct, &pt, data_len + IEEE80211_CCMP_MIC_LEN, b_0); - return crypto_aead_decrypt(&aead_req.req); + return crypto_aead_decrypt(aead_req); } struct crypto_aead *ieee80211_aes_key_setup_encrypt(const u8 key[])