Re: [PATCH net v1] ptp: fix NULL deref when adjtime/adjfine are missing

[Xuanqiang Luo]

Hi Vadim

在 2026/8/26 20:24, Vadim Fedorenko 写道:
> On 26/08/2026 11:10, Xuanqiang Luo wrote:
> > From: Xuanqiang Luo <luoxuanqiang@xxxxxxxxxx>
> >
> > 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