Commit 6e1ff618 authored by Alexander Lobakin's avatar Alexander Lobakin Committed by Paolo Abeni

ice: fix access-beyond-end in the switch code

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: 9daf8208 ("ice: Add support for switch filter programming")
Fixes: 66486d89 ("ice: replace single-element array used for C struct hack")
Signed-off-by: default avatarAlexander Lobakin <alexandr.lobakin@intel.com>
Reviewed-by: default avatarMarcin Szycik <marcin.szycik@linux.intel.com>
Acked-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
Link: https://lore.kernel.org/r/20220601105924.2841410-1-alexandr.lobakin@intel.comSigned-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent c6fbbf1e
...@@ -47,8 +47,3 @@ ice-$(CONFIG_DCB) += ice_dcb.o ice_dcb_nl.o ice_dcb_lib.o ...@@ -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_RFS_ACCEL) += ice_arfs.o
ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o
ice-$(CONFIG_ICE_SWITCHDEV) += ice_eswitch.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
...@@ -601,12 +601,30 @@ struct ice_aqc_sw_rules { ...@@ -601,12 +601,30 @@ struct ice_aqc_sw_rules {
__le32 addr_low; __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 /* Add/Update/Get/Remove lookup Rx/Tx command/response entry
* This structures describes the lookup rules and associated actions. "index" * 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 * 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. * used to identify the rule for Update/Get/Remove commands.
*/ */
struct ice_sw_rule_lkup_rx_tx { struct ice_sw_rule_lkup_rx_tx {
struct ice_aqc_sw_rules_elem_hdr hdr;
__le16 recipe_id; __le16 recipe_id;
#define ICE_SW_RECIPE_LOGICAL_PORT_FWD 10 #define ICE_SW_RECIPE_LOGICAL_PORT_FWD 10
/* Source port for LOOKUP_RX and source VSI in case of LOOKUP_TX */ /* Source port for LOOKUP_RX and source VSI in case of LOOKUP_TX */
...@@ -683,14 +701,16 @@ struct ice_sw_rule_lkup_rx_tx { ...@@ -683,14 +701,16 @@ struct ice_sw_rule_lkup_rx_tx {
* lookup-type * lookup-type
*/ */
__le16 hdr_len; __le16 hdr_len;
u8 hdr[]; u8 hdr_data[];
}; } __packed __aligned(sizeof(__le16));
/* Add/Update/Remove large action command/response entry /* Add/Update/Remove large action command/response entry
* "index" is returned as part of a response to a successful Add command, and * "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. * can be used to identify the action for Update/Get/Remove commands.
*/ */
struct ice_sw_rule_lg_act { struct ice_sw_rule_lg_act {
struct ice_aqc_sw_rules_elem_hdr hdr;
__le16 index; /* Index in large action table */ __le16 index; /* Index in large action table */
__le16 size; __le16 size;
/* Max number of large actions */ /* Max number of large actions */
...@@ -744,45 +764,19 @@ struct ice_sw_rule_lg_act { ...@@ -744,45 +764,19 @@ struct ice_sw_rule_lg_act {
#define ICE_LG_ACT_STAT_COUNT_S 3 #define ICE_LG_ACT_STAT_COUNT_S 3
#define ICE_LG_ACT_STAT_COUNT_M (0x7F << ICE_LG_ACT_STAT_COUNT_S) #define ICE_LG_ACT_STAT_COUNT_M (0x7F << ICE_LG_ACT_STAT_COUNT_S)
__le32 act[]; /* array of size for actions */ __le32 act[]; /* array of size for actions */
}; } __packed __aligned(sizeof(__le16));
/* Add/Update/Remove VSI list command/response entry /* Add/Update/Remove VSI list command/response entry
* "index" is returned as part of a response to a successful Add command, and * "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. * can be used to identify the VSI list for Update/Get/Remove commands.
*/ */
struct ice_sw_rule_vsi_list { struct ice_sw_rule_vsi_list {
struct ice_aqc_sw_rules_elem_hdr hdr;
__le16 index; /* Index of VSI/Prune list */ __le16 index; /* Index of VSI/Prune list */
__le16 number_vsi; __le16 number_vsi;
__le16 vsi[]; /* Array of number_vsi VSI numbers */ __le16 vsi[]; /* Array of number_vsi VSI numbers */
}; } __packed __aligned(sizeof(__le16));
/* 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;
};
/* Query PFC Mode (direct 0x0302) /* Query PFC Mode (direct 0x0302)
* Set PFC Mode (direct 0x0303) * Set PFC Mode (direct 0x0303)
......
This diff is collapsed.
...@@ -23,9 +23,6 @@ ...@@ -23,9 +23,6 @@
#define ICE_PROFID_IPV6_GTPU_TEID 46 #define ICE_PROFID_IPV6_GTPU_TEID 46
#define ICE_PROFID_IPV6_GTPU_IPV6_TCP_INNER 70 #define ICE_PROFID_IPV6_GTPU_IPV6_TCP_INNER 70
#define ICE_SW_RULE_RX_TX_NO_HDR_SIZE \
(offsetof(struct ice_aqc_sw_rules_elem, pdata.lkup_tx_rx.hdr))
/* VSI context structure for add/get/update/free operations */ /* VSI context structure for add/get/update/free operations */
struct ice_vsi_ctx { struct ice_vsi_ctx {
u16 vsi_num; u16 vsi_num;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment