From mboxrd@z Thu Jan  1 00:00:00 1970
From: Al Viro <viro@ZenIV.linux.org.uk>
Subject: Re: [PATCH 1/7] fix hnode refcounting
Date: Sat, 8 Sep 2018 16:03:41 +0100
Message-ID: <20180908150340.GC19965@ZenIV.linux.org.uk>
References: <20180905190134.GQ19965@ZenIV.linux.org.uk>
 <20180905190414.5477-1-viro@ZenIV.linux.org.uk>
 <3bd95332-a12e-6226-8ac3-61e88b0f3cfd@mojatatu.com>
 <20180907023529.GV19965@ZenIV.linux.org.uk>
 <612ba054-370d-d118-b439-c68ea466eec9@mojatatu.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: netdev@vger.kernel.org, Cong Wang <xiyou.wangcong@gmail.com>,
        Jiri Pirko <jiri@resnulli.us>, stable@vger.kernel.org
To: Jamal Hadi Salim <jhs@mojatatu.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:60670 "EHLO
        ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726498AbeIHTts (ORCPT
        <rfc822;netdev@vger.kernel.org>); Sat, 8 Sep 2018 15:49:48 -0400
Content-Disposition: inline
In-Reply-To: <612ba054-370d-d118-b439-c68ea466eec9@mojatatu.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Fri, Sep 07, 2018 at 08:13:56AM -0400, Jamal Hadi Salim wrote:
> > 	} else {
> >                  bool last;
> > 
> >                  err = tfilter_del_notify(net, skb, n, tp, block,
> >                                           q, parent, fh, false, &last,
> >                                           extack);
> > How can we ever get there with NULL fh?
> > 
> 
> Try:
> tc filter delete dev $P parent ffff: protocol ip prio 10 u32
> tcm handle is 0, so will hit that code path.

Huh?  It will hit tcf_proto_destroy() (and thus u32_destroy()), but where will
it hit u32_delete()?  Sure, we have fh == NULL there; what happens next is
                if (t->tcm_handle == 0) {
                        tcf_chain_tp_remove(chain, &chain_info, tp);    
                        tfilter_notify(net, skb, n, tp, block, q, parent, fh,
                                       RTM_DELTFILTER, false);
                        tcf_proto_destroy(tp, extack);
and that's it.  IDGI...  Direct experiment shows that on e.g.
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip prio 10 u32 match ip protocol 1 0xff
tc filter delete dev eth0 parent ffff: protocol ip prio 10 u32
we get u32_destroy() called, with u32_destroy_hnode() called by it,
but no u32_delete() is called at all, let alone with ht == NULL...