Re: [PATCH net v2] net/sched: cls_fw: fix NULL dereference of "old" filters before change()

[Davide Caratti <dcaratti@redhat.com>]

On Fri, Apr 03, 2026 at 02:59:22PM -0400, Jamal Hadi Salim wrote:
> On Fri, Apr 3, 2026 at 12:04 PM Davide Caratti <dcaratti@redhat.com> wrote:
> >
> > Like pointed out by Sashiko [1],
> 
> Just found out about this ;-> So bye-bye old AI? ;->
> I must say, finding this specific bug is impressive.

[...] 

> Observation on consistency pov:
> You will always get a "q" when you invoke tcf_block_q() if there are
> no shared blocks attached. But that doesnt mean the "create"
> configuration is complete; it is only complete if step 3 above
> completes. This is because we are not sure if change() will result in
> "old" or "new" lookup setup. So the check you added for q may be
> inconsistent from that perspective and fw should have returned -1 like
> all classifiers...

"Hello Jamal, you are absolutely right! (emoji with fire) However, ..."  :)

> I cant think of a simple solution to verify if the config is
> "inconsistent" other than to add something that gets checked in the
> datapath (and when absent, return -1)

^^ This. Specifically for cls_fw, that would mean converting fw_change() to
allocate some control data also for the "old" uapi, and I think it's too
much effort for the legacy.

IIUC the inconsistent behavior is: for a small amount of time, fwmark
classifier used in the "old" way would classify also when the filter's
'handle' is not zero.

> From that perspective, your check is not catastrophic, so it may be ok.
> At minimal the Fixes: needs to change to Vlads commit? Good news is
> only fw _seems_ to suffer from this challenge...

This is also what I understood by reading the code. Sure, I will edit
the Fixes: tag. Thanks for reading!

-- 
davide