linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Anna-Maria Behnsen <anna-maria@linutronix.de>
To: Phil Chang <phil.chang@mediatek.com>
Cc: alix.wu@mediatek.com, angelogioacchino.delregno@collabora.com,
	frederic@kernel.org, jy.ho@mediatek.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, matthias.bgg@gmail.com,
	phil.chang@mediatek.com, tglx@linutronix.de
Subject: Re: [PATCH] hrtimer: check hrtimer with a NULL function
Date: Thu, 06 Jun 2024 15:14:37 +0200	[thread overview]
Message-ID: <877cf2i56q.fsf@somnus> (raw)
In-Reply-To: <20240605135106.16065-1-phil.chang@mediatek.com>

Phil Chang <phil.chang@mediatek.com> writes:

>>> simillar with timers, check for timer->function == NULL.
>>> If the pointer is NULL, discard the request silently.
>
>> Can you please explain, why this change is required?
>
>> The statement "similar to timers" is not a valid explaination as timer
>> list timers and hrtimers are two different things. The function pointer
>> for timer list timers is explicitly set to NULL in shutdown path to
>> prevent unwanted rearming of the timer. For hrtimers there is no
>> shutdown function implemented and function is never set to NULL by
>> hrtimer code.
>>
> The timer->function is provided  by caller, which is invaild if fuction is NULL, 
> and currently, the hrtime code does not perform any checks to validate this. 
> Passing a NULL function can lead to a system panic, with a backtrace likes:
> ```
>    __hrtimer_run_queues+0x1d8/0x3b8
>    hrtimer_interrupt+0xdc/0x3a0
>    arch_timer_handler_phys+0x54/0x94
>    handle_percpu_devid_irq+0xb8/0x308
>    handle_domain_irq+0x78/0xec
>    gic_handle_irq+0x50/0x10c
>    call_on_irq_stack+0x38/0x54
>    do_interrupt_handler+0x40/0x98
> ```
> This backtrace does not clearly indicate the source of the invalid usage of hrtimer.

To make it more clear to the inexperienced hrtimer user that it is
mandatory to initialize timer->function to be able to use hrtimers, but
to prevent the kernel to crash, please add a warning in
hrtimer_start_range_ns(). The check can be done without locking the
hrtimer base. When the function pointer is not set, return directly
without doing anything. So this above mentioned backtrace will no longer
appear, as the timer is not added to a queue.

Thanks,

	Anna-Maria

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-06-06 13:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-05  8:41 [PATCH] hrtimer: check hrtimer with a NULL function Phil Chang
2024-06-05 10:15 ` Anna-Maria Behnsen
2024-06-05 13:51   ` Phil Chang
2024-06-06 13:14     ` Anna-Maria Behnsen [this message]
2024-06-07  2:26       ` [PATCH v2] " Phil Chang
2024-06-10  9:47         ` Anna-Maria Behnsen
2024-06-10 13:31           ` [PATCH v3] " Phil Chang
2024-06-17 16:19             ` Anna-Maria Behnsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877cf2i56q.fsf@somnus \
    --to=anna-maria@linutronix.de \
    --cc=alix.wu@mediatek.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=frederic@kernel.org \
    --cc=jy.ho@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=phil.chang@mediatek.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).