All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ioannis Fountzoulas" <ioannis.fountzoulas@nokia.com>
Cc: "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	linux-rtc@vger.kernel.org
Subject: Re: [PATCH] rtc: armada38x: do not advertise update interrupt (UIE) support
Date: Sat, 04 Jul 2026 21:23:38 +0000	[thread overview]
Message-ID: <20260704212338.C18CD1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260704210710.658574-1-ioannis.fountzoulas@nokia.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Clearing RTC_FEATURE_UPDATE_INTERRUPT degrades UIE performance to software polling and introduces a silent dependency on CONFIG_RTC_INTF_DEV_UIE_EMUL, risking ABI breakage.

Pre-existing issues:
- [High] The patch implements a driver-specific workaround for a core RTC subsystem vulnerability, masking the issue in armada38x while leaving all other native UIE-capable RTC drivers vulnerable to a DoS (soft lockup).
- [High] Unprotected hardware register writes in armada38x_rtc_set_offset and armada38x_rtc_reset break hardware write protocol when preempted by an alarm interrupt, potentially causing silent RTC write failures.
--

commit e4e1f958746b053e26726c0a8b1203537d9bd06e
Author: Ioannis Fountzoulas <ioannis.fountzoulas@nokia.com>

rtc: armada38x: do not advertise update interrupt (UIE) support

chrony enables RTC update interrupts via the RTC_UIE_ON ioctl to track
RTC drift. On the armada38x driver this request is served by the RTC
core's native path, which arms a 1 second periodic timer that is
re-programmed on the alarm and serviced by rtc_timer_do_work().

> diff --git a/drivers/rtc/rtc-armada38x.c b/drivers/rtc/rtc-armada38x.c
> index 245290ae1a8de..da036d819649c 100644
> --- a/drivers/rtc/rtc-armada38x.c
> +++ b/drivers/rtc/rtc-armada38x.c
> @@ -526,6 +526,14 @@ static __init int armada38x_rtc_probe(struct platform_device *pdev)
>  	else
>  		clear_bit(RTC_FEATURE_ALARM, rtc->rtc_dev->features);
>  
> +	/*
> +	 * A large forward step of the RTC time makes
> +	 * rtc_timer_do_work() replay one period per elapsed second and can
> +	 * loop long enough to trigger a soft lockup. Do not advertise
> +	 * native UIE; RTC_UIE_ON then uses the poll-based emulation.
> +	 */
> +	clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->rtc_dev->features);

[Severity: High]
This is a pre-existing issue, but does this driver-specific workaround
mask a core RTC subsystem vulnerability, leaving all other native UIE-capable
RTC drivers vulnerable to a soft lockup?

If a userspace process with CAP_SYS_TIME initiates a large forward time jump,
a lockless TOCTOU race can occur in drivers/rtc/interface.c:rtc_set_time().

rtc_set_time() checks rtc->uie_rtctimer.enabled, which can read as 0 if
rtc_timer_do_work() is concurrently processing the timer.

rtc_set_time() then updates the hardware time without re-arming the timer
to the new time.

The subsequent run of rtc_timer_do_work() finds the timer expiry far in the
past and enters an unbounded while-loop, advancing the timer by 1 second per
iteration, pinning the CPU.

Could we fix this race in the core RTC subsystem rather than disabling native
UIE in this driver?

[Severity: Medium]
Will clearing this feature bit introduce a regression by degrading UIE
performance to software polling and adding a silent dependency on
CONFIG_RTC_INTF_DEV_UIE_EMUL?

If the kernel lacks CONFIG_RTC_INTF_DEV_UIE_EMUL, RTC_UIE_ON will fail and
return -EINVAL, potentially breaking userspace tools like chrony.

If it is enabled, it uses CPU-intensive polling instead of hardware interrupts.

[Severity: High]
This isn't a bug introduced by this patch, but do unprotected hardware register
writes in armada38x_rtc_set_offset() and armada38x_rtc_reset() break the
hardware write protocol if preempted by an alarm interrupt?

Both functions call rtc_delayed_write() without holding rtc->lock or disabling
interrupts.

If an RTC alarm interrupt fires during this time, the interrupt handler
interleaves its own rtc_delayed_write().

When the interrupt returns, the interrupted write executes without the required
preceding dummy writes to RTC_STATUS (hardware erratum workaround RES-3124064),
potentially causing silent RTC write failures.

> +
>  	/* Update RTC-MBUS bridge timing parameters */
>  	rtc->data->update_mbus_timing(rtc);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704210710.658574-1-ioannis.fountzoulas@nokia.com?part=1

  reply	other threads:[~2026-07-04 21:23 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-04 21:07 [PATCH] rtc: armada38x: do not advertise update interrupt (UIE) support Ioannis Fountzoulas
2026-07-04 21:23 ` sashiko-bot [this message]
2026-07-04 22:13 ` Alexandre Belloni

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=20260704212338.C18CD1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=ioannis.fountzoulas@nokia.com \
    --cc=linux-rtc@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.