[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b4d675a4-b7ad-4ecf-8d19-6bf08b452472@linux.dev>
Date: Wed, 29 Oct 2025 18:55:59 +0000
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Tim Hostetler <thostet@...gle.com>
Cc: Kuniyuki Iwashima <kuniyu@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
richardcochran@...il.com, andrew+netdev@...n.ch, davem@...emloft.net,
edumazet@...gle.com, junjie.cao@...el.com, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, pabeni@...hat.com,
syzbot+c8c0e7ccabd456541612@...kaller.appspotmail.com,
syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] ptp: guard ptp_clock_gettime() if neither gettimex64 nor
On 29/10/2025 16:37, Tim Hostetler wrote:
> On Tue, Oct 28, 2025 at 4:57 PM Vadim Fedorenko
> <vadim.fedorenko@...ux.dev> wrote:
>>
>> On 28.10.2025 23:54, Kuniyuki Iwashima wrote:
>>> On Tue, Oct 28, 2025 at 4:45 PM Vadim Fedorenko
>>> <vadim.fedorenko@...ux.dev> wrote:
>>>>
>>>> On 28.10.2025 23:13, Jakub Kicinski wrote:
>>>>> On Tue, 28 Oct 2025 15:51:50 +0000 Kuniyuki Iwashima wrote:
>>>>>> From: Richard Cochran <richardcochran@...il.com>
>>>>>> Date: Tue, 28 Oct 2025 07:09:41 -0700
>>>>>>> On Tue, Oct 28, 2025 at 05:51:43PM +0800, Junjie Cao wrote:
>>>>>>>> Syzbot reports a NULL function pointer call on arm64 when
>>>>>>>> ptp_clock_gettime() falls back to ->gettime64() and the driver provides
>>>>>>>> neither ->gettimex64() nor ->gettime64(). This leads to a crash in the
>>>>>>>> posix clock gettime path.
>>>>>>>
>>>>>>> Drivers must provide a gettime method.
>>>>>>>
>>>>>>> If they do not, then that is a bug in the driver.
>>>>>>
>>>>>> AFAICT, only GVE does not have gettime() and settime(), and
>>>>>> Tim (CCed) was preparing a fix and mostly ready to post it.
>>>>>
>>>>> cc: Vadim who promised me a PTP driver test :) Let's make sure we
>>>>> tickle gettime/setting in that test..
>>>>
>>>> Heh, call gettime/settime is easy. But in case of absence of these callbacks
>>>> the kernel will crash - not sure we can gather good signal in such case?
>>>
>>> At least we could catch it on NIPA.
>>>
>>> but I suggested Tim adding WARN_ON_ONCE(!info->gettime64 &&
>>> !info-> getimex64) in ptp_clock_register() so that a developer can
>>> notice that even while loading a buggy module.
>>
>> Yeah, that looks like a solution
>
> Yes, I was actually going to post the fix to gve today (I'll still do
> that as ptp_clock_gettime() is not the only function to assume a
> gettime64 or gettimex64 implementation) and shortly after posting
> Kuniyuki's suggested fix to ptp_clock_register() as such:
>
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index ef020599b771..f2d9cf4a455e 100644
> --- a/drivers/ptp/ptp_clock.c
> +++ b/drivers/ptp/ptp_clock.c
> @@ -325,6 +325,9 @@ struct ptp_clock *ptp_clock_register(struct
> ptp_clock_info *info,
> if (info->n_alarm > PTP_MAX_ALARMS)
> return ERR_PTR(-EINVAL);
>
> + if (WARN_ON_ONCE(!info->gettimex64 && !info->gettime64))
> + return ERR_PTR(-EINVAL);
> +
> /* Initialize a clock structure. */
> ptp = kzalloc(sizeof(struct ptp_clock), GFP_KERNEL);
> if (!ptp) {
> --
>
> I also have a similar patch for checking for settime64's function pointer.
>
> But I have no objections to Junjie posting a v2 in this manner instead
> of waiting for me.
WARN_ON_ONCE is better in terms of reducing the amount of review work.
Driver developers will be automatically notified about improper
implementation while Junjie's patch will simply hide the problem.
Powered by blists - more mailing lists