Skip to content

Commit

Permalink
ice: fix access-beyond-end in the switch code
Browse files Browse the repository at this point in the history
Global `-Warray-bounds` enablement revealed some problems, one of
which is the way we define and use AQC rules messages.
In fact, they have a shared header, followed by the actual message,
which can be of one of several different formats. So it is
straightforward enough to define that header as a separate struct
and then embed it into message structures as needed, but currently
all the formats reside in one union coupled with the header. Then,
the code allocates only the memory needed for a particular message
format, leaving the union potentially incomplete.
There are no actual reads or writes beyond the end of an allocated
chunk, but at the same time, the whole implementation is fragile and
backed by an equilibrium rather than strong type and memory checks.

Define the structures the other way around: one for the common
header and the rest for the actual formats with the header embedded.
There are no places where several union members would be used at the
same time anyway. This allows to use proper struct_size() and let
the compiler know what is going to be done.
Finally, unsilence `-Warray-bounds` back for ice_switch.c.

Other little things worth mentioning:
* &ice_sw_rule_vsi_list_query is not used anywhere, remove it. It's
  weird anyway to talk to hardware with purely kernel types
  (bitmaps);
* expand the ICE_SW_RULE_*_SIZE() macros to pass a structure
  variable name to struct_size() to let it do strict typechecking;
* rename ice_sw_rule_lkup_rx_tx::hdr to ::hdr_data to keep ::hdr
  for the header structure to have the same name for it constistenly
  everywhere;
* drop the duplicate of %ICE_SW_RULE_RX_TX_NO_HDR_SIZE residing in
  ice_switch.h.

Fixes: 9daf820 ("ice: Add support for switch filter programming")
Fixes: 66486d8 ("ice: replace single-element array used for C struct hack")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>
Acked-by: Tony Nguyen <anthony.l.nguyen@intel.com>
Link: https://lore.kernel.org/r/20220601105924.2841410-1-alexandr.lobakin@intel.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
  • Loading branch information
alobakin authored and Paolo Abeni committed Jun 2, 2022
1 parent c6fbbf1 commit 6e1ff61
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 139 deletions.
5 changes: 0 additions & 5 deletions drivers/net/ethernet/intel/ice/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,3 @@ ice-$(CONFIG_DCB) += ice_dcb.o ice_dcb_nl.o ice_dcb_lib.o
ice-$(CONFIG_RFS_ACCEL) += ice_arfs.o
ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o
ice-$(CONFIG_ICE_SWITCHDEV) += ice_eswitch.o

# FIXME: temporarily silence -Warray-bounds on non W=1+ builds
ifndef KBUILD_EXTRA_WARN
CFLAGS_ice_switch.o += -Wno-array-bounds
endif
58 changes: 26 additions & 32 deletions drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
Original file line number Diff line number Diff line change
Expand Up @@ -601,12 +601,30 @@ struct ice_aqc_sw_rules {
__le32 addr_low;
};

/* Add switch rule response:
* Content of return buffer is same as the input buffer. The status field and
* LUT index are updated as part of the response
*/
struct ice_aqc_sw_rules_elem_hdr {
__le16 type; /* Switch rule type, one of T_... */
#define ICE_AQC_SW_RULES_T_LKUP_RX 0x0
#define ICE_AQC_SW_RULES_T_LKUP_TX 0x1
#define ICE_AQC_SW_RULES_T_LG_ACT 0x2
#define ICE_AQC_SW_RULES_T_VSI_LIST_SET 0x3
#define ICE_AQC_SW_RULES_T_VSI_LIST_CLEAR 0x4
#define ICE_AQC_SW_RULES_T_PRUNE_LIST_SET 0x5
#define ICE_AQC_SW_RULES_T_PRUNE_LIST_CLEAR 0x6
__le16 status;
} __packed __aligned(sizeof(__le16));

/* Add/Update/Get/Remove lookup Rx/Tx command/response entry
* This structures describes the lookup rules and associated actions. "index"
* is returned as part of a response to a successful Add command, and can be
* used to identify the rule for Update/Get/Remove commands.
*/
struct ice_sw_rule_lkup_rx_tx {
struct ice_aqc_sw_rules_elem_hdr hdr;

__le16 recipe_id;
#define ICE_SW_RECIPE_LOGICAL_PORT_FWD 10
/* Source port for LOOKUP_RX and source VSI in case of LOOKUP_TX */
Expand Down Expand Up @@ -683,14 +701,16 @@ struct ice_sw_rule_lkup_rx_tx {
* lookup-type
*/
__le16 hdr_len;
u8 hdr[];
};
u8 hdr_data[];
} __packed __aligned(sizeof(__le16));

/* Add/Update/Remove large action command/response entry
* "index" is returned as part of a response to a successful Add command, and
* can be used to identify the action for Update/Get/Remove commands.
*/
struct ice_sw_rule_lg_act {
struct ice_aqc_sw_rules_elem_hdr hdr;

__le16 index; /* Index in large action table */
__le16 size;
/* Max number of large actions */
Expand Down Expand Up @@ -744,45 +764,19 @@ struct ice_sw_rule_lg_act {
#define ICE_LG_ACT_STAT_COUNT_S 3
#define ICE_LG_ACT_STAT_COUNT_M (0x7F << ICE_LG_ACT_STAT_COUNT_S)
__le32 act[]; /* array of size for actions */
};
} __packed __aligned(sizeof(__le16));

/* Add/Update/Remove VSI list command/response entry
* "index" is returned as part of a response to a successful Add command, and
* can be used to identify the VSI list for Update/Get/Remove commands.
*/
struct ice_sw_rule_vsi_list {
struct ice_aqc_sw_rules_elem_hdr hdr;

__le16 index; /* Index of VSI/Prune list */
__le16 number_vsi;
__le16 vsi[]; /* Array of number_vsi VSI numbers */
};

/* Query VSI list command/response entry */
struct ice_sw_rule_vsi_list_query {
__le16 index;
DECLARE_BITMAP(vsi_list, ICE_MAX_VSI);
} __packed;

/* Add switch rule response:
* Content of return buffer is same as the input buffer. The status field and
* LUT index are updated as part of the response
*/
struct ice_aqc_sw_rules_elem {
__le16 type; /* Switch rule type, one of T_... */
#define ICE_AQC_SW_RULES_T_LKUP_RX 0x0
#define ICE_AQC_SW_RULES_T_LKUP_TX 0x1
#define ICE_AQC_SW_RULES_T_LG_ACT 0x2
#define ICE_AQC_SW_RULES_T_VSI_LIST_SET 0x3
#define ICE_AQC_SW_RULES_T_VSI_LIST_CLEAR 0x4
#define ICE_AQC_SW_RULES_T_PRUNE_LIST_SET 0x5
#define ICE_AQC_SW_RULES_T_PRUNE_LIST_CLEAR 0x6
__le16 status;
union {
struct ice_sw_rule_lkup_rx_tx lkup_tx_rx;
struct ice_sw_rule_lg_act lg_act;
struct ice_sw_rule_vsi_list vsi_list;
struct ice_sw_rule_vsi_list_query vsi_list_query;
} __packed pdata;
};
} __packed __aligned(sizeof(__le16));

/* Query PFC Mode (direct 0x0302)
* Set PFC Mode (direct 0x0303)
Expand Down
Loading

0 comments on commit 6e1ff61

Please sign in to comment.