diff mbox

xfrm: use ISO C standard for array in linux/xfrm.h

Message ID 1350402153-3889-1-git-send-email-nicolas.dichtel@6wind.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Nicolas Dichtel Oct. 16, 2012, 3:42 p.m. UTC
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Use the ISO C standard compliant form instead of the gcc extension.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/uapi/linux/xfrm.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Steffen Klassert Oct. 17, 2012, 5:43 a.m. UTC | #1
On Tue, Oct 16, 2012 at 05:42:33PM +0200, nicolas.dichtel@6wind.com wrote:
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> 
> Use the ISO C standard compliant form instead of the gcc extension.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>  include/uapi/linux/xfrm.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
> index 28e493b..6a6a605 100644
> --- a/include/uapi/linux/xfrm.h
> +++ b/include/uapi/linux/xfrm.h
> @@ -30,7 +30,7 @@ struct xfrm_sec_ctx {
>  	__u8	ctx_alg;
>  	__u16	ctx_len;
>  	__u32	ctx_sid;
> -	char	ctx_str[0];
> +	char	ctx_str[];

Hm, what's the benefit of such a change? The kernel source is full
of these foo[0] type arrays. I don't see a reason to change this
just for xfrm.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Oct. 17, 2012, 6:16 a.m. UTC | #2
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 17 Oct 2012 07:43:43 +0200

> On Tue, Oct 16, 2012 at 05:42:33PM +0200, nicolas.dichtel@6wind.com wrote:
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> 
>> Use the ISO C standard compliant form instead of the gcc extension.
>> 
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> ---
>>  include/uapi/linux/xfrm.h | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>> 
>> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
>> index 28e493b..6a6a605 100644
>> --- a/include/uapi/linux/xfrm.h
>> +++ b/include/uapi/linux/xfrm.h
>> @@ -30,7 +30,7 @@ struct xfrm_sec_ctx {
>>  	__u8	ctx_alg;
>>  	__u16	ctx_len;
>>  	__u32	ctx_sid;
>> -	char	ctx_str[0];
>> +	char	ctx_str[];
> 
> Hm, what's the benefit of such a change? The kernel source is full
> of these foo[0] type arrays. I don't see a reason to change this
> just for xfrm.

Agreed.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Dichtel Oct. 17, 2012, 7:50 a.m. UTC | #3
Le 17/10/2012 07:43, Steffen Klassert a écrit :
> On Tue, Oct 16, 2012 at 05:42:33PM +0200, nicolas.dichtel@6wind.com wrote:
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>
>> Use the ISO C standard compliant form instead of the gcc extension.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> ---
>>   include/uapi/linux/xfrm.h | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
>> index 28e493b..6a6a605 100644
>> --- a/include/uapi/linux/xfrm.h
>> +++ b/include/uapi/linux/xfrm.h
>> @@ -30,7 +30,7 @@ struct xfrm_sec_ctx {
>>   	__u8	ctx_alg;
>>   	__u16	ctx_len;
>>   	__u32	ctx_sid;
>> -	char	ctx_str[0];
>> +	char	ctx_str[];
>
> Hm, what's the benefit of such a change? The kernel source is full
> of these foo[0] type arrays. I don't see a reason to change this
> just for xfrm.
>
We got a problem with "ip xfrm state add" when compiled with gcc 4.4.6.
Error was "*** buffer overflow detected ***: ip terminated", because when we try 
to copy the key in struct xfrm_algo, the function strncpy() calls some builtin 
checks about the size of the destination buffer, which is 0. With the standard 
notation, there is no problem.

With gcc 4.7.0, there is no problem, I don't know exactly which version of gcc 
triggers the problem.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steffen Klassert Oct. 17, 2012, 8:27 a.m. UTC | #4
On Wed, Oct 17, 2012 at 09:50:58AM +0200, Nicolas Dichtel wrote:
> We got a problem with "ip xfrm state add" when compiled with gcc 4.4.6.
> Error was "*** buffer overflow detected ***: ip terminated", because
> when we try to copy the key in struct xfrm_algo, the function
> strncpy() calls some builtin checks about the size of the
> destination buffer, which is 0. With the standard notation, there is
> no problem.

This is likely to be a gcc FORITFY bug. Try to build iproute2 with
the -D_FORTIFY_SOURCE=0 flag.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Dichtel Oct. 17, 2012, 9:03 a.m. UTC | #5
Le 17/10/2012 10:27, Steffen Klassert a écrit :
> On Wed, Oct 17, 2012 at 09:50:58AM +0200, Nicolas Dichtel wrote:
>> We got a problem with "ip xfrm state add" when compiled with gcc 4.4.6.
>> Error was "*** buffer overflow detected ***: ip terminated", because
>> when we try to copy the key in struct xfrm_algo, the function
>> strncpy() calls some builtin checks about the size of the
>> destination buffer, which is 0. With the standard notation, there is
>> no problem.
>
> This is likely to be a gcc FORITFY bug. Try to build iproute2 with
> the -D_FORTIFY_SOURCE=0 flag.
>
Right, it works. Thank you.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index 28e493b..6a6a605 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -30,7 +30,7 @@  struct xfrm_sec_ctx {
 	__u8	ctx_alg;
 	__u16	ctx_len;
 	__u32	ctx_sid;
-	char	ctx_str[0];
+	char	ctx_str[];
 };
 
 /* Security Context Domains of Interpretation */
@@ -93,27 +93,27 @@  struct xfrm_replay_state_esn {
 	__u32		oseq_hi;
 	__u32		seq_hi;
 	__u32		replay_window;
-	__u32		bmp[0];
+	__u32		bmp[];
 };
 
 struct xfrm_algo {
 	char		alg_name[64];
 	unsigned int	alg_key_len;    /* in bits */
-	char		alg_key[0];
+	char		alg_key[];
 };
 
 struct xfrm_algo_auth {
 	char		alg_name[64];
 	unsigned int	alg_key_len;    /* in bits */
 	unsigned int	alg_trunc_len;  /* in bits */
-	char		alg_key[0];
+	char		alg_key[];
 };
 
 struct xfrm_algo_aead {
 	char		alg_name[64];
 	unsigned int	alg_key_len;	/* in bits */
 	unsigned int	alg_icv_len;	/* in bits */
-	char		alg_key[0];
+	char		alg_key[];
 };
 
 struct xfrm_stats {