linux-kernel - Re: [PATCH iwlwifi-fixes] wifi: iwlwifi: Implement settime64 as stub for MVM/MLD PTP

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: <1218241a.4a90.19b203d3fd8.Coremail.00107082@163.com>
Date: Mon, 15 Dec 2025 12:20:43 +0800 (CST)
From: "David Wang" <00107082@....com>
To: "Yao Zi" <me@...ao.cc>
Cc: thostet@...gle.com, daniel.gabay@...el.com, jeffbai@...c.io,
	johannes.berg@...el.com, kexybiscuit@...c.io,
	linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org,
	miriam.rachel.korenblit@...el.com, nathan@...nel.org,
	netdev@...r.kernel.org, pagadala.yesu.anjaneyulu@...el.com,
	richardcochran@...il.com, "Kuniyuki Iwashima" <kuniyu@...gle.com>
Subject: Re: [PATCH iwlwifi-fixes] wifi: iwlwifi: Implement settime64 as
 stub for MVM/MLD PTP


At 2025-12-15 11:42:17, "Yao Zi" <me@...ao.cc> wrote:
>On Sun, Dec 14, 2025 at 06:12:57PM +0800, David Wang wrote:
>> On Thu, Dec 04, 2025 at 12:32:04PM +0000, Yao Zi wrote:
>> > Since commit dfb073d32cac ("ptp: Return -EINVAL on ptp_clock_register if
>> > required ops are NULL"), PTP clock registered through ptp_clock_register
>> > is required to have ptp_clock_info.settime64 set, however, neither MVM
>> > nor MLD's PTP clock implementation sets it, resulting in warnings when
>> > the interface starts up, like
>> > 
>> > WARNING: drivers/ptp/ptp_clock.c:325 at ptp_clock_register+0x2c8/0x6b8, CPU#1: wpa_supplicant/469
>> > CPU: 1 UID: 0 PID: 469 Comm: wpa_supplicant Not tainted 6.18.0+ #101 PREEMPT(full)
>> > ra: ffff800002732cd4 iwl_mvm_ptp_init+0x114/0x188 [iwlmvm]
>> > ERA: 9000000002fdc468 ptp_clock_register+0x2c8/0x6b8
>> > iwlwifi 0000:01:00.0: Failed to register PHC clock (-22)
>> > 
>> > I don't find an appropriate firmware interface to implement settime64()
>> > for iwlwifi MLD/MVM, thus instead create a stub that returns
>> > -EOPTNOTSUPP only, suppressing the warning and allowing the PTP clock to
>> > be registered.
>> 
>> This seems disturbing....If a null settime64 deserve a kernel WARN dump, so should
>> a settime64 which returns error.
>
>They're separate things. A ptp clock implementing not provinding
>settime64() or gettime64()/gettimex64() callback will crash when
>userspace tries to call clock_gettime()/clock_settime() on it, since
>either ptp_clock_settime() or ptp_clock_gettime() invokes these
>callbacks unconditionally.
>
>However, failing with -ENOTSUPP/-EOPNOTSUPP when clock_settime() isn't
>supported by a dynamic POSIX clock device is a documented behavior, see
>man-page clock_getres(2).
>
>> Before fixing the warning, the expected behavior of settime64 should be specified clearly,
>
>I think failing with -EOPNOTSUPP (which is the same as -ENOTSUPP on
>Linux) when the operation isn't supported is well-documented, and is
>suitable for this case.
>
>One may argue that it'd be helpful for ptp_clock_register() to provide
>a default implementation of settime64() that always fails with
>-EOPNOTSUPP when the driver doesn't provide one.
>
>However, it's likely a programming bug when gettime64()/settime64() is
>missing, so the current behavior of warning sounds reasonable to me.
>
>> hence why the dfb073d32cac ("ptp: Return -EINVAL on ptp_clock_register if required ops are NULL")?
>
>You may be interested in the original series[1] where the idea of
>warning for missing settime64/gettime64/gettimex64 callbacks came up.

Thanks for the information, this link holds way more relevant information than the *Link* tag in dfb073d32cac.  :)

But isn't  the patch in [1]  and similar change to settime64 better? 
What is the  difference between a null callback and a callback returning error? aren't they saying the same
thing: "this device does not support it"?



David

>
>Also cc Kuniyuki, in case that I missed something or got it wrong.
>
>> 
>> David
>
>Best regards,
>Yao Zi
>
>> > 
>> > Reported-by: Nathan Chancellor <nathan@...nel.org>
>> > Closes: https://lore.kernel.org/all/20251108044822.GA3262936@ax162/
>> > Signed-off-by: Yao Zi <ziyao@...root.org>
>> > ---
>> 
>
>[1]: https://lore.kernel.org/all/20251028095143.396385-1-junjie.cao@intel.com/