hi Po Liu,
On Fri, 2020-05-29 at 02:43 +0000, Po Liu wrote:
> Can you share the test step?
sure, an invalid value of the control action is sufficient:
# tc action add action gate index 2 clockid CLOCK_TAI goto chain 42
> Clockid by default is set with CLOCK_TAI.
not in the error path of tcf_gate_init(), see below:
> And INIT_LIST_HEAD() also called in the init.
...ditto. In the error path of tcf_gate_init(), these two initializations
are not done. Looking at the call trace, validation of the control action
fails here:
365 err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
366 if (err < 0)
367 goto release_idr;
then, the execution jumps to 'release_idr' thus skipping INIT_LIST_HEAD()
and hrtimer_init():
442 release_idr:
443 tcf_idr_release(*a, bind);
444 return err;
because of this, tcf_gate_cleanup() is invoked with
'to_gate(*a)->param.entries'
all filled with zeros, and the same applies to
'to_gate(*a)->hitimer'
and
'to_gate(*a)->param.tfcg_clockid'
> So I think maybe there is better method to avoid the duplicated code.
I'm not sure of what duplication you are referring to, but I suspect it's
those to_gate(*a) inside the if (ret == ACT_P_CREATED) { ... } statement:
I'm sending right now a v2 where I moved the assignment of 'gact' earlier.
Looking again at the error path of tcf_gate_init(), I suspect there is
another bug: the validation of 'tcfg_cycletime' and 'TCA_GATE_ENTRY_LIST'
is suspicious, because it overwrites the action's configuration with wrong
ones, thus causing semi-configured rules.
But it's unrelated to this kernel panic, so probably it deserves a
separate patch (and moreover, I don't have yet scripts that to verify it).
But I can follow-up on this in the next days, if you want.
thanks for looking at this,
--
davide
