diff mbox series

mwl8k: Avoid overlapping composite structs that contain flex-arrays

Message ID 20240316150712.4633-1-erick.archer@gmx.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series mwl8k: Avoid overlapping composite structs that contain flex-arrays | expand

Commit Message

Erick Archer March 16, 2024, 3:07 p.m. UTC 
When a struct containing a flexible array is included in another struct,
and there is a member after the struct-with-flex-array, there is a
possibility of memory overlap. These cases must be audited [1]. See:

struct inner {
	...
	int flex[];
};

struct outer {
	...
	struct inner header;
	int overlap;
	...
};

This is the scenario for the "struct mwl8k_cmd_pkt" structure that is
included in the following "struct mwl8k_cmd_*" structures:

struct mwl8k_cmd_get_hw_spec_sta
struct mwl8k_cmd_get_hw_spec_ap
struct mwl8k_cmd_set_hw_spec
struct mwl8k_cmd_mac_multicast_adr
struct mwl8k_cmd_get_stat
struct mwl8k_cmd_radio_control
struct mwl8k_cmd_rf_tx_power
struct mwl8k_cmd_tx_power
struct mwl8k_cmd_rf_antenna
struct mwl8k_cmd_set_beacon
struct mwl8k_cmd_bbp_reg_access
struct mwl8k_cmd_set_post_scan
struct mwl8k_cmd_set_rf_channel
struct mwl8k_cmd_update_set_aid
struct mwl8k_cmd_set_rate
struct mwl8k_cmd_finalize_join
struct mwl8k_cmd_set_rts_threshold
struct mwl8k_cmd_set_slot
struct mwl8k_cmd_set_edca_params
struct mwl8k_cmd_set_wmm_mode
struct mwl8k_cmd_mimo_config
struct mwl8k_cmd_use_fixed_rate_sta
struct mwl8k_cmd_use_fixed_rate_ap
struct mwl8k_cmd_enable_sniffer
struct mwl8k_cmd_update_mac_addr
struct mwl8k_cmd_set_rate_adapt_mode
struct mwl8k_cmd_get_watchdog_bitmap
struct mwl8k_cmd_bss_start
struct mwl8k_cmd_bastream
struct mwl8k_cmd_set_new_stn
struct mwl8k_cmd_update_encryption
struct mwl8k_cmd_set_key
struct mwl8k_cmd_update_stadb

The pattern is like the one shown below:

struct mwl8k_cmd_pkt {
	...
	char payload[];
} __packed;

struct mwl8k_cmd_* {
	struct mwl8k_cmd_pkt header;
	...
};

In this case, because the flexible array "payload" is only used in the
"mwl8k_load_fw_image" function, it is best to define a new structure for
the packet header called "struct mwl8k_cmd_pkt_hdr". This way, the
"struct mwl8k_cmd_pkt" and all the affected "struct mwl8k_cmd_*" used
for commands can now be defined using this new header structure.

For consistency, although the "struct mwl8k_cmd_set_pre_scan" does not
suffer from the overlapping scenario, also use the new header structure
to define it.

Moreover, change the prototype of the "mwl8k_post_cmd" function and the
"mwl8k_post_pervif_cmd" function because it is not necessary to pass the
whole packet structure. It is enough to use only the header structure.
Also, change the return type of the "__mwl8k_cmd_mac_multicast_adr"
function for the same reason.

As a final point, refactor the necessary calls, use the new members of
the structures and change some variable names and types to achieve the
desired goal.

Link: https://github.com/KSPP/linux/issues/202 [1]
Signed-off-by: Erick Archer <erick.archer@gmx.com>
---
Hi everyone,

This patch is based on my understanding of the code. Any comments would
be greatly appreciated.

Also, I have verified that the old and new packet structure are the same
size:

struct mwl8k_cmd_pkt_old {
        __le16  code;
        __le16  length;
        __u8    seq_num;
        __u8    macid;
        __le16  result;
        char    payload[];
} __packed;

struct mwl8k_cmd_pkt_hdr {
        __le16  code;
        __le16  length;
        __u8    seq_num;
        __u8    macid;
        __le16  result;
} __packed;

