On Thu 02 May 2019 at 03:48, Cong Wang [off-list ref] wrote:

On Wed, May 1, 2019 at 2:27 AM Matteo Croce [off-list ref] wrote:

quoted

On Tue, Apr 30, 2019 at 11:25 PM Cong Wang [off-list ref] wrote:

quoted

On Mon, Apr 29, 2019 at 10:38 AM Matteo Croce [off-list ref] wrote:

quoted

When a matchall classifier is added, there is a small time interval in
which tp->root is NULL. If we receive a packet in this small time slice
a NULL pointer dereference will happen, leading to a kernel panic:

Hmm, why not just check tp->root against NULL in mall_classify()?

Also, which is the offending commit here? Please add a Fixes: tag.

Thanks.

Hi,

I just want to avoid an extra check which would be made for every packet.
Probably the benefit over a check is negligible, but it's still a
per-packet thing.
If you prefer a simple check, I can make a v2 that way.

Yeah, I think that is better, you can add an unlikely() for performance
concern, as NULL is a rare case.

quoted

For the fixes tag, I didn't put it as I'm not really sure about the
offending commit. I guess it's the following, what do you think?

commit ed76f5edccc98fa66f2337f0b3b255d6e1a568b7
Author: Vlad Buslov [off-list ref]
Date:   Mon Feb 11 10:55:38 2019 +0200

    net: sched: protect filter_chain list with filter_chain_lock mutex

I think you are right, this is the commit introduced the code
that inserts the tp before fully initializing it. Please Cc Vlad
for your v2, in case we blame a wrong commit here.


BTW, it looks like cls_cgroup needs a same fix. Please audit
other tc filters as well.

Thanks!

Sorry for late response. This is indeed the offending commit that should
be referenced by fixes tag.

Thanks for fixing this, Matteo!