On Sat, Oct 29, 2022 at 12:12:26AM -0700, Kees Cook wrote:
> On Thu, Oct 27, 2022 at 03:20:47PM -0500, Gustavo A. R. Silva wrote:
> > When built with Control Flow Integrity, function prototypes between
> > caller and function declaration must match. These mismatches are visible
> > at compile time with the new -Wcast-function-type-strict in Clang[1].
> >
> > Fix a total of 227 warnings like these:
> >
> > drivers/net/ethernet/brocade/bna/bna_enet.c:519:3: warning: cast from 'void (*)(struct bna_ethport *, enum bna_ethport_event)' to 'bfa_fsm_t' (aka 'void (*)(void *, int)') converts to incompatible function type [-Wcast-function-type-strict]
> > bfa_fsm_set_state(ethport, bna_ethport_sm_down);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > The bna state machine code heavily overloads its state machine functions,
> > so these have been separated into their own sets of structs, enums,
> > typedefs, and helper functions. There are almost zero binary code changes,
> > all seem to be related to header file line numbers changing, or the
> > addition of the new stats helper.
>
> This looks like it borrowed from
> https://lore.kernel.org/linux-hardening/20220929230334.2109344-1-keescook@chromium.org/
> Nice to get a couple hundred more fixed. :)
Yep; you're right. That's exactly the patch I was staring at
while doing these changes. :)
>
> > [1] https://reviews.llvm.org/D134831
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> > Changes in v2:
> > - None. This patch is new in the series.
>
> This is relatively stand-alone (not an iw_handler patch), so it could
> also go separately too.
My criteria here was that all these patches avoid clashing function
prototypes. So, they could be put together into a series, regardless
if they are "iw_handler" related patches.
> Reviewed-by: Kees Cook <keescook@chromium.org>
Thanks!
--
Gustavo
@@ -18,15 +18,43 @@
/* BFA state machine interfaces */
-typedef void (*bfa_sm_t)(void *sm, int event);
-
/* For converting from state machine function to state encoding. */
-struct bfa_sm_table {
- bfa_sm_t sm; /*!< state machine function */
- int state; /*!< state machine encoding */
- char *name; /*!< state name for display */
-};
-#define BFA_SM(_sm) ((bfa_sm_t)(_sm))
+#define BFA_SM_TABLE(n, s, e, t) \
+struct s; \
+enum e; \
+typedef void (*t)(struct s *, enum e); \
+ \
+struct n ## _sm_table_s { \
+ t sm; /* state machine function */ \
+ int state; /* state machine encoding */ \
+ char *name; /* state name for display */ \
+}; \
+ \
+static inline int \
+n ## _sm_to_state(struct n ## _sm_table_s *smt, t sm) \
+{ \
+ int i = 0; \
+ \
+ while (smt[i].sm && smt[i].sm != sm) \
+ i++; \
+ return smt[i].state; \
+}
+
+BFA_SM_TABLE(iocpf, bfa_iocpf, iocpf_event, bfa_fsm_iocpf_t)
+BFA_SM_TABLE(ioc, bfa_ioc, ioc_event, bfa_fsm_ioc_t)
+BFA_SM_TABLE(cmdq, bfa_msgq_cmdq, cmdq_event, bfa_fsm_msgq_cmdq_t)
+BFA_SM_TABLE(rspq, bfa_msgq_rspq, rspq_event, bfa_fsm_msgq_rspq_t)
+
+BFA_SM_TABLE(ioceth, bna_ioceth, bna_ioceth_event, bna_fsm_ioceth_t)
+BFA_SM_TABLE(enet, bna_enet, bna_enet_event, bna_fsm_enet_t)
+BFA_SM_TABLE(ethport, bna_ethport, bna_ethport_event, bna_fsm_ethport_t)
+BFA_SM_TABLE(tx, bna_tx, bna_tx_event, bna_fsm_tx_t)
+BFA_SM_TABLE(rxf, bna_rxf, bna_rxf_event, bna_fsm_rxf_t)
+BFA_SM_TABLE(rx, bna_rx, bna_rx_event, bna_fsm_rx_t)
+
+#undef BFA_SM_TABLE
+
+#define BFA_SM(_sm) (_sm)
/* State machine with entry actions. */
typedef void (*bfa_fsm_t)(void *fsm, int event);
@@ -41,24 +69,12 @@ typedef void (*bfa_fsm_t)(void *fsm, int event);
static void oc ## _sm_ ## st ## _entry(otype * fsm)
#define bfa_fsm_set_state(_fsm, _state) do { \
- (_fsm)->fsm = (bfa_fsm_t)(_state); \
+ (_fsm)->fsm = (_state); \
_state ## _entry(_fsm); \
} while (0)
#define bfa_fsm_send_event(_fsm, _event) ((_fsm)->fsm((_fsm), (_event)))
-#define bfa_fsm_cmp_state(_fsm, _state) \
- ((_fsm)->fsm == (bfa_fsm_t)(_state))
-
-static inline int
-bfa_sm_to_state(const struct bfa_sm_table *smt, bfa_sm_t sm)
-{
- int i = 0;
-
- while (smt[i].sm && smt[i].sm != sm)
- i++;
- return smt[i].state;
-}
-
+#define bfa_fsm_cmp_state(_fsm, _state) ((_fsm)->fsm == (_state))
/* Generic wait counter. */
typedef void (*bfa_wc_resume_t) (void *cbarg);
@@ -114,7 +114,7 @@ bfa_fsm_state_decl(bfa_ioc, disabling, struct bfa_ioc, enum ioc_event);
bfa_fsm_state_decl(bfa_ioc, disabled, struct bfa_ioc, enum ioc_event);
bfa_fsm_state_decl(bfa_ioc, hwfail, struct bfa_ioc, enum ioc_event);
-static struct bfa_sm_table ioc_sm_table[] = {
+static struct ioc_sm_table_s ioc_sm_table[] = {
{BFA_SM(bfa_ioc_sm_uninit), BFA_IOC_UNINIT},
{BFA_SM(bfa_ioc_sm_reset), BFA_IOC_RESET},
{BFA_SM(bfa_ioc_sm_enabling), BFA_IOC_ENABLING},
@@ -183,7 +183,7 @@ bfa_fsm_state_decl(bfa_iocpf, disabling_sync, struct bfa_iocpf,
enum iocpf_event);
bfa_fsm_state_decl(bfa_iocpf, disabled, struct bfa_iocpf, enum iocpf_event);
-static struct bfa_sm_table iocpf_sm_table[] = {
+static struct iocpf_sm_table_s iocpf_sm_table[] = {
{BFA_SM(bfa_iocpf_sm_reset), BFA_IOCPF_RESET},
{BFA_SM(bfa_iocpf_sm_fwcheck), BFA_IOCPF_FWMISMATCH},
{BFA_SM(bfa_iocpf_sm_mismatch), BFA_IOCPF_FWMISMATCH},
@@ -2860,12 +2860,12 @@ static enum bfa_ioc_state
bfa_ioc_get_state(struct bfa_ioc *ioc)
{
enum bfa_iocpf_state iocpf_st;
- enum bfa_ioc_state ioc_st = bfa_sm_to_state(ioc_sm_table, ioc->fsm);
+ enum bfa_ioc_state ioc_st = ioc_sm_to_state(ioc_sm_table, ioc->fsm);
if (ioc_st == BFA_IOC_ENABLING ||
ioc_st == BFA_IOC_FAIL || ioc_st == BFA_IOC_INITFAIL) {
- iocpf_st = bfa_sm_to_state(iocpf_sm_table, ioc->iocpf.fsm);
+ iocpf_st = iocpf_sm_to_state(iocpf_sm_table, ioc->iocpf.fsm);
switch (iocpf_st) {
case BFA_IOCPF_SEMWAIT:
@@ -2983,7 +2983,7 @@ bfa_nw_iocpf_timeout(struct bfa_ioc *ioc)
{
enum bfa_iocpf_state iocpf_st;
- iocpf_st = bfa_sm_to_state(iocpf_sm_table, ioc->iocpf.fsm);
+ iocpf_st = iocpf_sm_to_state(iocpf_sm_table, ioc->iocpf.fsm);
if (iocpf_st == BFA_IOCPF_HWINIT)
bfa_ioc_poll_fwinit(ioc);
@@ -147,16 +147,20 @@ struct bfa_ioc_notify {
(__notify)->cbarg = (__cbarg); \
} while (0)
+enum iocpf_event;
+
struct bfa_iocpf {
- bfa_fsm_t fsm;
+ void (*fsm)(struct bfa_iocpf *s, enum iocpf_event e);
struct bfa_ioc *ioc;
bool fw_mismatch_notified;
bool auto_recover;
u32 poll_time;
};
+enum ioc_event;
+
struct bfa_ioc {
- bfa_fsm_t fsm;
+ void (*fsm)(struct bfa_ioc *s, enum ioc_event e);
struct bfa *bfa;
struct bfa_pcidev pcidev;
struct timer_list ioc_timer;
@@ -55,8 +55,10 @@ enum bfa_msgq_cmdq_flags {
BFA_MSGQ_CMDQ_F_DB_UPDATE = 1,
};
+enum cmdq_event;
+
struct bfa_msgq_cmdq {
- bfa_fsm_t fsm;
+ void (*fsm)(struct bfa_msgq_cmdq *s, enum cmdq_event e);
enum bfa_msgq_cmdq_flags flags;
u16 producer_index;
@@ -81,8 +83,10 @@ enum bfa_msgq_rspq_flags {
typedef void (*bfa_msgq_mcfunc_t)(void *cbarg, struct bfi_msgq_mhdr *mhdr);
+enum rspq_event;
+
struct bfa_msgq_rspq {
- bfa_fsm_t fsm;
+ void (*fsm)(struct bfa_msgq_rspq *s, enum rspq_event e);
enum bfa_msgq_rspq_flags flags;
u16 producer_index;
@@ -1257,7 +1257,7 @@ bna_enet_mtu_get(struct bna_enet *enet)
void
bna_enet_enable(struct bna_enet *enet)
{
- if (enet->fsm != (bfa_sm_t)bna_enet_sm_stopped)
+ if (enet->fsm != bna_enet_sm_stopped)
return;
enet->flags |= BNA_ENET_F_ENABLED;
@@ -1751,12 +1751,12 @@ bna_ioceth_uninit(struct bna_ioceth *ioceth)
void
bna_ioceth_enable(struct bna_ioceth *ioceth)
{
- if (ioceth->fsm == (bfa_fsm_t)bna_ioceth_sm_ready) {
+ if (ioceth->fsm == bna_ioceth_sm_ready) {
bnad_cb_ioceth_ready(ioceth->bna->bnad);
return;
}
- if (ioceth->fsm == (bfa_fsm_t)bna_ioceth_sm_stopped)
+ if (ioceth->fsm == bna_ioceth_sm_stopped)
bfa_fsm_send_event(ioceth, IOCETH_E_ENABLE);
}
@@ -1956,7 +1956,7 @@ static void
bna_rx_stop(struct bna_rx *rx)
{
rx->rx_flags &= ~BNA_RX_F_ENET_STARTED;
- if (rx->fsm == (bfa_fsm_t) bna_rx_sm_stopped)
+ if (rx->fsm == bna_rx_sm_stopped)
bna_rx_mod_cb_rx_stopped(&rx->bna->rx_mod, rx);
else {
rx->stop_cbfn = bna_rx_mod_cb_rx_stopped;
@@ -2535,7 +2535,7 @@ bna_rx_destroy(struct bna_rx *rx)
void
bna_rx_enable(struct bna_rx *rx)
{
- if (rx->fsm != (bfa_sm_t)bna_rx_sm_stopped)
+ if (rx->fsm != bna_rx_sm_stopped)
return;
rx->rx_flags |= BNA_RX_F_ENABLED;
@@ -3523,7 +3523,7 @@ bna_tx_destroy(struct bna_tx *tx)
void
bna_tx_enable(struct bna_tx *tx)
{
- if (tx->fsm != (bfa_sm_t)bna_tx_sm_stopped)
+ if (tx->fsm != bna_tx_sm_stopped)
return;
tx->flags |= BNA_TX_F_ENABLED;
@@ -312,8 +312,10 @@ struct bna_attr {
/* IOCEth */
+enum bna_ioceth_event;
+
struct bna_ioceth {
- bfa_fsm_t fsm;
+ void (*fsm)(struct bna_ioceth *s, enum bna_ioceth_event e);
struct bfa_ioc ioc;
struct bna_attr attr;
@@ -334,8 +336,10 @@ struct bna_pause_config {
enum bna_status rx_pause;
};
+enum bna_enet_event;
+
struct bna_enet {
- bfa_fsm_t fsm;
+ void (*fsm)(struct bna_enet *s, enum bna_enet_event e);
enum bna_enet_flags flags;
enum bna_enet_type type;
@@ -360,8 +364,10 @@ struct bna_enet {
/* Ethport */
+enum bna_ethport_event;
+
struct bna_ethport {
- bfa_fsm_t fsm;
+ void (*fsm)(struct bna_ethport *s, enum bna_ethport_event e);
enum bna_ethport_flags flags;
enum bna_link_status link_status;
@@ -454,13 +460,16 @@ struct bna_txq {
};
/* Tx object */
+
+enum bna_tx_event;
+
struct bna_tx {
/* This should be the first one */
struct list_head qe;
int rid;
int hw_id;
- bfa_fsm_t fsm;
+ void (*fsm)(struct bna_tx *s, enum bna_tx_event e);
enum bna_tx_flags flags;
enum bna_tx_type type;
@@ -698,8 +707,11 @@ struct bna_rxp {
};
/* RxF structure (hardware Rx Function) */
+
+enum bna_rxf_event;
+
struct bna_rxf {
- bfa_fsm_t fsm;
+ void (*fsm)(struct bna_rxf *s, enum bna_rxf_event e);
struct bfa_msgq_cmd_entry msgq_cmd;
union {
@@ -769,13 +781,16 @@ struct bna_rxf {
};
/* Rx object */
+
+enum bna_rx_event;
+
struct bna_rx {
/* This should be the first one */
struct list_head qe;
int rid;
int hw_id;
- bfa_fsm_t fsm;
+ void (*fsm)(struct bna_rx *s, enum bna_rx_event e);
enum bna_rx_type type;