linux-kernel - Re: [PATCH net-next v2] ptp: remove unnecessary mutex lock in ptp_clock

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: <20250703133450.kaoncopz3i2ileug@skbuf>
Date: Thu, 3 Jul 2025 16:34:50 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Jeongjun Park <aha310510@...il.com>
Cc: richardcochran@...il.com, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2] ptp: remove unnecessary mutex lock in
 ptp_clock_unregister()

On Thu, Jul 03, 2025 at 02:53:40PM +0900, Jeongjun Park wrote:
> ptp_clock_unregister() is called by ptp core and several drivers that
> require ptp clock feature. And in this function, ptp_vclock_in_use()
> is called to check if ptp virtual clock is in use, and
> ptp->is_virtual_clock, ptp->n_vclocks are checked.
> 
> It is true that you should always check ptp->is_virtual_clock to see if
> you are using ptp virtual clock, but you do not necessarily need to
> check ptp->n_vclocks.
> 
> ptp->n_vclocks is a feature need by ptp sysfs or some ptp cores, so in
> most cases, except for these callers, it is not necessary to check.
> 
> The problem is that ptp_clock_unregister() checks ptp->n_vclocks even
> when called by a driver other than the ptp core, and acquires
> ptp->n_vclocks_mux to avoid concurrency issues when checking.
> 
> I think this logic is inefficient,

Can you present a single metric by which you qualify this logic as
inefficient?

> so I think it would be appropriate to modify the caller function that
> must check ptp->n_vclocks to check ptp->n_vclocks in advance before
> calling ptp_clock_unregister().

And which are the call sites of ptp_clock_unregister() which should
check ptp->n_vclocks in advance, in your proposal?