Patchwork [v2,iproute] ipxfrm: use alloca to allocate stack space

login
register
mail settings
Submitter Mike Frysinger
Date Jan. 18, 2013, 4 a.m.
Message ID <1358481650-20816-1-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/213450/
State Accepted
Delegated to: stephen hemminger
Headers show

Comments

Mike Frysinger - Jan. 18, 2013, 4 a.m.
Clang doesn't support the gcc extension for embeddeding flexible arrays
inside of structures.  Use the slightly more portable alloca().

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- use alloca rather than flexible arrays in structures

 ip/ipxfrm.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)
David Laight - Jan. 18, 2013, 10 a.m.
> Clang doesn't support the gcc extension for embeddeding flexible arrays
> inside of structures.  Use the slightly more portable alloca().
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v2
> 	- use alloca rather than flexible arrays in structures
...
> +#include <alloca.h>

I'm not sure what happens in Linux, but alloca.h is likely
to try to use the libc version of alloca().
While that worked 20 years ago, these days compilers then
to use %sp relative addressing so you must only use the
compiler builtin alloca() code.

With gcc, compiling with -std=c89 (I think that is the one)
causes the builtin alloca() not to be defined.
By default alloca() is part of the language and the header
isn't needed.

Use of alloca() also stops some of the stack overwrite
protection being enabled.

In this case I suspect malloc() and free() is best.
(Or change the called function ...)

	David



--
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
Mike Frysinger - Jan. 18, 2013, 5:41 p.m.
On Friday 18 January 2013 05:00:53 David Laight wrote:
> > Clang doesn't support the gcc extension for embeddeding flexible arrays
> > inside of structures.  Use the slightly more portable alloca().
> > 
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> > v2
> > 	- use alloca rather than flexible arrays in structures
> ...
> > +#include <alloca.h>
> 
> I'm not sure what happens in Linux, but alloca.h is likely
> to try to use the libc version of alloca().
> While that worked 20 years ago, these days compilers then
> to use %sp relative addressing so you must only use the
> compiler builtin alloca() code.
> 
> With gcc, compiling with -std=c89 (I think that is the one)
> causes the builtin alloca() not to be defined.
> By default alloca() is part of the language and the header
> isn't needed.
> 
> Use of alloca() also stops some of the stack overwrite
> protection being enabled.

seems like a short coming of the C library then ?  with glibc, if it detects 
the active compiler is gcc, it will define alloca() to the gcc builtin.
-mike

Patch

diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index c7b3420..dda4a7a 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -25,6 +25,7 @@ 
  *	Masahide NAKAMURA @USAGI
  */
 
+#include <alloca.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -555,16 +556,13 @@  static inline void xfrm_algo_print(struct xfrm_algo *algo, int type, int len,
 static void xfrm_aead_print(struct xfrm_algo_aead *algo, int len,
 			    FILE *fp, const char *prefix)
 {
-	struct {
-		struct xfrm_algo algo;
-		char key[algo->alg_key_len / 8];
-	} base;
+	struct xfrm_algo *base_algo = alloca(sizeof(*base_algo) + algo->alg_key_len / 8);
 
-	memcpy(base.algo.alg_name, algo->alg_name, sizeof(base.algo.alg_name));
-	base.algo.alg_key_len = algo->alg_key_len;
-	memcpy(base.algo.alg_key, algo->alg_key, algo->alg_key_len / 8);
+	memcpy(base_algo->alg_name, algo->alg_name, sizeof(base_algo->alg_name));
+	base_algo->alg_key_len = algo->alg_key_len;
+	memcpy(base_algo->alg_key, algo->alg_key, algo->alg_key_len / 8);
 
-	__xfrm_algo_print(&base.algo, XFRMA_ALG_AEAD, len, fp, prefix, 0);
+	__xfrm_algo_print(base_algo, XFRMA_ALG_AEAD, len, fp, prefix, 0);
 
 	fprintf(fp, " %d", algo->alg_icv_len);
 
@@ -574,16 +572,13 @@  static void xfrm_aead_print(struct xfrm_algo_aead *algo, int len,
 static void xfrm_auth_trunc_print(struct xfrm_algo_auth *algo, int len,
 				  FILE *fp, const char *prefix)
 {
-	struct {
-		struct xfrm_algo algo;
-		char key[algo->alg_key_len / 8];
-	} base;
+	struct xfrm_algo *base_algo = alloca(sizeof(*base_algo) + algo->alg_key_len / 8);
 
-	memcpy(base.algo.alg_name, algo->alg_name, sizeof(base.algo.alg_name));
-	base.algo.alg_key_len = algo->alg_key_len;
-	memcpy(base.algo.alg_key, algo->alg_key, algo->alg_key_len / 8);
+	memcpy(base_algo->alg_name, algo->alg_name, sizeof(base_algo->alg_name));
+	base_algo->alg_key_len = algo->alg_key_len;
+	memcpy(base_algo->alg_key, algo->alg_key, algo->alg_key_len / 8);
 
-	__xfrm_algo_print(&base.algo, XFRMA_ALG_AUTH_TRUNC, len, fp, prefix, 0);
+	__xfrm_algo_print(base_algo, XFRMA_ALG_AUTH_TRUNC, len, fp, prefix, 0);
 
 	fprintf(fp, " %d", algo->alg_trunc_len);