Re: general protection fault in qdisc_put

From: Cong Wang
Date: Mon Sep 09 2019 - 19:14:50 EST


On Sun, Sep 8, 2019 at 10:19 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> I see two solutions:
>
> (a) move the
>
> q->qdisc = &noop_qdisc;
>
> up earlier in sfb_init(), so that qdisc is always initialized
> after sfb_init(), even on failure.
>
> (b) just make qdisc_put(NULL) just silently work as a no-op.
>
> (c) change all the semantics to not call ->destroy if ->init failed.
>
> Honestly, (a) seems very fragile - do all the other init routines do
> this? And (c) sounds like a big change, and very fragile too.
>
> So I'd suggest that qdisc_put() be made to just ignore a NULL pointer
> (and maybe an error pointer too?).

I think (a) is the best solution here.

(c) changes too much, we already rely on this behavior.

(b) is not bad either, just very slightly more risky.

Alternatively, we can add a quick NULL check inside
sfb_destroy().

I can send out a patch if you don't.

Thanks for looking at this!