linux-kernel - Re: [PATCH v2] ptp: remove ptp->n_vclocks check logic in ptp_vclock_in

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: <CAO9qdTEjQ5414un7Yw604paECF=6etVKSDSnYmZzZ6Pg3LurXw@mail.gmail.com>
Date: Wed, 4 Jun 2025 23:10:59 +0900
From: Jeongjun Park <aha310510@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: richardcochran@...il.com, andrew+netdev@...n.ch, davem@...emloft.net, 
	edumazet@...gle.com, pabeni@...hat.com, yangbo.lu@....com, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] ptp: remove ptp->n_vclocks check logic in ptp_vclock_in_use()

Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Mon, 26 May 2025 20:00:53 +0900 Jeongjun Park wrote:
> > If you need to check n_vclocks when checking
> > whether ptp virtual clock is in use, it means that caller function has
> > already performed work related to n_vclocks, and in this case, it is
> > appropriate to perform n_vclocks check and n_vclocks_mux lock in caller
> > function.
>
> Can you be a little less abstract in this explanation, given that
> ptp_vclock_in_use() only has a handful of callers?
> For ptp_clock_freerun() do you mean the ->has_cycles check?

There are two main cases where we need to check if ptp vclock is in use:

1. Check if ptp clock shall be free running (ptp_clock_freerun)
2. Check if ptp clock can be unregistered (ptp_clock_unregister)

In the first case, If I'm not misreading the code, ptp_clock_freerun()
determines whether it can run based on the result of ptp_vclock_in_use()
if ptp->has_cycles is false.

However, ptp_clock_{set,adj}time() , which calls ptp_clock_freerun(),
does not use n_vclocks . Therefore, it would be more appropriate to
remove unnecessary lock and n_vclocks checks when calling these
functions, both to prevent false positives in lockdep and to reduce
performance overhead.

And in the second case as well, there is no need to check n_vclocks in
every caller function. Because n_vclocks itself is a concept added for
physical/virtual clocks conversion in ptp physical clock sysfs, functions
other than sysfs actually do not need to check n_vclocks. Moreover,
since ptp_clock_unregister() is called by many functions, locking
n_vclocks_mux and checking n_vclocks when called by a function that
does not use n_vclocks causes performance overhead.

Therefore, after reviewing from various aspects, I think that the
unnecessary n_vclocks check and n_vclocks_mux lock in ptp_vclock_in_use()
should be removed to prevent false lockdep detection and performance
overhead, and it is more appropriate to change the n_vclocks check to be
performed first in the function that actually uses n_vclocks.