diff mbox series

[net] wireguard: allowedips: fix compilation warning for stack limit exceeded

Message ID 20230526204134.29058-1-ansuelsmth@gmail.com
State New
Headers
Series [net] wireguard: allowedips: fix compilation warning for stack limit exceeded |

Commit Message

Christian Marangi May 26, 2023, 8:41 p.m. UTC 
  On some arch (for example IPQ8074) and other with
KERNEL_STACKPROTECTOR_STRONG enabled, the following compilation error is
triggered:
drivers/net/wireguard/allowedips.c: In function 'root_remove_peer_lists':
drivers/net/wireguard/allowedips.c:80:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
   80 | }
      | ^
drivers/net/wireguard/allowedips.c: In function 'root_free_rcu':
drivers/net/wireguard/allowedips.c:67:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
   67 | }
      | ^
cc1: all warnings being treated as errors

Since these are free function and returns void, using function that can
fail is not ideal since an error would result in data not freed.
Since the free are under RCU lock, we can allocate the required stack
array as static outside the function and memset when needed.
This effectively fix the stack frame warning without changing how the
function work.

Fixes: Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/net/wireguard/allowedips.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Christian Marangi May 27, 2023, 12:47 p.m. UTC | #1 
On Sat, May 27, 2023 at 03:07:21PM +0200, Simon Horman wrote:
> On Fri, May 26, 2023 at 10:41:34PM +0200, Christian Marangi wrote:
> > On some arch (for example IPQ8074) and other with
> > KERNEL_STACKPROTECTOR_STRONG enabled, the following compilation error is
> > triggered:
> > drivers/net/wireguard/allowedips.c: In function 'root_remove_peer_lists':
> > drivers/net/wireguard/allowedips.c:80:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
> >    80 | }
> >       | ^
> > drivers/net/wireguard/allowedips.c: In function 'root_free_rcu':
> > drivers/net/wireguard/allowedips.c:67:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
> >    67 | }
> >       | ^
> > cc1: all warnings being treated as errors
> > 
> > Since these are free function and returns void, using function that can
> > fail is not ideal since an error would result in data not freed.
> > Since the free are under RCU lock, we can allocate the required stack
> > array as static outside the function and memset when needed.
> > This effectively fix the stack frame warning without changing how the
> > function work.
> > 
> > Fixes: Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
> 
> nit: Not sure if this can be fixed-up manually.
>      But one instance of 'Fixes: ' is enough.
> 

An oversight by me sorry. Will send v2 with the tag fixed after 24
hours. Totally fine if the patch is OK to fix this when it's merged by
the maintainers.
Simon Horman May 27, 2023, 1:07 p.m. UTC | #2 
On Fri, May 26, 2023 at 10:41:34PM +0200, Christian Marangi wrote:
> On some arch (for example IPQ8074) and other with
> KERNEL_STACKPROTECTOR_STRONG enabled, the following compilation error is
> triggered:
> drivers/net/wireguard/allowedips.c: In function 'root_remove_peer_lists':
> drivers/net/wireguard/allowedips.c:80:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>    80 | }
>       | ^
> drivers/net/wireguard/allowedips.c: In function 'root_free_rcu':
> drivers/net/wireguard/allowedips.c:67:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>    67 | }
>       | ^
> cc1: all warnings being treated as errors
> 
> Since these are free function and returns void, using function that can
> fail is not ideal since an error would result in data not freed.
> Since the free are under RCU lock, we can allocate the required stack
> array as static outside the function and memset when needed.
> This effectively fix the stack frame warning without changing how the
> function work.
> 
> Fixes: Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")

nit: Not sure if this can be fixed-up manually.
     But one instance of 'Fixes: ' is enough.

> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> Cc: stable@vger.kernel.org
diff mbox series

Patch

diff --git a/drivers/net/wireguard/allowedips.c b/drivers/net/wireguard/allowedips.c
index 5bf7822c53f1..c129082f04c6 100644
--- a/drivers/net/wireguard/allowedips.c
+++ b/drivers/net/wireguard/allowedips.c
@@ -53,12 +53,16 @@  static void node_free_rcu(struct rcu_head *rcu)
 	kmem_cache_free(node_cache, container_of(rcu, struct allowedips_node, rcu));
 }
 
+static struct allowedips_node *tmpstack[MAX_ALLOWEDIPS_BITS];
+
 static void root_free_rcu(struct rcu_head *rcu)
 {
-	struct allowedips_node *node, *stack[MAX_ALLOWEDIPS_BITS] = {
-		container_of(rcu, struct allowedips_node, rcu) };
+	struct allowedips_node *node, **stack = tmpstack;
 	unsigned int len = 1;
 
+	memset(stack, 0, sizeof(*stack) * MAX_ALLOWEDIPS_BITS);
+	stack[0] = container_of(rcu, struct allowedips_node, rcu);
+
 	while (len > 0 && (node = stack[--len])) {
 		push_rcu(stack, node->bit[0], &len);
 		push_rcu(stack, node->bit[1], &len);
@@ -68,9 +72,12 @@  static void root_free_rcu(struct rcu_head *rcu)
 
 static void root_remove_peer_lists(struct allowedips_node *root)
 {
-	struct allowedips_node *node, *stack[MAX_ALLOWEDIPS_BITS] = { root };
+	struct allowedips_node *node, **stack = tmpstack;
 	unsigned int len = 1;
 
+	memset(stack, 0, sizeof(*stack) * MAX_ALLOWEDIPS_BITS);
+	stack[0] = root;
+
 	while (len > 0 && (node = stack[--len])) {
 		push_rcu(stack, node->bit[0], &len);
 		push_rcu(stack, node->bit[1], &len);