All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Alexandre Mergnat <amergnat@baylibre.com>
Cc: Eddie Huang <eddie.huang@mediatek.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-rtc@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 3/5] rtc: Fix the RTC time comparison issues adding cast
Date: Fri, 11 Apr 2025 15:38:40 +0200	[thread overview]
Message-ID: <202504111338408af44d7b@mail.local> (raw)
In-Reply-To: <20250109-enable-rtc-v3-3-f003e8144419@baylibre.com>

On 11/04/2025 14:35:56+0200, Alexandre Mergnat wrote:
> The RTC subsystem was experiencing comparison issues between signed and
> unsigned time values. When comparing time64_t variables (signed) with
> potentially unsigned range values, incorrect results could occur leading
> to runtime errors.
> 
> Adds explicit type casts to time64_t for critical RTC time comparisons
> in both class.c and interface.c files. The changes ensure proper
> handling of negative time values during range validation and offset
> calculations, particularly when dealing with timestamps before 1970.
> 
> The previous implementation might incorrectly interpret negative values
> as extremely large positive values, causing unexpected behavior in the
> RTC hardware abstraction logic.
> 

range_max is explicitly unsigned, casting it to a signed value will
break drivers.

> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
>  drivers/rtc/class.c     | 6 +++---
>  drivers/rtc/interface.c | 8 ++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index e31fa0ad127e9..1ee3f609f92ea 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c
> @@ -282,7 +282,7 @@ static void rtc_device_get_offset(struct rtc_device *rtc)
>  	 * then we can not expand the RTC range by adding or subtracting one
>  	 * offset.
>  	 */
> -	if (rtc->range_min == rtc->range_max)
> +	if (rtc->range_min == (time64_t)rtc->range_max)
>  		return;
>  
>  	ret = device_property_read_u32(rtc->dev.parent, "start-year",
> @@ -299,7 +299,7 @@ static void rtc_device_get_offset(struct rtc_device *rtc)
>  	if (!rtc->set_start_time)
>  		return;
>  
> -	range_secs = rtc->range_max - rtc->range_min + 1;
> +	range_secs = (time64_t)rtc->range_max - rtc->range_min + 1;
>  
>  	/*
>  	 * If the start_secs is larger than the maximum seconds (rtc->range_max)
> @@ -327,7 +327,7 @@ static void rtc_device_get_offset(struct rtc_device *rtc)
>  	 *
>  	 * Otherwise the offset seconds should be 0.
>  	 */
> -	if (rtc->start_secs > rtc->range_max ||
> +	if (rtc->start_secs > (time64_t)rtc->range_max ||
>  	    rtc->start_secs + range_secs - 1 < rtc->range_min)
>  		rtc->offset_secs = rtc->start_secs - rtc->range_min;
>  	else if (rtc->start_secs > rtc->range_min)
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index aaf76406cd7d7..93bdf06807f23 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -37,7 +37,7 @@ static void rtc_add_offset(struct rtc_device *rtc, struct rtc_time *tm)
>  	 */
>  	if ((rtc->start_secs > rtc->range_min && secs >= rtc->start_secs) ||
>  	    (rtc->start_secs < rtc->range_min &&
> -	     secs <= (rtc->start_secs + rtc->range_max - rtc->range_min)))
> +	     secs <= (time64_t)(rtc->start_secs + rtc->range_max - rtc->range_min)))
>  		return;
>  
>  	rtc_time64_to_tm(secs + rtc->offset_secs, tm);
> @@ -58,7 +58,7 @@ static void rtc_subtract_offset(struct rtc_device *rtc, struct rtc_time *tm)
>  	 * device. Otherwise we need to subtract the offset to make the time
>  	 * values are valid for RTC hardware device.
>  	 */
> -	if (secs >= rtc->range_min && secs <= rtc->range_max)
> +	if (secs >= rtc->range_min && secs <= (time64_t)rtc->range_max)
>  		return;
>  
>  	rtc_time64_to_tm(secs - rtc->offset_secs, tm);
> @@ -66,7 +66,7 @@ static void rtc_subtract_offset(struct rtc_device *rtc, struct rtc_time *tm)
>  
>  static int rtc_valid_range(struct rtc_device *rtc, struct rtc_time *tm)
>  {
> -	if (rtc->range_min != rtc->range_max) {
> +	if (rtc->range_min != (time64_t)rtc->range_max) {
>  		time64_t time = rtc_tm_to_time64(tm);
>  		time64_t range_min = rtc->set_start_time ? rtc->start_secs :
>  			rtc->range_min;
> @@ -74,7 +74,7 @@ static int rtc_valid_range(struct rtc_device *rtc, struct rtc_time *tm)
>  			(rtc->start_secs + rtc->range_max - rtc->range_min) :
>  			rtc->range_max;
>  
> -		if (time < range_min || time > range_max)
> +		if (time < range_min || time > (time64_t)range_max)
>  			return -ERANGE;
>  	}
>  
> 
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


  reply	other threads:[~2025-04-11 13:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-11 12:35 [PATCH v3 0/5] Enable RTC for the MT6357 Alexandre Mergnat
2025-04-11 12:35 ` [PATCH v3 1/5] rtc: mt6359: Add mt6357 support Alexandre Mergnat
2025-04-11 12:35 ` [PATCH v3 2/5] rtc: Add handling of pre-1970 dates in time conversion functions Alexandre Mergnat
2025-04-11 12:35 ` [PATCH v3 3/5] rtc: Fix the RTC time comparison issues adding cast Alexandre Mergnat
2025-04-11 13:38   ` Alexandre Belloni [this message]
2025-04-14 10:46     ` Alexandre Mergnat
2025-04-14 22:30   ` Uwe Kleine-König
2025-04-16 11:12     ` Geert Uytterhoeven
2025-04-11 12:35 ` [PATCH v3 4/5] rtc: mt6397: Remove start time parameters Alexandre Mergnat
2025-04-11 13:36   ` Alexandre Belloni
2025-04-11 13:39     ` Alexandre Belloni
2025-04-14 11:09       ` AngeloGioacchino Del Regno
2025-04-14 13:56         ` Alexandre Mergnat
2025-04-14 21:34           ` Uwe Kleine-König
2025-04-14 21:58             ` Alexandre Belloni
2025-04-11 12:35 ` [PATCH v3 5/5] arm64: dts: mediatek: Set RTC start year property Alexandre Mergnat

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=202504111338408af44d7b@mail.local \
    --to=alexandre.belloni@bootlin.com \
    --cc=amergnat@baylibre.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=eddie.huang@mediatek.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=robh@kernel.org \
    --cc=sean.wang@mediatek.com \
    /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.