Subject: Re: [PATCH] net: openvswitch: fix flow memory leak in
 ovs_flow_cmd_new
To:     Simon Horman <simon.horman@corigine.com>
Cc:     Pravin B Shelar <pshelar@ovn.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Eelco Chaudron <echaudro@redhat.com>, netdev@vger.kernel.org,
        dev@openvswitch.org, linux-kernel@vger.kernel.org,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        lvc-project@linuxtesting.org
References: <20230131191939.901288-1-pchelkin@ispras.ru>
 <Y9qI/vBRPlDFwkAh@corigine.com>
From:   Fedor Pchelkin <pchelkin@ispras.ru>
Message-ID: <a0be13d0-22d5-b92b-9fed-4faeed30fdce@ispras.ru>
Date:   Wed, 1 Feb 2023 19:28:09 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <Y9qI/vBRPlDFwkAh@corigine.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/1/23 6:45 PM, Simon Horman wrote:
> I see this would work by virtue of kfree(key) doing nothing
> of key is NULL, the error case in question. And that otherwise key is
> non-NULL if this path is hit.
> 
> However, the idiomatic approach to error handling is for the error path
> to unwind resource allocations in the reverse order that they were made.
> And for goto labels to control how far to unwind.
> 

You are right, thanks. Have to keep 'goto' structured, otherwise there
would be a 'goto' mess.

> So I think the following would be more in keeping with the intention of the
> code. Even if it is a somewhat more verbose change.
> 
> *compile tested only!*

I'll test this on error paths and resend the patch.