netdev - Re: [PATCH net 1/2] net: ethernet: mtk_eth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <795e8a96-7f9d-640d-465d-670ae9efbab0@nbd.name>
Date:   Thu, 23 Mar 2023 09:55:44 +0100
From:   Felix Fietkau <nbd@....name>
To:     Jakub Kicinski <kuba@...nel.org>,
        Simon Horman <simon.horman@...igine.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net 1/2] net: ethernet: mtk_eth_soc: fix flow block
 refcounting

On 23.03.23 06:37, Jakub Kicinski wrote:
> On Wed, 22 Mar 2023 15:33:52 +0100 Simon Horman wrote:
>> On Tue, Mar 21, 2023 at 02:37:18PM +0100, Felix Fietkau wrote:
>> > Since we call flow_block_cb_decref on FLOW_BLOCK_UNBIND, we need to call
>> > flow_block_cb_incref unconditionally, even for a newly allocated cb.
>> > Fixes a use-after-free bug. Also fix the accidentally inverted refcount
>> > check on unbind.  
>> 
>> Firstly, it's usually best to have one fix per patch.
> 
> Or at least reword the commit message to make it look like it's fixing
> the refcounting logic?
I thought that was clear in the title already :)
I've considered splitting the patch in two, but decided against it, 
because it could cause bisect issues.
Right now the accidentally inverted logic is preventing the 
use-after-free bug from showing up with a single flow block, so that 
would break if I only fix one part without the other.

Will send v2 based on your suggestions.

- Felix