netdev - Re: [PATCH net v3 3/3] bnxt_en: handle tpa

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: <6243b59f-dc40-4d97-bf16-77ae276e4a06@huawei.com>
Date: Tue, 31 Dec 2024 19:49:59 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: Michael Chan <michael.chan@...adcom.com>, David Wei <dw@...idwei.uk>,
	<netdev@...r.kernel.org>, Andy Gospodarek <andrew.gospodarek@...adcom.com>,
	Somnath Kotur <somnath.kotur@...adcom.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net v3 3/3] bnxt_en: handle tpa_info in queue API
 implementation

On 2024/12/13 20:17, Yunsheng Lin wrote:
> On 2024/12/12 22:56, Jakub Kicinski wrote:
>> On Thu, 12 Dec 2024 19:23:52 +0800 Yunsheng Lin wrote:
>>> It seems an extra RCU sync is not really needed if page_pool_destroy()
>>> for the old page_pool is called between napi_disable() and napi_enable()
>>> as page_pool_destroy() already have a RCU sync.
>>
>> I did my best.
>>
> 
> I am not sure how to interpret the above comment.
> 
> Anyway, I guess it can be said the patch in [1] is only trying to fix
> a use-after-freed problem basing on the assumption that the softirq
> context on the same CPU has ensured sequential execution and a specific
> NAPI instance executed between different CPUs has also ensured sequential
> execution, so page_pool_napi_local() will only return true for a softirq
> context specific to the CPU being pool->p.napi->list_owner when list_owner
> != -1 after napi_enable() is called, and page_pool_napi_local() will always
> return false after napi_disable() is called as pool->p.napi->list_owner is
> always -1 even when skb_defer_free_flush() can be called without binding to
> any NAPI instance and can be executed in the softirq context of any CPU.
> 
> If there is any timing window you think that might cause page_pool_napi_local()
> to return true unexpectly, it would be good to be more specific about it and
> a bigger rcu lock might be needed instead of a small rcu lock in [1].

Please let me know if there is other obvious timing window that need fixing.
If not, I am planning to keep that patch as part of the dma unmapping patchset
as the dma unmapping fix patch depends on synchronize_rcu() added in that patch
and the time window is so small that it doesn't seem to be an urgent fix, so
target the net-next as the dma unmapping fix does.

> 
> 1. https://lore.kernel.org/all/20241120103456.396577-2-linyunsheng@huawei.com/#r
>