Re: [PATH net] net/sched: ets: fix crash when flipping from 'strict' to 'quantum'

On Tue, Aug 31, 2021 at 11:16:44AM -0700, Cong Wang wrote:

On Tue, Aug 31, 2021 at 2:54 AM Davide Caratti [off-list ref] wrote:

quoted

hello Cong, thanks a lot for looking at this!

On Mon, Aug 30, 2021 at 05:43:09PM -0700, Cong Wang wrote:

quoted

On Tue, Aug 24, 2021 at 3:34 PM Davide Caratti [off-list ref] wrote:

[...]

quoted

quoted

quoted

Then, a call to ets_qdisc_reset() will attempt to
do list_del(&alist) with 'alist' filled with zero, hence the NULL pointer
dereference.

I am confused about how we end up having NULL in list head.

[...]

quoted

So, I can probably send a patch (for net-next, when it reopens) that removes
this INIT_LIST_HEAD() line; anyway, its presence is harmless IMO. WDYT?

Actually I am thinking about the opposite, that is, always initializing the
list head. ;) Unless we always use list_add*() before list_del(), initializing
it unconditionally is a correct fix.

uh, maybe I get the point.

we can do a INIT_LIST_HEAD(&cl[i].alist) in ets_qdisc_init() with 'i' ranging
from 0 to TCQ_ETS_MAX_BANDS - 1, then the memset() in [1] needs to be
replaced with something that clears all members of struct ets_class except
'alist'. At this point, the INIT_LIST_HEAD() line in ets_qdisc_change() can be
removed. 

I can re-run the kseltest and eventually send a patch for that (targeting
net-next, no need to rush), is that ok for you?

thanks,

-- 
davide


[1] https://elixir.bootlin.com/linux/v5.14/source/net/sched/sch_ets.c#L690