struct mwl8k_cmd_pkt_new {
        struct mwl8k_cmd_pkt_hdr header;
        char payload[];
} __packed;

static_assert(sizeof(struct mwl8k_cmd_pkt_old) == sizeof(struct mwl8k_cmd_pkt_new));

Best regards,
Erick
---
 drivers/net/wireless/marvell/mwl8k.c | 145 ++++++++++++++-------------
 1 file changed, 75 insertions(+), 70 deletions(-)

--
2.25.1

Comments

Gustavo A. R. Silva March 16, 2024, 6:59 p.m. UTC | #1 
[..]

> 
> Link: https://github.com/KSPP/linux/issues/202 [1]
> Signed-off-by: Erick Archer <erick.archer@gmx.com>
> ---
> Hi everyone,
> 
> This patch is based on my understanding of the code. Any comments would
> be greatly appreciated.

Thanks for looking into this. :)

I'm currently in the process of trying a general solution for all these
composite structures without having to use two separate structs, avoid too
much code churn, and continue allowing for __counted_by() annotations at
the same time.

I'll be sending a bunch of patches once the merge window closes. So, for
now, I think it's wise to wait for those patches.

More comments below.

[..]

> diff --git a/drivers/net/wireless/marvell/mwl8k.c b/drivers/net/wireless/marvell/mwl8k.c
> index ce8fea76dbb2..57de32ba4efc 100644
> --- a/drivers/net/wireless/marvell/mwl8k.c
> +++ b/drivers/net/wireless/marvell/mwl8k.c
> @@ -586,13 +586,17 @@ static int mwl8k_request_firmware(struct mwl8k_priv *priv, char *fw_image,
>   	return 0;
>   }
> 
> -struct mwl8k_cmd_pkt {
> +struct mwl8k_cmd_pkt_hdr {
>   	__le16	code;
>   	__le16	length;
>   	__u8	seq_num;
>   	__u8	macid;
>   	__le16	result;
> -	char	payload[];
> +} __packed;
> +
> +struct mwl8k_cmd_pkt {
> +	struct mwl8k_cmd_pkt_hdr header;
> +	char payload[];
>   } __packed;

One of the problems with this is that `struct mwl8k_cmd_pkt` is candidate for a
`__counted_by()` annotation:

@@ -592,7 +592,7 @@ struct mwl8k_cmd_pkt {
         __u8    seq_num;
         __u8    macid;
         __le16  result;
-       char    payload[];
+       char    payload[] __counted_by(length);
  } __packed;

and with the changes you propose, that is not possible anymore because the counter
member must be at the same level or in an anonymous struct also at the same level
as `payload`.

Thanks
--
Gustavo
Erick Archer March 17, 2024, 3:22 p.m. UTC | #2 
Hi Gustavo,

On Sat, Mar 16, 2024 at 12:59:11PM -0600, Gustavo A. R. Silva wrote:
>
> [..]
>
> >
> > Link: https://github.com/KSPP/linux/issues/202 [1]
> > Signed-off-by: Erick Archer <erick.archer@gmx.com>
> > ---
> > Hi everyone,
> >
> > This patch is based on my understanding of the code. Any comments would
> > be greatly appreciated.
>
> Thanks for looking into this. :)
>
> I'm currently in the process of trying a general solution for all these
> composite structures without having to use two separate structs, avoid too
> much code churn, and continue allowing for __counted_by() annotations at
> the same time.

I searched the mailing list and found several of your patches:

Link: https://lore.kernel.org/linux-hardening/ZfCXBykRw5XqBvf0@neat/
Link: https://lore.kernel.org/linux-hardening/cover.1709658886.git.gustavoars@kernel.org/
Link: https://lore.kernel.org/linux-hardening/ZeeaRuTpuxInH6ZB@neat/

In all of them you use the `struct_group_tagged()` helper to solve the
overlapping scenario. Great proposal ;)

> I'll be sending a bunch of patches once the merge window closes. So, for
> now, I think it's wise to wait for those patches.

