All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [iproute] ipxfrm: simplify algo code a bit
@ 2013-01-18  2:39 Mike Frysinger
  2013-01-18  3:26 ` David Miller
  2013-01-18  4:00 ` [PATCH v2] [iproute] ipxfrm: use alloca to allocate stack space Mike Frysinger
  0 siblings, 2 replies; 6+ messages in thread
From: Mike Frysinger @ 2013-01-18  2:39 UTC (permalink / raw)
  To: stephen.hemminger, netdev

The current code sets up a structure with xfrm_algo embedded in it, but
doesn't use the supplemental key field.  Drop it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
note: i don't have a system where this code runs (probably missing options in
my kernel).  i checked xfrm_aead_print a bit but couldn't quite figure it out.
it doesn't seem like it needs that trailing space.

 ip/ipxfrm.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index c7b3420..bd313e6 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -555,16 +555,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;
 
-	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 +571,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;
 
-	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);
 
-- 
1.8.0.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] [iproute] ipxfrm: simplify algo code a bit
  2013-01-18  2:39 [PATCH] [iproute] ipxfrm: simplify algo code a bit Mike Frysinger
@ 2013-01-18  3:26 ` David Miller
  2013-01-18  4:00   ` Mike Frysinger
  2013-01-18  4:00 ` [PATCH v2] [iproute] ipxfrm: use alloca to allocate stack space Mike Frysinger
  1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2013-01-18  3:26 UTC (permalink / raw)
  To: vapier; +Cc: stephen.hemminger, netdev

From: Mike Frysinger <vapier@gentoo.org>
Date: Thu, 17 Jan 2013 21:39:06 -0500

> The current code sets up a structure with xfrm_algo embedded in it, but
> doesn't use the supplemental key field.  Drop it.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

It's for the storage of the zero length array at the end of
xfrm_algo.

Don't change code you don't understand.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] [iproute] ipxfrm: simplify algo code a bit
  2013-01-18  3:26 ` David Miller
@ 2013-01-18  4:00   ` Mike Frysinger
  0 siblings, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2013-01-18  4:00 UTC (permalink / raw)
  To: David Miller; +Cc: stephen.hemminger, netdev

[-- Attachment #1: Type: Text/Plain, Size: 529 bytes --]

On Thursday 17 January 2013 22:26:46 David Miller wrote:
> From: Mike Frysinger <vapier@gentoo.org>
> Date: Thu, 17 Jan 2013 21:39:06 -0500
> > The current code sets up a structure with xfrm_algo embedded in it, but
> > doesn't use the supplemental key field.  Drop it.
> 
> It's for the storage of the zero length array at the end of
> xfrm_algo.

ok, so we can use alloca instead

> Don't change code you don't understand.

so you're just going to be pointlessly hostile everywhere ?  stop wasting 
time.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] [iproute] ipxfrm: use alloca to allocate stack space
  2013-01-18  2:39 [PATCH] [iproute] ipxfrm: simplify algo code a bit Mike Frysinger
  2013-01-18  3:26 ` David Miller
@ 2013-01-18  4:00 ` Mike Frysinger
  2013-01-18 10:00   ` David Laight
  1 sibling, 1 reply; 6+ messages in thread
From: Mike Frysinger @ 2013-01-18  4:00 UTC (permalink / raw)
  To: stephen.hemminger, netdev

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

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* RE: [PATCH v2] [iproute] ipxfrm: use alloca to allocate stack space
  2013-01-18  4:00 ` [PATCH v2] [iproute] ipxfrm: use alloca to allocate stack space Mike Frysinger
@ 2013-01-18 10:00   ` David Laight
  2013-01-18 17:41     ` Mike Frysinger
  0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2013-01-18 10:00 UTC (permalink / raw)
  To: Mike Frysinger, stephen.hemminger, netdev

> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] [iproute] ipxfrm: use alloca to allocate stack space
  2013-01-18 10:00   ` David Laight
@ 2013-01-18 17:41     ` Mike Frysinger
  0 siblings, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2013-01-18 17:41 UTC (permalink / raw)
  To: David Laight; +Cc: stephen.hemminger, netdev

[-- Attachment #1: Type: Text/Plain, Size: 1090 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-18 17:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-18  2:39 [PATCH] [iproute] ipxfrm: simplify algo code a bit Mike Frysinger
2013-01-18  3:26 ` David Miller
2013-01-18  4:00   ` Mike Frysinger
2013-01-18  4:00 ` [PATCH v2] [iproute] ipxfrm: use alloca to allocate stack space Mike Frysinger
2013-01-18 10:00   ` David Laight
2013-01-18 17:41     ` Mike Frysinger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.