net: stmmac: Avoid VLA usage
Subsystems (4)
PATCH 1/1 net: stmmac: Avoid VLA usage
In the quest to remove all stack VLAs from the kernel[1], this switches the "status" stack buffer to use the existing small (8) upper bound on how many queues can be checked for DMA, and adds a sanity-check just to make sure it doesn't operate under pathological conditions.
[1] http://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
--- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +5 -1
@@ -2045,7 +2045,11 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv) tx_channel_count : rx_channel_count;u32 chan;bool poll_scheduled = false; -int status[channels_to_check]; +int status[max_t (u32,MTL_MAX_TX_QUEUES ,MTL_MAX_RX_QUEUES )]; + +/* Make sure we never check beyond our status buffer. */ +if (WARN_ON_ONCE (channels_to_check >ARRAY_SIZE (status))) + channels_to_check =ARRAY_SIZE (status);/* Each DMA channel can be used for rx and tx simultaneously, yet * napi_struct is embedded in struct stmmac_rx_queue rather than in a
Signature · 6 lines
2.7.4 -- Kees Cook Pixel Security
Hi Kees,
Quoted 9 lines · Kees Cook
In the quest to remove all stack VLAs from the kernel[1], this switches the "status" stack buffer to use the existing small (8) upper bound on how many queues can be checked for DMA, and adds a sanity-check just to make sure it doesn't operate under pathological conditions.
Signed-off-by Kees Cook <[email protected]>
I rather prefer the variables declaration in reverse-tree order, but thats just a minor pick.
Thanks and Best Regards,
Jose Miguel Abreu
PS: Is VLA warning switch in gcc already active? Because I didn't see this warning in my builds.
Quoted 8 lines · Jose Abreu
Hi Kees,
Quoted 5 lines · Kees Cook
In the quest to remove all stack VLAs from the kernel[1], this switches the "status" stack buffer to use the existing small (8) upper bound on how many queues can be checked for DMA, and adds a sanity-check just to make sure it doesn't operate under pathological conditions.
Quoted 7 lines
Quoted 4 lines
Signed-off-by Kees Cook <[email protected]>I rather prefer the variables declaration in reverse-tree order, but thats just a minor pick.
I can explicitly reorder the other variables, if you want?
Quoted 1 line
Reviewed-by Jose Abreu <[email protected]>
Thanks!
Quoted 2 lines
PS: Is VLA warning switch in gcc already active? Because I didn't see this warning in my builds.
It is not. A bunch of people have been building with KCFLAGS=-Wvla to find the VLAs and sending patches. Once we get rid of them all, we can add the flag to the top-level Makefile.
-Kees
Signature · 2 lines
Kees Cook Pixel Security
Quoted 9 lines · Kees Cook
Quoted 8 lines · Jose Abreu
Hi Kees,
Quoted 5 lines · Kees Cook
In the quest to remove all stack VLAs from the kernel[1], this switches the "status" stack buffer to use the existing small (8) upper bound on how many queues can be checked for DMA, and adds a sanity-check just to make sure it doesn't operate under pathological conditions.
Quoted 7 lines
Quoted 6 lines
Quoted 4 lines
Signed-off-by Kees Cook <[email protected]>I rather prefer the variables declaration in reverse-tree order, but thats just a minor pick.
I can explicitly reorder the other variables, if you want?
No need by me, unless Giuseppe or Alexandre prefer that. Thanks!
Best Regards,
Jose Miguel Abreu
Quoted 12 lines
Quoted 2 lines
Reviewed-by Jose Abreu <[email protected]>Thanks!
Quoted 2 lines
PS: Is VLA warning switch in gcc already active? Because I didn't see this warning in my builds.
It is not. A bunch of people have been building with KCFLAGS=-Wvla to find the VLAs and sending patches. Once we get rid of them all, we can add the flag to the top-level
Makefile.-Kees
Quoted 14 lines · Jose Abreu
Quoted 11 lines · Kees Cook
Quoted 10 lines · Jose Abreu
Hi Kees,
Quoted 7 lines · Kees Cook
In the quest to remove all stack VLAs from the kernel[1], this switches the "status" stack buffer to use the existing small (8) upper bound on how many queues can be checked for DMA, and adds a sanity-check just to make sure it doesn't operate under pathological conditions.
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Signed-off-by Kees Cook <[email protected]>I rather prefer the variables declaration in reverse-tree order, but thats just a minor pick.
I can explicitly reorder the other variables, if you want?
No need by me, unless Giuseppe or Alexandre prefer that. Thanks!
No need.
Quoted 17 lines
Best Regards,
Jose Miguel AbreuQuoted 12 lines
Quoted 2 lines
Reviewed-by Jose Abreu <[email protected]>Thanks!
Quoted 2 lines
PS: Is VLA warning switch in gcc already active? Because I didn't see this warning in my builds.
It is not. A bunch of people have been building with KCFLAGS=-Wvla to find the VLAs and sending patches. Once we get rid of them all, we can add the flag to the top-level
Makefile.-Kees
From: Kees Cook <[email protected]>
Date: Tue, 1 May 2018 14:01:30 -0700
Quoted 8 lines
In the quest to remove all stack VLAs from the kernel[1], this switches the "status" stack buffer to use the existing small (8) upper bound on how many queues can be checked for DMA, and adds a sanity-check just to make sure it doesn't operate under pathological conditions.
[1] http://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
Signed-off-by Kees Cook <[email protected]>
Applied to net-next, thank you.