From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

ptp_clock_adjtime() invokes ->adjtime and ->adjfine unconditionally,
but both callbacks are optional. clock_adjtime() with ADJ_SETOFFSET
or ADJ_FREQUENCY therefore oopses on a PHC that implements neither.

iavf registers such a read-only clock: no adjtime/adjfine, and
max_adj left at 0. ADJ_SETOFFSET hits the NULL ->adjtime after the
offset is validated. ADJ_FREQUENCY with freq 0 converts to 0 ppb,
passes the max_adj check, and hits the NULL ->adjfine.

Return -EOPNOTSUPP when the requested callback is missing, as the
adjphase path already does, rather than adding driver stubs.

Fixes: d94ba80ebbea ("ptp: Added a brand new class driver for ptp clocks.")
Fixes: 75ab70ec5cef ("ptp: remove the .adjfreq interface function")
Cc: stable@vger.kernel.org
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 4111342d64f03..b5df120be4d1b 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -158,6 +158,8 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx)
 
 		kt = timespec64_to_ktime(ts);
 		delta = ktime_to_ns(kt);
+		if (!ops->adjtime)
+			return -EOPNOTSUPP;
 		err = ops->adjtime(ops, delta);
 	} else if (tx->modes & ADJ_FREQUENCY) {
 		long ppb;
@@ -174,6 +176,8 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct __kernel_timex *tx)
 		ppb = scaled_ppm_to_ppb(tx->freq);
 		if (ppb > ops->max_adj || ppb < -ops->max_adj)
 			return -ERANGE;
+		if (!ops->adjfine)
+			return -EOPNOTSUPP;
 		err = ops->adjfine(ops, tx->freq);
 		if (!err)
 			ptp->dialed_frequency = tx->freq;
-- 
2.43.0

On 26/08/2026 11:10, Xuanqiang Luo wrote:
> From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
> 
> ptp_clock_adjtime() invokes ->adjtime and ->adjfine unconditionally,
> but both callbacks are optional. 

Could you please link to the source of this statement?

clock_adjtime() with ADJ_SETOFFSET
> or ADJ_FREQUENCY therefore oopses on a PHC that implements neither.
> 
> iavf registers such a read-only clock: no adjtime/adjfine, and
> max_adj left at 0. ADJ_SETOFFSET hits the NULL ->adjtime after the
> offset is validated. ADJ_FREQUENCY with freq 0 converts to 0 ppb,
> passes the max_adj check, and hits the NULL ->adjfine.
> 
> Return -EOPNOTSUPP when the requested callback is missing, as the
> adjphase path already does, rather than adding driver stubs.

If PHC does not implement adjtime/adjfine/adjphase then it is a free-
running clock. ptp_clock_adjtime() has the check for such clocks.
To properly fix core part we have to extend ptp_clock and/or
ptp_clock_info to properly signal that registered clock is
free-running/read-only.

Hi Vadim

在 2026/8/26 20:24, Vadim Fedorenko 写道:
> On 26/08/2026 11:10, Xuanqiang Luo wrote:
>> From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
>>
>> ptp_clock_adjtime() invokes ->adjtime and ->adjfine unconditionally,
>> but both callbacks are optional. 
>
> Could you please link to the source of this statement?
>
Ah, I noticed that ops->adjphase is checked for NULL, while
ops->adjtime and ops->adjfine are not. I then found that the iavf
driver does not register either callback, making the NULL-dereference
paths reachable. This led me to believe that these callbacks were
optional.

However, the PTP hardware clock documentation explicitly states:
     Clock drivers must implement all of the functions in the interface. If
     a clock does not offer a particular ancillary feature, then the driver
     should just return -EOPNOTSUPP from those functions.

Link: https://docs.kernel.org/driver-api/ptp.html#writing-clock-drivers

Therefore, iavf does not comply with this requirement, and fixing the
issue in the iavf driver is more appropriate.

Thank you for pointing this out, Vadim.

> clock_adjtime() with ADJ_SETOFFSET
>> or ADJ_FREQUENCY therefore oopses on a PHC that implements neither.
>>
>> iavf registers such a read-only clock: no adjtime/adjfine, and
>> max_adj left at 0. ADJ_SETOFFSET hits the NULL ->adjtime after the
>> offset is validated. ADJ_FREQUENCY with freq 0 converts to 0 ppb,
>> passes the max_adj check, and hits the NULL ->adjfine.
>>
>> Return -EOPNOTSUPP when the requested callback is missing, as the
>> adjphase path already does, rather than adding driver stubs.
>
> If PHC does not implement adjtime/adjfine/adjphase then it is a free-
> running clock. ptp_clock_adjtime() has the check for such clocks.
> To properly fix core part we have to extend ptp_clock and/or
> ptp_clock_info to properly signal that registered clock is
> free-running/read-only.

I understand your suggestion. However, I plan to fix the iavf driver
by providing the missing callbacks and returning -EOPNOTSUPP, as
required by the PTP documentation.

An explicit free-running/read-only state would only cover PHCs with no
adjustment callbacks at all. It would not cover PHCs that implement only
a subset of the adjustment callbacks. Marking those clocks as
free-running would also incorrectly reject operations that they do
support.

Therefore, I think a driver-side fix is more appropriate for this case.
I will revise the patch to fix iavf accordingly.

Thank you very much for your comments!

BRs,
Xuanqiang

On Thu, Aug 27, 2026 at 02:43:31PM +0800, Xuanqiang Luo wrote:

> However, the PTP hardware clock documentation explicitly states:
>     Clock drivers must implement all of the functions in the interface. If
>     a clock does not offer a particular ancillary feature, then the driver
>     should just return -EOPNOTSUPP from those functions.
> 
> Link: https://docs.kernel.org/driver-api/ptp.html#writing-clock-drivers
> 
> Therefore, iavf does not comply with this requirement, and fixing the
> issue in the iavf driver is more appropriate.

+1

Thanks,
Richard