From: Arnd Bergmann <[email protected]>
gcc-11 with KASAN on 32-bit arm produces a warning about a function
that needs a lot of stack space:
drivers/net/wireless/cisco/airo.c: In function 'setup_card.constprop':
drivers/net/wireless/cisco/airo.c:3960:1: error: the frame size of 1512 bytes is larger than 1400 bytes [-Werror=frame-larger-than=]
Most of this is from a single large structure that could be dynamically
allocated or moved into the per-device structure. However, as the callers
all seem to have a fairly well bounded call chain, the easiest change
is to pull out the part of the function that needs the large variables
into a separate function and mark that as noinline_for_stack. This does
not reduce the total stack usage, but it gets rid of the warning and
requires minimal changes otherwise.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/wireless/cisco/airo.c | 117 +++++++++++++++++-------------
1 file changed, 65 insertions(+), 52 deletions(-)
diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
index e35e1380ae43..540ba694899c 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -3818,6 +3818,68 @@ static inline void set_auth_type(struct airo_info *local, int auth_type)
local->last_auth = auth_type;
}
+static int noinline_for_stack airo_readconfig(struct airo_info *ai, u8 *mac, int lock)
+{
+ int i, status;
+ /* large variables, so don't inline this function,
+ * maybe change to kmalloc
+ */
+ tdsRssiRid rssi_rid;
+ CapabilityRid cap_rid;
+
+ kfree(ai->SSID);
+ ai->SSID = NULL;
+ // general configuration (read/modify/write)
+ status = readConfigRid(ai, lock);
+ if (status != SUCCESS) return ERROR;
+
+ status = readCapabilityRid(ai, &cap_rid, lock);
+ if (status != SUCCESS) return ERROR;
+
+ status = PC4500_readrid(ai, RID_RSSI, &rssi_rid, sizeof(rssi_rid), lock);
+ if (status == SUCCESS) {
+ if (ai->rssi || (ai->rssi = kmalloc(512, GFP_KERNEL)) != NULL)
+ memcpy(ai->rssi, (u8*)&rssi_rid + 2, 512); /* Skip RID length member */
+ }
+ else {
+ kfree(ai->rssi);
+ ai->rssi = NULL;
+ if (cap_rid.softCap & cpu_to_le16(8))
+ ai->config.rmode |= RXMODE_NORMALIZED_RSSI;
+ else
+ airo_print_warn(ai->dev->name, "unknown received signal "
+ "level scale");
+ }
+ ai->config.opmode = adhoc ? MODE_STA_IBSS : MODE_STA_ESS;
+ set_auth_type(ai, AUTH_OPEN);
+ ai->config.modulation = MOD_CCK;
+
+ if (le16_to_cpu(cap_rid.len) >= sizeof(cap_rid) &&
+ (cap_rid.extSoftCap & cpu_to_le16(1)) &&
+ micsetup(ai) == SUCCESS) {
+ ai->config.opmode |= MODE_MIC;
+ set_bit(FLAG_MIC_CAPABLE, &ai->flags);
+ }
+
+ /* Save off the MAC */
+ for (i = 0; i < ETH_ALEN; i++) {
+ mac[i] = ai->config.macAddr[i];
+ }
+
+ /* Check to see if there are any insmod configured
+ rates to add */
+ if (rates[0]) {
+ memset(ai->config.rates, 0, sizeof(ai->config.rates));
+ for (i = 0; i < 8 && rates[i]; i++) {
+ ai->config.rates[i] = rates[i];
+ }
+ }
+ set_bit (FLAG_COMMIT, &ai->flags);
+
+ return SUCCESS;
+}
+
+
static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
{
Cmd cmd;
@@ -3864,58 +3926,9 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
if (lock)
up(&ai->sem);
if (ai->config.len == 0) {
- int i;
- tdsRssiRid rssi_rid;
- CapabilityRid cap_rid;
-
- kfree(ai->SSID);
- ai->SSID = NULL;
- // general configuration (read/modify/write)
- status = readConfigRid(ai, lock);
- if (status != SUCCESS) return ERROR;
-
- status = readCapabilityRid(ai, &cap_rid, lock);
- if (status != SUCCESS) return ERROR;
-
- status = PC4500_readrid(ai, RID_RSSI,&rssi_rid, sizeof(rssi_rid), lock);
- if (status == SUCCESS) {
- if (ai->rssi || (ai->rssi = kmalloc(512, GFP_KERNEL)) != NULL)
- memcpy(ai->rssi, (u8*)&rssi_rid + 2, 512); /* Skip RID length member */
- }
- else {
- kfree(ai->rssi);
- ai->rssi = NULL;
- if (cap_rid.softCap & cpu_to_le16(8))
- ai->config.rmode |= RXMODE_NORMALIZED_RSSI;
- else
- airo_print_warn(ai->dev->name, "unknown received signal "
- "level scale");
- }
- ai->config.opmode = adhoc ? MODE_STA_IBSS : MODE_STA_ESS;
- set_auth_type(ai, AUTH_OPEN);
- ai->config.modulation = MOD_CCK;
-
- if (le16_to_cpu(cap_rid.len) >= sizeof(cap_rid) &&
- (cap_rid.extSoftCap & cpu_to_le16(1)) &&
- micsetup(ai) == SUCCESS) {
- ai->config.opmode |= MODE_MIC;
- set_bit(FLAG_MIC_CAPABLE, &ai->flags);
- }
-
- /* Save off the MAC */
- for (i = 0; i < ETH_ALEN; i++) {
- mac[i] = ai->config.macAddr[i];
- }
-
- /* Check to see if there are any insmod configured
- rates to add */
- if (rates[0]) {
- memset(ai->config.rates, 0, sizeof(ai->config.rates));
- for (i = 0; i < 8 && rates[i]; i++) {
- ai->config.rates[i] = rates[i];
- }
- }
- set_bit (FLAG_COMMIT, &ai->flags);
+ status = airo_readconfig(ai, mac, lock);
+ if (status != SUCCESS)
+ return ERROR;
}
/* Setup the SSIDs if present */
--
2.29.2
On 3/23/21 6:16 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> gcc-11 with KASAN on 32-bit arm produces a warning about a function
> that needs a lot of stack space:
>
> drivers/net/wireless/cisco/airo.c: In function 'setup_card.constprop':
> drivers/net/wireless/cisco/airo.c:3960:1: error: the frame size of 1512 bytes is larger than 1400 bytes [-Werror=frame-larger-than=]
>
> Most of this is from a single large structure that could be dynamically
> allocated or moved into the per-device structure. However, as the callers
> all seem to have a fairly well bounded call chain, the easiest change
> is to pull out the part of the function that needs the large variables
> into a separate function and mark that as noinline_for_stack. This does
> not reduce the total stack usage, but it gets rid of the warning and
> requires minimal changes otherwise.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> drivers/net/wireless/cisco/airo.c | 117 +++++++++++++++++-------------
> 1 file changed, 65 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
> index e35e1380ae43..540ba694899c 100644
> --- a/drivers/net/wireless/cisco/airo.c
> +++ b/drivers/net/wireless/cisco/airo.c
> @@ -3818,6 +3818,68 @@ static inline void set_auth_type(struct airo_info *local, int auth_type)
> local->last_auth = auth_type;
> }
>
> +static int noinline_for_stack airo_readconfig(struct airo_info *ai, u8 *mac, int lock)
> +{
> + int i, status;
> + /* large variables, so don't inline this function,
> + * maybe change to kmalloc
> + */
> + tdsRssiRid rssi_rid;
> + CapabilityRid cap_rid;
> +
> + kfree(ai->SSID);
> + ai->SSID = NULL;
> + // general configuration (read/modify/write)
> + status = readConfigRid(ai, lock);
> + if (status != SUCCESS) return ERROR;
> +
> + status = readCapabilityRid(ai, &cap_rid, lock);
> + if (status != SUCCESS) return ERROR;
> +
> + status = PC4500_readrid(ai, RID_RSSI, &rssi_rid, sizeof(rssi_rid), lock);
> + if (status == SUCCESS) {
> + if (ai->rssi || (ai->rssi = kmalloc(512, GFP_KERNEL)) != NULL)
> + memcpy(ai->rssi, (u8*)&rssi_rid + 2, 512); /* Skip RID length member */
> + }
> + else {
> + kfree(ai->rssi);
> + ai->rssi = NULL;
> + if (cap_rid.softCap & cpu_to_le16(8))
> + ai->config.rmode |= RXMODE_NORMALIZED_RSSI;
> + else
> + airo_print_warn(ai->dev->name, "unknown received signal "
> + "level scale");
> + }
> + ai->config.opmode = adhoc ? MODE_STA_IBSS : MODE_STA_ESS;
> + set_auth_type(ai, AUTH_OPEN);
> + ai->config.modulation = MOD_CCK;
> +
> + if (le16_to_cpu(cap_rid.len) >= sizeof(cap_rid) &&
> + (cap_rid.extSoftCap & cpu_to_le16(1)) &&
> + micsetup(ai) == SUCCESS) {
> + ai->config.opmode |= MODE_MIC;
> + set_bit(FLAG_MIC_CAPABLE, &ai->flags);
> + }
> +
> + /* Save off the MAC */
> + for (i = 0; i < ETH_ALEN; i++) {
> + mac[i] = ai->config.macAddr[i];
> + }
> +
> + /* Check to see if there are any insmod configured
> + rates to add */
> + if (rates[0]) {
> + memset(ai->config.rates, 0, sizeof(ai->config.rates));
> + for (i = 0; i < 8 && rates[i]; i++) {
> + ai->config.rates[i] = rates[i];
> + }
> + }
> + set_bit (FLAG_COMMIT, &ai->flags);
> +
> + return SUCCESS;
> +}
> +
> +
> static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
> {
> Cmd cmd;
> @@ -3864,58 +3926,9 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock)
> if (lock)
> up(&ai->sem);
> if (ai->config.len == 0) {
> - int i;
> - tdsRssiRid rssi_rid;
This is mostly an array, and what I guess is the problem.
> - CapabilityRid cap_rid;
> -
> - kfree(ai->SSID);
> - ai->SSID = NULL;
> - // general configuration (read/modify/write)
> - status = readConfigRid(ai, lock);
> - if (status != SUCCESS) return ERROR;
> -
> - status = readCapabilityRid(ai, &cap_rid, lock);
> - if (status != SUCCESS) return ERROR;
> -
> - status = PC4500_readrid(ai, RID_RSSI,&rssi_rid, sizeof(rssi_rid), lock);
This is reading into a stack temp
> - if (status == SUCCESS) {
> - if (ai->rssi || (ai->rssi = kmalloc(512, GFP_KERNEL)) != NULL)
> - memcpy(ai->rssi, (u8*)&rssi_rid + 2, 512); /* Skip RID length member */
Here is the temp being copied to heap memory.
This is the only use of rssi_rid
Why not do the alloc in PC4500_read ?
The call would be something like
status = PC4500_readrid(ai, RID_RSSI, &ai->rssi, sizeof(), lock)
or since ai, is common reduce the signature of the function and make the call
PC4500_readid(ai, RID_RSSI, lock)
Tom
> - }
> - else {
> - kfree(ai->rssi);
> - ai->rssi = NULL;
> - if (cap_rid.softCap & cpu_to_le16(8))
> - ai->config.rmode |= RXMODE_NORMALIZED_RSSI;
> - else
> - airo_print_warn(ai->dev->name, "unknown received signal "
> - "level scale");
> - }
> - ai->config.opmode = adhoc ? MODE_STA_IBSS : MODE_STA_ESS;
> - set_auth_type(ai, AUTH_OPEN);
> - ai->config.modulation = MOD_CCK;
> -
> - if (le16_to_cpu(cap_rid.len) >= sizeof(cap_rid) &&
> - (cap_rid.extSoftCap & cpu_to_le16(1)) &&
> - micsetup(ai) == SUCCESS) {
> - ai->config.opmode |= MODE_MIC;
> - set_bit(FLAG_MIC_CAPABLE, &ai->flags);
> - }
> -
> - /* Save off the MAC */
> - for (i = 0; i < ETH_ALEN; i++) {
> - mac[i] = ai->config.macAddr[i];
> - }
> -
> - /* Check to see if there are any insmod configured
> - rates to add */
> - if (rates[0]) {
> - memset(ai->config.rates, 0, sizeof(ai->config.rates));
> - for (i = 0; i < 8 && rates[i]; i++) {
> - ai->config.rates[i] = rates[i];
> - }
> - }
> - set_bit (FLAG_COMMIT, &ai->flags);
> + status = airo_readconfig(ai, mac, lock);
> + if (status != SUCCESS)
> + return ERROR;
> }
>
> /* Setup the SSIDs if present */
Arnd Bergmann <[email protected]> wrote:
> From: Arnd Bergmann <[email protected]>
>
> gcc-11 with KASAN on 32-bit arm produces a warning about a function
> that needs a lot of stack space:
>
> drivers/net/wireless/cisco/airo.c: In function 'setup_card.constprop':
> drivers/net/wireless/cisco/airo.c:3960:1: error: the frame size of 1512 bytes is larger than 1400 bytes [-Werror=frame-larger-than=]
>
> Most of this is from a single large structure that could be dynamically
> allocated or moved into the per-device structure. However, as the callers
> all seem to have a fairly well bounded call chain, the easiest change
> is to pull out the part of the function that needs the large variables
> into a separate function and mark that as noinline_for_stack. This does
> not reduce the total stack usage, but it gets rid of the warning and
> requires minimal changes otherwise.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
Patch applied to wireless-drivers-next.git, thanks.
7909a590eba6 airo: work around stack usage warning
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches