public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: ipv6: xfrm6_state: remove VLA usage
@ 2018-03-09 12:21 Andreas Christoforou
  2018-03-09 12:35 ` Steffen Klassert
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Andreas Christoforou @ 2018-03-09 12:21 UTC (permalink / raw)
  To: keescook
  Cc: kernel-hardening, Andreas Christoforou, Steffen Klassert,
	Herbert Xu, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
	netdev, linux-kernel

The kernel would like to have all stack VLA usage removed[1].

Signed-off-by: Andreas Christoforou <andreaschristofo@gmail.com>
---
 net/ipv6/xfrm6_state.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
index b15075a..45c0d98 100644
--- a/net/ipv6/xfrm6_state.c
+++ b/net/ipv6/xfrm6_state.c
@@ -62,7 +62,12 @@ __xfrm6_sort(void **dst, void **src, int n, int (*cmp)(void *p), int maxclass)
 {
 	int i;
 	int class[XFRM_MAX_DEPTH];
-	int count[maxclass];
+	int *count;
+
+	count = kcalloc(maxclass + 1, sizeof(*count), GFP_KERNEL);
+
+	if (!count)
+		return -ENOMEM;
 
 	memset(count, 0, sizeof(count));
 
@@ -80,6 +85,7 @@ __xfrm6_sort(void **dst, void **src, int n, int (*cmp)(void *p), int maxclass)
 		src[i] = NULL;
 	}
 
+	kfree(count);
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ipv6: xfrm6_state: remove VLA usage
  2018-03-09 12:21 [PATCH] net: ipv6: xfrm6_state: remove VLA usage Andreas Christoforou
@ 2018-03-09 12:35 ` Steffen Klassert
  2018-03-09 12:49 ` Mathias Krause
  2018-03-09 18:35 ` Sergei Shtylyov
  2 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2018-03-09 12:35 UTC (permalink / raw)
  To: Andreas Christoforou
  Cc: keescook, kernel-hardening, Herbert Xu, David S. Miller,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, linux-kernel

On Fri, Mar 09, 2018 at 02:21:46PM +0200, Andreas Christoforou wrote:
> The kernel would like to have all stack VLA usage removed[1].
> 
> Signed-off-by: Andreas Christoforou <andreaschristofo@gmail.com>

Can you please explain why you want this change?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ipv6: xfrm6_state: remove VLA usage
  2018-03-09 12:21 [PATCH] net: ipv6: xfrm6_state: remove VLA usage Andreas Christoforou
  2018-03-09 12:35 ` Steffen Klassert
@ 2018-03-09 12:49 ` Mathias Krause
  2018-03-09 13:02   ` Steffen Klassert
  2018-03-09 18:35 ` Sergei Shtylyov
  2 siblings, 1 reply; 6+ messages in thread
From: Mathias Krause @ 2018-03-09 12:49 UTC (permalink / raw)
  To: Andreas Christoforou
  Cc: Kees Cook, kernel-hardening, Steffen Klassert, Herbert Xu,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	linux-kernel

On 9 March 2018 at 13:21, Andreas Christoforou
<andreaschristofo@gmail.com> wrote:
> The kernel would like to have all stack VLA usage removed[1].
>
> Signed-off-by: Andreas Christoforou <andreaschristofo@gmail.com>
> ---
>  net/ipv6/xfrm6_state.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
> index b15075a..45c0d98 100644
> --- a/net/ipv6/xfrm6_state.c
> +++ b/net/ipv6/xfrm6_state.c
> @@ -62,7 +62,12 @@ __xfrm6_sort(void **dst, void **src, int n, int (*cmp)(void *p), int maxclass)
>  {
>         int i;
>         int class[XFRM_MAX_DEPTH];
> -       int count[maxclass];
> +       int *count;
> +
> +       count = kcalloc(maxclass + 1, sizeof(*count), GFP_KERNEL);
> +
> +       if (!count)
> +               return -ENOMEM;
>
>         memset(count, 0, sizeof(count));
>
> @@ -80,6 +85,7 @@ __xfrm6_sort(void **dst, void **src, int n, int (*cmp)(void *p), int maxclass)
>                 src[i] = NULL;
>         }
>
> +       kfree(count);
>         return 0;
>  }

Instead of dynamically allocating and freeing memory here, shouldn't
we just get rid of the maxclass parameter and use XFRM_MAX_DEPTH as
size for the count[] array, too?

