Re: [Patch net 2/2] net_sched: commit action insertions together

[Vlad Buslov <vlad@buslov.dev>]

On Wed 23 Sep 2020 at 06:56, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> syzbot is able to trigger a failure case inside the loop in
> tcf_action_init(), and when this happens we clean up with
> tcf_action_destroy(). But, as these actions are already inserted
> into the global IDR, other parallel process could free them
> before tcf_action_destroy(), then we will trigger a use-after-free.
>
> Fix this by deferring the insertions even later, after the loop,
> and committing all the insertions in a separate loop, so we will
> never fail in the middle of the insertions any more.
>
> One side effect is that the window between alloction and final
> insertion becomes larger, now it is more likely that the loop in
> tcf_del_walker() sees the placeholder -EBUSY pointer. So we have
> to check for error pointer in tcf_del_walker().
>
> Reported-and-tested-by: syzbot+2287853d392e4b42374a@syzkaller.appspotmail.com
> Fixes: 0190c1d452a9 ("net: sched: atomically check-allocate action")
> Cc: Vlad Buslov <vladbu@mellanox.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---

Reviewed-by: Vlad Buslov <vlad@buslov.dev>