So, are you working in a patch for the "mwl8k"? Or do you prefer
a v2 of this patch based on your proposal?

>
> More comments below.
>
> [..]
>
> > diff --git a/drivers/net/wireless/marvell/mwl8k.c b/drivers/net/wireless/marvell/mwl8k.c
> > index ce8fea76dbb2..57de32ba4efc 100644
> > --- a/drivers/net/wireless/marvell/mwl8k.c
> > +++ b/drivers/net/wireless/marvell/mwl8k.c
> > @@ -586,13 +586,17 @@ static int mwl8k_request_firmware(struct mwl8k_priv *priv, char *fw_image,
> >   	return 0;
> >   }
> >
> > -struct mwl8k_cmd_pkt {
> > +struct mwl8k_cmd_pkt_hdr {
> >   	__le16	code;
> >   	__le16	length;
> >   	__u8	seq_num;
> >   	__u8	macid;
> >   	__le16	result;
> > -	char	payload[];
> > +} __packed;
> > +
> > +struct mwl8k_cmd_pkt {
> > +	struct mwl8k_cmd_pkt_hdr header;
> > +	char payload[];
> >   } __packed;
>
> One of the problems with this is that `struct mwl8k_cmd_pkt` is candidate for a
> `__counted_by()` annotation:
>
> @@ -592,7 +592,7 @@ struct mwl8k_cmd_pkt {
>         __u8    seq_num;
>         __u8    macid;
>         __le16  result;
> -       char    payload[];
> +       char    payload[] __counted_by(length);
>  } __packed;
>
> and with the changes you propose, that is not possible anymore because the counter
> member must be at the same level or in an anonymous struct also at the same level
> as `payload`.

Ok, I understand the problem you raise and I agree.
Anyway, thanks for your comments.

Best regards,
Erick

> Thanks
> --
> Gustavo
>
Gustavo A. R. Silva March 17, 2024, 8:07 p.m. UTC | #3 
> So, are you working in a patch for the "mwl8k"? Or do you prefer
> a v2 of this patch based on your proposal?

I'm working on multiple different patches to address the whole thing
across the kernel.

I think it's wise for you to wait and see how those patches are received.
Once the best approach to fix these issues becomes clear and widely accepted,
it will be easier for you to join us and help address the rest. Meanwhile,
we want to avoid any possible overlap or stepping on each other's toes.

Thanks
--
Gustavo
diff mbox series

Patch

diff --git a/drivers/net/wireless/marvell/mwl8k.c b/drivers/net/wireless/marvell/mwl8k.c
index ce8fea76dbb2..57de32ba4efc 100644
--- a/drivers/net/wireless/marvell/mwl8k.c
+++ b/drivers/net/wireless/marvell/mwl8k.c
@@ -586,13 +586,17 @@  static int mwl8k_request_firmware(struct mwl8k_priv *priv, char *fw_image,
 	return 0;
 }

-struct mwl8k_cmd_pkt {
+struct mwl8k_cmd_pkt_hdr {
 	__le16	code;
 	__le16	length;
 	__u8	seq_num;
 	__u8	macid;
 	__le16	result;
-	char	payload[];
+} __packed;
+
+struct mwl8k_cmd_pkt {
+	struct mwl8k_cmd_pkt_hdr header;
+	char payload[];
 } __packed;

 /*
@@ -652,17 +656,17 @@  static int mwl8k_load_fw_image(struct mwl8k_priv *priv,
 	if (cmd == NULL)
 		return -ENOMEM;

-	cmd->code = cpu_to_le16(MWL8K_CMD_CODE_DNLD);
-	cmd->seq_num = 0;
-	cmd->macid = 0;
-	cmd->result = 0;
+	cmd->header.code = cpu_to_le16(MWL8K_CMD_CODE_DNLD);
+	cmd->header.seq_num = 0;
+	cmd->header.macid = 0;
+	cmd->header.result = 0;

 	done = 0;
 	while (length) {
 		int block_size = length > 256 ? 256 : length;

 		memcpy(cmd->payload, data + done, block_size);
-		cmd->length = cpu_to_le16(block_size);
+		cmd->header.length = cpu_to_le16(block_size);

 		rc = mwl8k_send_fw_load_cmd(priv, cmd,
 						sizeof(*cmd) + block_size);
@@ -674,7 +678,7 @@  static int mwl8k_load_fw_image(struct mwl8k_priv *priv,
 	}

 	if (!rc) {
-		cmd->length = 0;
+		cmd->header.length = 0;
 		rc = mwl8k_send_fw_load_cmd(priv, cmd, sizeof(*cmd));
 	}

@@ -2201,7 +2205,8 @@  static void mwl8k_enable_bsses(struct ieee80211_hw *hw, bool enable,
 /* Timeout firmware commands after 10s */
 #define MWL8K_CMD_TIMEOUT_MS	10000

-static int mwl8k_post_cmd(struct ieee80211_hw *hw, struct mwl8k_cmd_pkt *cmd)
+static int mwl8k_post_cmd(struct ieee80211_hw *hw,
+			  struct mwl8k_cmd_pkt_hdr *hdr)
 {
 	DECLARE_COMPLETION_ONSTACK(cmd_wait);
 	struct mwl8k_priv *priv = hw->priv;
@@ -2214,7 +2219,7 @@  static int mwl8k_post_cmd(struct ieee80211_hw *hw, struct mwl8k_cmd_pkt *cmd)
 	u32 bitmap = 0;

 	wiphy_dbg(hw->wiphy, "Posting %s [%d]\n",
-		  mwl8k_cmd_name(cmd->code, buf, sizeof(buf)), cmd->macid);
+		  mwl8k_cmd_name(hdr->code, buf, sizeof(buf)), hdr->macid);

 	/* Before posting firmware commands that could change the hardware
 	 * characteristics, make sure that all BSSes are stopped temporary.
@@ -2226,7 +2231,7 @@  static int mwl8k_post_cmd(struct ieee80211_hw *hw, struct mwl8k_cmd_pkt *cmd)
 		return rc;

 	if (priv->ap_fw && priv->running_bsses) {
-		switch (le16_to_cpu(cmd->code)) {
+		switch (le16_to_cpu(hdr->code)) {
 		case MWL8K_CMD_SET_RF_CHANNEL:
 		case MWL8K_CMD_RADIO_CONTROL:
 		case MWL8K_CMD_RF_TX_POWER:
@@ -2240,9 +2245,9 @@  static int mwl8k_post_cmd(struct ieee80211_hw *hw, struct mwl8k_cmd_pkt *cmd)
 		}
 	}

-	cmd->result = (__force __le16) 0xffff;
-	dma_size = le16_to_cpu(cmd->length);
-	dma_addr = dma_map_single(&priv->pdev->dev, cmd, dma_size,
+	hdr->result = (__force __le16)0xffff;
+	dma_size = le16_to_cpu(hdr->length);
+	dma_addr = dma_map_single(&priv->pdev->dev, hdr, dma_size,
 				  DMA_BIDIRECTIONAL);
 	if (dma_mapping_error(&priv->pdev->dev, dma_addr)) {
 		rc = -ENOMEM;
@@ -2267,7 +2272,7 @@  static int mwl8k_post_cmd(struct ieee80211_hw *hw, struct mwl8k_cmd_pkt *cmd)

 	if (!timeout) {
 		wiphy_err(hw->wiphy, "Command %s timeout after %u ms\n",
-			  mwl8k_cmd_name(cmd->code, buf, sizeof(buf)),
+			  mwl8k_cmd_name(hdr->code, buf, sizeof(buf)),
 			  MWL8K_CMD_TIMEOUT_MS);
 		rc = -ETIMEDOUT;
 	} else {
@@ -2275,15 +2280,15 @@  static int mwl8k_post_cmd(struct ieee80211_hw *hw, struct mwl8k_cmd_pkt *cmd)

 		ms = MWL8K_CMD_TIMEOUT_MS - jiffies_to_msecs(timeout);

-		rc = cmd->result ? -EINVAL : 0;
+		rc = hdr->result ? -EINVAL : 0;
 		if (rc)
 			wiphy_err(hw->wiphy, "Command %s error 0x%x\n",
-				  mwl8k_cmd_name(cmd->code, buf, sizeof(buf)),
-				  le16_to_cpu(cmd->result));
+				  mwl8k_cmd_name(hdr->code, buf, sizeof(buf)),
+				  le16_to_cpu(hdr->result));
 		else if (ms > 2000)
 			wiphy_notice(hw->wiphy, "Command %s took %d ms\n",
-				     mwl8k_cmd_name(cmd->code,
-						    buf, sizeof(buf)),
+				     mwl8k_cmd_name(hdr->code, buf,
+						    sizeof(buf)),
 				     ms);
 	}

@@ -2298,11 +2303,11 @@  static int mwl8k_post_cmd(struct ieee80211_hw *hw, struct mwl8k_cmd_pkt *cmd)

 static int mwl8k_post_pervif_cmd(struct ieee80211_hw *hw,
 				 struct ieee80211_vif *vif,
-				 struct mwl8k_cmd_pkt *cmd)
+				 struct mwl8k_cmd_pkt_hdr *hdr)
 {
 	if (vif != NULL)
-		cmd->macid = MWL8K_VIF(vif)->macid;
-	return mwl8k_post_cmd(hw, cmd);
+		hdr->macid = MWL8K_VIF(vif)->macid;
+	return mwl8k_post_cmd(hw, hdr);
 }

 /*
@@ -2350,7 +2355,7 @@  static void mwl8k_setup_5ghz_band(struct ieee80211_hw *hw)
  * CMD_GET_HW_SPEC (STA version).
  */
 struct mwl8k_cmd_get_hw_spec_sta {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__u8 hw_rev;
 	__u8 host_interface;
 	__le16 num_mcaddrs;
@@ -2499,7 +2504,7 @@  static int mwl8k_cmd_get_hw_spec_sta(struct ieee80211_hw *hw)
  * CMD_GET_HW_SPEC (AP version).
  */
 struct mwl8k_cmd_get_hw_spec_ap {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__u8 hw_rev;
 	__u8 host_interface;
 	__le16 num_wcb;
@@ -2593,7 +2598,7 @@  static int mwl8k_cmd_get_hw_spec_ap(struct ieee80211_hw *hw)
  * CMD_SET_HW_SPEC.
  */
 struct mwl8k_cmd_set_hw_spec {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__u8 hw_rev;
 	__u8 host_interface;
 	__le16 num_mcaddrs;
@@ -2670,7 +2675,7 @@  static int mwl8k_cmd_set_hw_spec(struct ieee80211_hw *hw)
  * CMD_MAC_MULTICAST_ADR.
  */
 struct mwl8k_cmd_mac_multicast_adr {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__le16 numaddr;
 	__u8 addr[][ETH_ALEN];
@@ -2681,7 +2686,7 @@  struct mwl8k_cmd_mac_multicast_adr {
 #define MWL8K_ENABLE_RX_ALL_MULTICAST	0x0004
 #define MWL8K_ENABLE_RX_BROADCAST	0x0008

-static struct mwl8k_cmd_pkt *
+static struct mwl8k_cmd_pkt_hdr *
 __mwl8k_cmd_mac_multicast_adr(struct ieee80211_hw *hw, int allmulti,
 			      struct netdev_hw_addr_list *mc_list)
 {
@@ -2729,7 +2734,7 @@  __mwl8k_cmd_mac_multicast_adr(struct ieee80211_hw *hw, int allmulti,
  * CMD_GET_STAT.
  */
 struct mwl8k_cmd_get_stat {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le32 stats[64];
 } __packed;

@@ -2771,7 +2776,7 @@  static int mwl8k_cmd_get_stat(struct ieee80211_hw *hw,
  * CMD_RADIO_CONTROL.
  */
 struct mwl8k_cmd_radio_control {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__le16 control;
 	__le16 radio_on;
@@ -2832,7 +2837,7 @@  mwl8k_set_radio_preamble(struct ieee80211_hw *hw, bool short_preamble)
 #define MWL8K_RF_TX_POWER_LEVEL_TOTAL	8

 struct mwl8k_cmd_rf_tx_power {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__le16 support_level;
 	__le16 current_level;
@@ -2866,7 +2871,7 @@  static int mwl8k_cmd_rf_tx_power(struct ieee80211_hw *hw, int dBm)
 #define MWL8K_TX_POWER_LEVEL_TOTAL      12

 struct mwl8k_cmd_tx_power {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__le16 band;
 	__le16 channel;
@@ -2925,7 +2930,7 @@  static int mwl8k_cmd_tx_power(struct ieee80211_hw *hw,
  * CMD_RF_ANTENNA.
  */
 struct mwl8k_cmd_rf_antenna {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 antenna;
 	__le16 mode;
 } __packed;
@@ -2958,7 +2963,7 @@  mwl8k_cmd_rf_antenna(struct ieee80211_hw *hw, int antenna, int mask)
  * CMD_SET_BEACON.
  */
 struct mwl8k_cmd_set_beacon {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 beacon_len;
 	__u8 beacon[];
 };
@@ -2988,7 +2993,7 @@  static int mwl8k_cmd_set_beacon(struct ieee80211_hw *hw,
  * CMD_SET_PRE_SCAN.
  */
 struct mwl8k_cmd_set_pre_scan {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 } __packed;

 static int mwl8k_cmd_set_pre_scan(struct ieee80211_hw *hw)
@@ -3013,7 +3018,7 @@  static int mwl8k_cmd_set_pre_scan(struct ieee80211_hw *hw)
  * CMD_BBP_REG_ACCESS.
  */
 struct mwl8k_cmd_bbp_reg_access {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__le16 offset;
 	u8 value;
@@ -3054,7 +3059,7 @@  mwl8k_cmd_bbp_reg_access(struct ieee80211_hw *hw,
  * CMD_SET_POST_SCAN.
  */
 struct mwl8k_cmd_set_post_scan {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le32 isibss;
 	__u8 bssid[ETH_ALEN];
 } __packed;
@@ -3142,7 +3147,7 @@  static void mwl8k_update_survey(struct mwl8k_priv *priv,
  * CMD_SET_RF_CHANNEL.
  */
 struct mwl8k_cmd_set_rf_channel {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__u8 current_channel;
 	__le32 channel_flags;
@@ -3211,7 +3216,7 @@  static int mwl8k_cmd_set_rf_channel(struct ieee80211_hw *hw,
 #define MWL8K_FRAME_PROT_11N_HT_ALL			0x06

 struct mwl8k_cmd_update_set_aid {
-	struct	mwl8k_cmd_pkt header;
+	struct	mwl8k_cmd_pkt_hdr header;
 	__le16	aid;

 	 /* AP's MAC address (BSSID) */
@@ -3283,7 +3288,7 @@  mwl8k_cmd_set_aid(struct ieee80211_hw *hw,
  * CMD_SET_RATE.
  */
 struct mwl8k_cmd_set_rate {
-	struct	mwl8k_cmd_pkt header;
+	struct	mwl8k_cmd_pkt_hdr header;
 	__u8	legacy_rates[14];

 	/* Bitmap for supported MCS codes.  */
@@ -3319,7 +3324,7 @@  mwl8k_cmd_set_rate(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 #define MWL8K_FJ_BEACON_MAXLEN	128

 struct mwl8k_cmd_finalize_join {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le32 sleep_interval;	/* Number of beacon periods to sleep */
 	__u8 beacon_data[MWL8K_FJ_BEACON_MAXLEN];
 } __packed;
@@ -3358,7 +3363,7 @@  static int mwl8k_cmd_finalize_join(struct ieee80211_hw *hw, void *frame,
  * CMD_SET_RTS_THRESHOLD.
  */
 struct mwl8k_cmd_set_rts_threshold {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__le16 threshold;
 } __packed;
@@ -3388,7 +3393,7 @@  mwl8k_cmd_set_rts_threshold(struct ieee80211_hw *hw, int rts_thresh)
  * CMD_SET_SLOT.
  */
 struct mwl8k_cmd_set_slot {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__u8 short_slot;
 } __packed;
@@ -3417,7 +3422,7 @@  static int mwl8k_cmd_set_slot(struct ieee80211_hw *hw, bool short_slot_time)
  * CMD_SET_EDCA_PARAMS.
  */
 struct mwl8k_cmd_set_edca_params {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;

 	/* See MWL8K_SET_EDCA_XXX below */
 	__le16 action;
@@ -3502,7 +3507,7 @@  mwl8k_cmd_set_edca_params(struct ieee80211_hw *hw, __u8 qnum,
  * CMD_SET_WMM_MODE.
  */
 struct mwl8k_cmd_set_wmm_mode {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 } __packed;

@@ -3533,7 +3538,7 @@  static int mwl8k_cmd_set_wmm_mode(struct ieee80211_hw *hw, bool enable)
  * CMD_MIMO_CONFIG.
  */
 struct mwl8k_cmd_mimo_config {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le32 action;
 	__u8 rx_antenna_map;
 	__u8 tx_antenna_map;
@@ -3564,7 +3569,7 @@  static int mwl8k_cmd_mimo_config(struct ieee80211_hw *hw, __u8 rx, __u8 tx)
  * CMD_USE_FIXED_RATE (STA version).
  */
 struct mwl8k_cmd_use_fixed_rate_sta {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le32 action;
 	__le32 allow_rate_drop;
 	__le32 num_rates;
@@ -3606,7 +3611,7 @@  static int mwl8k_cmd_use_fixed_rate_sta(struct ieee80211_hw *hw)
  * CMD_USE_FIXED_RATE (AP version).
  */
 struct mwl8k_cmd_use_fixed_rate_ap {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le32 action;
 	__le32 allow_rate_drop;
 	__le32 num_rates;
@@ -3647,7 +3652,7 @@  mwl8k_cmd_use_fixed_rate_ap(struct ieee80211_hw *hw, int mcast, int mgmt)
  * CMD_ENABLE_SNIFFER.
  */
 struct mwl8k_cmd_enable_sniffer {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le32 action;
 } __packed;

@@ -3671,7 +3676,7 @@  static int mwl8k_cmd_enable_sniffer(struct ieee80211_hw *hw, bool enable)
 }

 struct mwl8k_cmd_update_mac_addr {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	union {
 		struct {
 			__le16 mac_type;
@@ -3756,7 +3761,7 @@  static inline int mwl8k_cmd_del_mac_addr(struct ieee80211_hw *hw,
  * CMD_SET_RATEADAPT_MODE.
  */
 struct mwl8k_cmd_set_rate_adapt_mode {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 action;
 	__le16 mode;
 } __packed;
@@ -3785,7 +3790,7 @@  static int mwl8k_cmd_set_rateadapt_mode(struct ieee80211_hw *hw, __u16 mode)
  * CMD_GET_WATCHDOG_BITMAP.
  */
 struct mwl8k_cmd_get_watchdog_bitmap {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	u8	bitmap;
 } __packed;

@@ -3865,7 +3870,7 @@  static void mwl8k_watchdog_ba_events(struct work_struct *work)
  * CMD_BSS_START.
  */
 struct mwl8k_cmd_bss_start {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le32 enable;
 } __packed;

@@ -3960,7 +3965,7 @@  struct mwl8k_destroy_ba_stream {
 } __packed;

 struct mwl8k_cmd_bastream {
-	struct mwl8k_cmd_pkt	header;
+	struct mwl8k_cmd_pkt_hdr	header;
 	__le32	action;
 	union {
 		struct mwl8k_create_ba_stream	create_params;
@@ -4070,7 +4075,7 @@  static void mwl8k_destroy_ba(struct ieee80211_hw *hw,
  * CMD_SET_NEW_STN.
  */
 struct mwl8k_cmd_set_new_stn {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;
 	__le16 aid;
 	__u8 mac_addr[6];
 	__le16 stn_id;
@@ -4206,7 +4211,7 @@  static int mwl8k_cmd_set_new_stn_del(struct ieee80211_hw *hw,
 #define MIC_KEY_LENGTH		8

 struct mwl8k_cmd_update_encryption {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;

 	__le32 action;
 	__le32 reserved;
@@ -4216,7 +4221,7 @@  struct mwl8k_cmd_update_encryption {
 } __packed;

 struct mwl8k_cmd_set_key {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;

 	__le32 action;
 	__le32 reserved;
@@ -4504,7 +4509,7 @@  struct peer_capability_info {
 } __packed;

 struct mwl8k_cmd_update_stadb {
-	struct mwl8k_cmd_pkt header;
+	struct mwl8k_cmd_pkt_hdr header;

 	/* See STADB_ACTION_TYPE */
 	__le32	action;
@@ -5174,7 +5179,7 @@  mwl8k_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 static u64 mwl8k_prepare_multicast(struct ieee80211_hw *hw,
 				   struct netdev_hw_addr_list *mc_list)
 {
-	struct mwl8k_cmd_pkt *cmd;
+	struct mwl8k_cmd_pkt_hdr *hdr;

 	/*
 	 * Synthesize and return a command packet that programs the
@@ -5183,9 +5188,9 @@  static u64 mwl8k_prepare_multicast(struct ieee80211_hw *hw,
 	 * we'll end up throwing this packet away and creating a new
 	 * one in mwl8k_configure_filter().
 	 */
-	cmd = __mwl8k_cmd_mac_multicast_adr(hw, 0, mc_list);
+	hdr = __mwl8k_cmd_mac_multicast_adr(hw, 0, mc_list);

-	return (unsigned long)cmd;
+	return (unsigned long)hdr;
 }

 static int
@@ -5234,7 +5239,7 @@  static void mwl8k_configure_filter(struct ieee80211_hw *hw,
 				   u64 multicast)
 {
 	struct mwl8k_priv *priv = hw->priv;
-	struct mwl8k_cmd_pkt *cmd = (void *)(unsigned long)multicast;
+	struct mwl8k_cmd_pkt_hdr *hdr = (void *)(unsigned long)multicast;

 	/*
 	 * AP firmware doesn't allow fine-grained control over
@@ -5242,7 +5247,7 @@  static void mwl8k_configure_filter(struct ieee80211_hw *hw,
 	 */
 	if (priv->ap_fw) {
 		*total_flags &= FIF_ALLMULTI | FIF_BCN_PRBRESP_PROMISC;
-		kfree(cmd);
+		kfree(hdr);
 		return;
 	}

@@ -5252,7 +5257,7 @@  static void mwl8k_configure_filter(struct ieee80211_hw *hw,
 	 */
 	if (*total_flags & (FIF_CONTROL | FIF_OTHER_BSS) &&
 	    mwl8k_configure_filter_sniffer(hw, changed_flags, total_flags)) {
-		kfree(cmd);
+		kfree(hdr);
 		return;
 	}

@@ -5260,7 +5265,7 @@  static void mwl8k_configure_filter(struct ieee80211_hw *hw,
 	*total_flags &= FIF_ALLMULTI | FIF_BCN_PRBRESP_PROMISC;

 	if (mwl8k_fw_lock(hw)) {
-		kfree(cmd);
+		kfree(hdr);
 		return;
 	}

@@ -5304,13 +5309,13 @@  static void mwl8k_configure_filter(struct ieee80211_hw *hw,
 	 * packets.
 	 */
 	if (*total_flags & FIF_ALLMULTI) {
-		kfree(cmd);
-		cmd = __mwl8k_cmd_mac_multicast_adr(hw, 1, NULL);
+		kfree(hdr);
+		hdr = __mwl8k_cmd_mac_multicast_adr(hw, 1, NULL);
 	}

-	if (cmd != NULL) {
-		mwl8k_post_cmd(hw, cmd);
-		kfree(cmd);
+	if (hdr) {
+		mwl8k_post_cmd(hw, hdr);
+		kfree(hdr);
 	}

 	mwl8k_fw_unlock(hw);