On 14/10/2026 15:58, Rand Deeb wrote:
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 <redacted>
Diagnosis and fix look correct to me.
Acked-by: Edward Cree <ecree.xilinx@gmail.com>
Sadly I don't have a working ef100 to test this on.