efx_tc_flower_replace_foreign() walks the rule's actions with an 'act' cursor. After a redirect action it adds the action set to the rule and sets the cursor to NULL: list_add_tail(&act->list, &rule->acts.list); act = NULL; if (fa->id == FLOW_ACTION_REDIRECT) break; /* end of the line */ The break only leaves the switch statement, so flow_action_for_each() carries on with the next action while act is NULL. A following redirect or mirred action then does "save = *act", and a following tunnel decap action passes act to efx_tc_flower_action_order_ok(), which reads act->decap. Both dereference a NULL pointer. The loop is missing the check that efx_tc_flower_replace() performs at the start of every iteration for the same cursor: reject the rule if an action follows a non-pipe action. The foreign variant was added without it. As a result, a TC flower rule on a tunnel netdevice that redirects to an sfc representor and lists another redirect, mirred or tunnel decap action after the first redirect causes a kernel NULL pointer dereference. It happens in the context of the task adding the filter, with efx->tc->mutex held. Adding such a rule requires CAP_NET_ADMIN. Add the same check at the top of the action loop in efx_tc_flower_replace_foreign(), returning -EINVAL with the same extack message as efx_tc_flower_replace(). The release path already copes with act being NULL. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 17654d84b47c ("sfc: add offloading of 'foreign' TC (decap) rules") Signed-off-by: Rand Deeb --- drivers/net/ethernet/sfc/tc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c index 25341acfb..4a740da04 100644 --- a/drivers/net/ethernet/sfc/tc.c +++ b/drivers/net/ethernet/sfc/tc.c @@ -1828,6 +1828,13 @@ static int efx_tc_flower_replace_foreign(struct efx_nic *efx, flow_action_for_each(i, fa, &fr->action) { struct efx_tc_action_set save; + if (!act) { + /* more actions after a non-pipe action */ + NL_SET_ERR_MSG_MOD(extack, "Action follows non-pipe action"); + rc = -EINVAL; + goto release; + } + switch (fa->id) { case FLOW_ACTION_REDIRECT: case FLOW_ACTION_MIRRED: base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907 -- 2.39.5