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] [thread-next>] [day] [month] [year] [list]

Message-ID: <6b876bf8-ad97-49c9-867d-f16f122bd514@huawei.com>
Date: Fri, 13 Dec 2024 20:17:06 +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/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].

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