Cheers,
Mathias

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ipv6: xfrm6_state: remove VLA usage
  2018-03-09 12:49 ` Mathias Krause
@ 2018-03-09 13:02   ` Steffen Klassert
  2018-03-09 13:49     ` Andreas Christoforou
  0 siblings, 1 reply; 6+ messages in thread
From: Steffen Klassert @ 2018-03-09 13:02 UTC (permalink / raw)
  To: Mathias Krause
  Cc: Andreas Christoforou, Kees Cook, kernel-hardening, Herbert Xu,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	linux-kernel

On Fri, Mar 09, 2018 at 01:49:07PM +0100, Mathias Krause wrote:
> On 9 March 2018 at 13:21, Andreas Christoforou
> <andreaschristofo@gmail.com> wrote:
> > The kernel would like to have all stack VLA usage removed[1].
> >
> > Signed-off-by: Andreas Christoforou <andreaschristofo@gmail.com>
> > ---
> >  net/ipv6/xfrm6_state.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
> > index b15075a..45c0d98 100644
> > --- a/net/ipv6/xfrm6_state.c
> > +++ b/net/ipv6/xfrm6_state.c
> > @@ -62,7 +62,12 @@ __xfrm6_sort(void **dst, void **src, int n, int (*cmp)(void *p), int maxclass)
> >  {
> >         int i;
> >         int class[XFRM_MAX_DEPTH];
> > -       int count[maxclass];
> > +       int *count;
> > +
> > +       count = kcalloc(maxclass + 1, sizeof(*count), GFP_KERNEL);
> > +
> > +       if (!count)
> > +               return -ENOMEM;
> >
> >         memset(count, 0, sizeof(count));
> >
> > @@ -80,6 +85,7 @@ __xfrm6_sort(void **dst, void **src, int n, int (*cmp)(void *p), int maxclass)
> >                 src[i] = NULL;
> >         }
> >
> > +       kfree(count);
> >         return 0;
> >  }
> 
> Instead of dynamically allocating and freeing memory here, shouldn't
> we just get rid of the maxclass parameter and use XFRM_MAX_DEPTH as
> size for the count[] array, too?

Right, that's the way to go. Aside from that, allocating
with GFP_KERNEL is definitely wrong here.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ipv6: xfrm6_state: remove VLA usage
  2018-03-09 13:02   ` Steffen Klassert
@ 2018-03-09 13:49     ` Andreas Christoforou
  0 siblings, 0 replies; 6+ messages in thread
From: Andreas Christoforou @ 2018-03-09 13:49 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Mathias Krause, Kees Cook, kernel-hardening, Herbert Xu,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1646 bytes --]

Right, thank you for your feedback I will create a new patch.

Kind Regards,
Andreas Christoforou

On Mar 9, 2018 15:02, "Steffen Klassert" <steffen.klassert@secunet.com>
wrote:

On Fri, Mar 09, 2018 at 01:49:07PM +0100, Mathias Krause wrote:
> On 9 March 2018 at 13:21, Andreas Christoforou
> <andreaschristofo@gmail.com> wrote:
> > The kernel would like to have all stack VLA usage removed[1].
> >
> > Signed-off-by: Andreas Christoforou <andreaschristofo@gmail.com>
> > ---
> >  net/ipv6/xfrm6_state.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
> > index b15075a..45c0d98 100644
> > --- a/net/ipv6/xfrm6_state.c
> > +++ b/net/ipv6/xfrm6_state.c
> > @@ -62,7 +62,12 @@ __xfrm6_sort(void **dst, void **src, int n, int
(*cmp)(void *p), int maxclass)
> >  {
> >         int i;
> >         int class[XFRM_MAX_DEPTH];
> > -       int count[maxclass];
> > +       int *count;
> > +
> > +       count = kcalloc(maxclass + 1, sizeof(*count), GFP_KERNEL);
> > +
> > +       if (!count)
> > +               return -ENOMEM;
> >
> >         memset(count, 0, sizeof(count));
> >
> > @@ -80,6 +85,7 @@ __xfrm6_sort(void **dst, void **src, int n, int
(*cmp)(void *p), int maxclass)
> >                 src[i] = NULL;
> >         }
> >
> > +       kfree(count);
> >         return 0;
> >  }
>
> Instead of dynamically allocating and freeing memory here, shouldn't
> we just get rid of the maxclass parameter and use XFRM_MAX_DEPTH as
> size for the count[] array, too?

Right, that's the way to go. Aside from that, allocating
with GFP_KERNEL is definitely wrong here.

[-- Attachment #2: Type: text/html, Size: 2684 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: ipv6: xfrm6_state: remove VLA usage
  2018-03-09 12:21 [PATCH] net: ipv6: xfrm6_state: remove VLA usage Andreas Christoforou
  2018-03-09 12:35 ` Steffen Klassert
  2018-03-09 12:49 ` Mathias Krause
@ 2018-03-09 18:35 ` Sergei Shtylyov
  2 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2018-03-09 18:35 UTC (permalink / raw)
  To: Andreas Christoforou, keescook
  Cc: kernel-hardening, Steffen Klassert, Herbert Xu, David S. Miller,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, linux-kernel

Hello!

On 03/09/2018 03:21 PM, Andreas Christoforou wrote:

> The kernel would like to have all stack VLA usage removed[1].
> 
> Signed-off-by: Andreas Christoforou <andreaschristofo@gmail.com>
> ---
>  net/ipv6/xfrm6_state.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
> index b15075a..45c0d98 100644
> --- a/net/ipv6/xfrm6_state.c
> +++ b/net/ipv6/xfrm6_state.c
> @@ -62,7 +62,12 @@ __xfrm6_sort(void **dst, void **src, int n, int (*cmp)(void *p), int maxclass)
>  {
>  	int i;
>  	int class[XFRM_MAX_DEPTH];
> -	int count[maxclass];
> +	int *count;
> +
> +	count = kcalloc(maxclass + 1, sizeof(*count), GFP_KERNEL);
> +

  Empty line not needed here.

> +	if (!count)
> +		return -ENOMEM;
>  
>  	memset(count, 0, sizeof(count));
>  
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-09 18:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-09 12:21 [PATCH] net: ipv6: xfrm6_state: remove VLA usage Andreas Christoforou
2018-03-09 12:35 ` Steffen Klassert
2018-03-09 12:49 ` Mathias Krause
2018-03-09 13:02   ` Steffen Klassert
2018-03-09 13:49     ` Andreas Christoforou
2018-03-09 18:35 ` Sergei Shtylyov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox