| Message ID | 1520598106-3271-1-git-send-email-andreaschristofo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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?
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
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.
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.
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
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; }
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(-)