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

Message ID 1395199933-4686-1-git-send-email-behanw@converseincode.com
State New
Headers show

Commit Message

Behan Webster March 19, 2014, 3:32 a.m.
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(-)

Comments

Johannes Berg March 19, 2014, 1:51 p.m. | #1
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

--
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/
David Laight March 19, 2014, 2:09 p.m. | #2
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
Behan Webster March 19, 2014, 2:25 p.m. | #3
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

Patch hide | download patch | download mbox

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