[PATCH] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[PATCH] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[behanw]

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.

struct {
	struct aead_request     req;
	u8                      priv[crypto_aead_reqsize(tfm)];
} aead_req;

This patch instead allocates the appropriate amount of memory using an char
array.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>
Signed-off-by: Behan Webster <behanw@converseincode.com>
Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com>
Signed-off-by: Mark Charlebois <charlebm@gmail.com>
---
 net/mac80211/aes_ccm.c | 44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
index 7c7df47..9612e95 100644
--- a/net/mac80211/aes_ccm.c
+++ b/net/mac80211/aes_ccm.c
@@ -23,12 +23,16 @@ 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
+		= (struct aead_request *) aead_req_data;
+
+	memset(&aead_req_data, 0, (sizeof(struct aead_request)+
+		crypto_aead_reqsize(tfm) + CRYPTO_MINALIGN));
 
 	sg_init_one(&pt, data, data_len);
 	sg_init_one(&assoc, &aad[2], be16_to_cpup((__be16 *)aad));
@@ -36,23 +40,27 @@ 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
+		= (struct aead_request *) aead_req_data;
+
+	memset(&aead_req_data, 0, (sizeof(struct aead_request)+
+		crypto_aead_reqsize(tfm) + CRYPTO_MINALIGN));
 
 	sg_init_one(&pt, data, data_len);
 	sg_init_one(&assoc, &aad[2], be16_to_cpup((__be16 *)aad));
@@ -60,12 +68,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[])
-- 
1.8.3.2

Re: [PATCH] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Johannes Berg]

On Thu, 2014-03-06 at 11:52 -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.

Fine, but

> +	char aead_req_data[sizeof(struct aead_request)
> +				+ crypto_aead_reqsize(tfm)
> +				+ CRYPTO_MINALIGN] CRYPTO_MINALIGN_ATTR;

You really should be using kernel coding style, which changes
indentation and has the + on the previous line.

> +	struct aead_request *aead_req
> +		= (struct aead_request *) aead_req_data;

(void *) is perfectly find and it'll probably fit on one line then.

> +	memset(&aead_req_data, 0, (sizeof(struct aead_request)+
> +		crypto_aead_reqsize(tfm) + CRYPTO_MINALIGN));

You don't need the size calculation again, you can use
sizeof(aead_req_data)

johannes

[PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[behanw]

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.

struct {
	struct aead_request     req;
	u8                      priv[crypto_aead_reqsize(tfm)];
} aead_req;

This patch instead allocates the appropriate amount of memory using an char
array.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>
Signed-off-by: Behan Webster <behanw@converseincode.com>
Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com>
Signed-off-by: Mark Charlebois <charlebm@gmail.com>
---
 net/mac80211/aes_ccm.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

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[])
-- 
1.8.3.2

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Joe Perches]

On Fri, 2014-03-07 at 17:26 -0800, behanw-k/hB3zQhLwledRVtV/plodBPR1lH4CV8@public.gmane.org wrote:
> From: Jan-Simon Möller <dl9pf-Mmb7MZpHnFY@public.gmane.org>
> 
> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Behan Webster]

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

-- 
Behan Webster
behanw@converseincode.com

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Joe Perches]

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-k/hB3zQhLwledRVtV/plodBPR1lH4CV8@public.gmane.org wrote:
> >> From: Jan-Simon Möller <dl9pf-Mmb7MZpHnFY@public.gmane.org>
> >>
> >> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Behan Webster]

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

-- 
Behan Webster
behanw@converseincode.com

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Stanislaw Gruszka]

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

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Behan Webster]

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

-- 
Behan Webster
behanw@converseincode.com

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Sergei Antonov]

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.

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Behan Webster]

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

-- 
Behan Webster
behanw@converseincode.com

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[PaX Team]

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.

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Sergei Antonov]

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))));

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Jan-Simon Möller]

On 9. März 2014 00:00:19 MEZ, Sergei Antonov <saproj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>On 8 March 2014 23:01, PaX Team <pageexec-Y8qEzhMunLyT9ig0jae3mg@public.gmane.org> 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



-- 
Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Behan Webster]

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

-- 
Behan Webster
behanw@converseincode.com

[PATCH v3] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[behanw]

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.

struct {
	struct aead_request     req;
	u8                      priv[crypto_aead_reqsize(tfm)];
} aead_req;

