Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 30 Mar 2021 04:25:12 +0530
From:   Kumar Kartikeya Dwivedi <memxor@gmail.com>
To:     Vlad Buslov <vladbu@nvidia.com>
Cc:     netdev@vger.kernel.org, Vlad Buslov <vladbu@mellanox.com>,
        Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Jiri Pirko <jiri@resnulli.us>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] net: sched: extend lifetime of new action in replace
 mode
Message-ID: <20210329225512.voaibzkfmyodcxv6@apollo>
References: <20210328064555.93365-1-memxor@gmail.com>
 <ygnhk0pqgwjb.fsf@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ygnhk0pqgwjb.fsf@nvidia.com>
Precedence: bulk

On Mon, Mar 29, 2021 at 02:35:12PM IST, Vlad Buslov wrote:
> it seems that there are two ways actions are overwritten/deleted:
>
> 1. Directly through action API, which is still serialized by rtnl lock.
>
> 2. Classifier API, which doesn't use rtnl lock anymore and can execute
> concurrently.
>
> Actions created by path 2 also have their bind count incremented which
> prevents them from being deleted by path 1 and cls API can only deleted
> them together with classifier that points to them.
>
> [...]
> So, what happens here is actions were 'deleted' concurrently (their
> tcfa_refcnt decremented by 1)? tcf_action_put_many() will decrement
> refcnt again, it will reach 0, actions get actually deleted and
> tcf_exts_validate() returns with non-error code, but exts->actions
> pointing to freed memory? Doesn't look like the patches fixes the
> described issue, unless I'm missing something.
>

Thanks for the review and comments.

You are absolutely right. This patch was totally broken. Your feedback however
was quite helpful in understanding the code. I sent a v2, please lmk if it's
correct (also with a hopefully thorough description of the problem & solution).

--
Kartikeya