This patch instead allocates the appropriate amount of memory using an char
array.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>
Signed-off-by: Behan Webster <behanw@converseincode.com>
Signed-off-by: Vinícius Tinti <viniciustinti@gmail.com>
Signed-off-by: Mark Charlebois <charlebm@gmail.com>
---
 net/mac80211/aes_ccm.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
index 7c7df47..20521f9 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)]
+		__aligned(__alignof__(struct aead_request));
+
+	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[])
-- 
1.8.3.2

Re: [PATCH v3] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Johannes Berg]

I'm confused.

On Tue, 2014-03-18 at 20:32 -0700, behanw@converseincode.com wrote:


>  	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)]
> +		__aligned(__alignof__(struct aead_request));

This looks fine, though I'd argue the blank lines before/after it
shouldn't be there, and the indentation should be a bit different, but I
was willing to clean that up.

>  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;

But why does the second instance use a completely different size/align?

johannes

Re: [PATCH v3] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Behan Webster]

On 03/19/14 06:51, Johannes Berg wrote:
> I'm confused.
>
> On Tue, 2014-03-18 at 20:32 -0700, behanw@converseincode.com wrote:
>
>
>>   	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)]
>> +		__aligned(__alignof__(struct aead_request));
> This looks fine, though I'd argue the blank lines before/after it
> shouldn't be there, and the indentation should be a bit different, but I
> was willing to clean that up.
Will fix.

>>   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;
> But why does the second instance use a completely different size/align?
Because I neglected to update it in both places. Sorry. Will fix.

Behan

-- 
Behan Webster
behanw@converseincode.com

[PATCH v4] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[behanw-k/hB3zQhLwledRVtV/plodBPR1lH4CV8]

From: Jan-Simon Möller <dl9pf-Mmb7MZpHnFY@public.gmane.org>

Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
compliant equivalent. This is the original VLAIS struct.

struct {
	struct aead_request     req;
	u8                      priv[crypto_aead_reqsize(tfm)];
} aead_req;

This patch instead allocates the appropriate amount of memory using an char
array.

The new code can be compiled with both gcc and clang.

Signed-off-by: Jan-Simon Möller <dl9pf-Mmb7MZpHnFY@public.gmane.org>
Signed-off-by: Behan Webster <behanw-k/hB3zQhLwledRVtV/plodBPR1lH4CV8@public.gmane.org>
Signed-off-by: Vinícius Tinti <viniciustinti-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Mark Charlebois <charlebm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 net/mac80211/aes_ccm.c | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
index 7c7df47..71e2abd 100644
--- a/net/mac80211/aes_ccm.c
+++ b/net/mac80211/aes_ccm.c
@@ -23,12 +23,12 @@ 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)]
+				__aligned(__alignof__(struct aead_request));
+	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 +36,22 @@ 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)]
+				__aligned(__alignof__(struct aead_request));
+	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 +59,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[])
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[Johannes Berg]

On Thu, 2014-03-20 at 23:39 -0700, 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.
> 
> struct {
> 	struct aead_request     req;
> 	u8                      priv[crypto_aead_reqsize(tfm)];
> } aead_req;
> 
> This patch instead allocates the appropriate amount of memory using an char
> array.
> 
> The new code can be compiled with both gcc and clang.

Applied.

johannes

RE: [PATCH v3] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[David Laight]

From: behanw@converseincode.com
> Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99
> compliant equivalent. This is the original VLAIS struct.
> 
> struct {
> 	struct aead_request     req;
> 	u8                      priv[crypto_aead_reqsize(tfm)];
> } aead_req;
> 
> This patch instead allocates the appropriate amount of memory using an char
> array.
> 
> The new code can be compiled with both gcc and clang.
> 
> Signed-off-by: Jan-Simon Mller <dl9pf@gmx.de>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> Signed-off-by: Vincius Tinti <viniciustinti@gmail.com>
> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
> ---
>  net/mac80211/aes_ccm.c | 40 ++++++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
> index 7c7df47..20521f9 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)]
> +		__aligned(__alignof__(struct aead_request));
> +
> +	struct aead_request *aead_req = (void *) aead_req_data;
> +
> +	memset(aead_req, 0, sizeof(aead_req_data));

It seems to me that the underlying function(s) ought to be changed so that
this isn't needed.
Passing an extra parameter would probably cost very little.

	David

Re: [PATCH v2] mac80211: LLVMLinux: Remove VLAIS usage from mac80211

[David Miller]

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.