[PATCH] rtc: interface: Alarm race handling should not discard preceding error

[PATCH] rtc: interface: Alarm race handling should not discard preceding error

[Anthony Pighin (Nokia)]

Commit 795cda8338ea ("rtc: interface: Fix long-standing race when setting
alarm") should not discard any errors from the preceding validations.

Prior to that commit, if the alarm feature was disabled, or the
set_alarm failed, a meaningful error code would be returned to the
caller for further action.

After, more often than not, the __rtc_read_time will cause a success
return code instead, misleading the caller.

An example of this is when timer_enqueue is called for a rtc-abx080x
device. Since that driver does not clear the alarm feature bit, but
instead relies on the set_alarm operation to return invalid, the discard
of the return code causes very different behaviour; i.e.
    hwclock: select() to /dev/rtc0 to wait for clock tick timed out

Fixes: 795cda8338ea ("rtc: interface: Fix long-standing race when setting alarm")
Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
---
 drivers/rtc/interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index b8b298efd9a9..1906f4884a83 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -457,7 +457,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
         * are in, we can return -ETIME to signal that the timer has already
         * expired, which is true in both cases.
         */
-       if ((scheduled - now) <= 1) {
+       if (!err && (scheduled - now) <= 1) {
                err = __rtc_read_time(rtc, &tm);
                if (err)
                        return err;
--
2.43.0

Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error

[Thorsten Leemhuis]

Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
regression for him caused by the commit from Esben (now also CCed) the
Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
details:
https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
and

Nick, could you maybe provide a tested-by tag here? Maybe that would
motivate someone to get this en route to mainline.

Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
encountered this on earlier series, as it was backported all the way to
5.15.y

Ciao, Thorsten

On 11/25/25 18:35, Anthony Pighin (Nokia) wrote:
> Commit 795cda8338ea ("rtc: interface: Fix long-standing race when setting
> alarm") should not discard any errors from the preceding validations.
> 
> Prior to that commit, if the alarm feature was disabled, or the
> set_alarm failed, a meaningful error code would be returned to the
> caller for further action.
> 
> After, more often than not, the __rtc_read_time will cause a success
> return code instead, misleading the caller.
> 
> An example of this is when timer_enqueue is called for a rtc-abx080x
> device. Since that driver does not clear the alarm feature bit, but
> instead relies on the set_alarm operation to return invalid, the discard
> of the return code causes very different behaviour; i.e.
>     hwclock: select() to /dev/rtc0 to wait for clock tick timed out
> 
> Fixes: 795cda8338ea ("rtc: interface: Fix long-standing race when setting alarm")
> Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
> ---
>  drivers/rtc/interface.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index b8b298efd9a9..1906f4884a83 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -457,7 +457,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
>          * are in, we can return -ETIME to signal that the timer has already
>          * expired, which is true in both cases.
>          */
> -       if ((scheduled - now) <= 1) {
> +       if (!err && (scheduled - now) <= 1) {
>                 err = __rtc_read_time(rtc, &tm);
>                 if (err)
>                         return err;
> --
> 2.43.0

Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error

[Nick Bowler]

On Tue, Dec 16, 2025 at 11:51:30AM +0100, Thorsten Leemhuis wrote:
> Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
> regression for him caused by the commit from Esben (now also CCed) the
> Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
> details:
> https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
> and
> 
> Nick, could you maybe provide a tested-by tag here? Maybe that would
> motivate someone to get this en route to mainline.
> 
> Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
> encountered this on earlier series, as it was backported all the way to
> 5.15.y

It was backported to 5.10.y and 5.4.y too, but only after I had reported
this regression back in October (and I guess 5.4.y is EOL now).

Tested-by: Nick Bowler <nbowler@draconx.ca>

Thanks,
  Nick

Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error

[Thorsten Leemhuis]

On 12/16/25 16:09, Nick Bowler wrote:
> On Tue, Dec 16, 2025 at 11:51:30AM +0100, Thorsten Leemhuis wrote:
>> Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
>> regression for him caused by the commit from Esben (now also CCed) the
>> Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
>> details:
>> https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
>> and
>>
>> Nick, could you maybe provide a tested-by tag here? Maybe that would
>> motivate someone to get this en route to mainline.
>>
>> Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
>> encountered this on earlier series, as it was backported all the way to
>> 5.15.y
> 
> It was backported to 5.10.y and 5.4.y too, but only after I had reported
> this regression back in October (and I guess 5.4.y is EOL now).
> 
> Tested-by: Nick Bowler <nbowler@draconx.ca>

Esben (author of the culprit) and Alexandre (who merged it): do you
still have reviewing/applying the patch at the start of this thread (
https://lore.kernel.org/all/BN0PR08MB6951415A751F236375A2945683D1A@BN0PR08MB6951.namprd08.prod.outlook.com/#t)
on your todo list?

Sorry for nagging, would just be good to finally get this 6.18-rc1
regression fixed in mainline so the fix can be backported to stable,
too. And due to the holidays I thought a quick reminder might be wise.

Or was this fixed somehow and I just missed it?

Ciao, Thorsten

#regzbot ignore-activity

Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error

[Esben Haabendal]

"Thorsten Leemhuis" <regressions@leemhuis.info> writes:

> On 12/16/25 16:09, Nick Bowler wrote:
>> On Tue, Dec 16, 2025 at 11:51:30AM +0100, Thorsten Leemhuis wrote:
>>> Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
>>> regression for him caused by the commit from Esben (now also CCed) the
>>> Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
>>> details:
>>> https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
>>> and
>>>
>>> Nick, could you maybe provide a tested-by tag here? Maybe that would
>>> motivate someone to get this en route to mainline.
>>>
>>> Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
>>> encountered this on earlier series, as it was backported all the way to
>>> 5.15.y
>>
>> It was backported to 5.10.y and 5.4.y too, but only after I had reported
>> this regression back in October (and I guess 5.4.y is EOL now).
>>
>> Tested-by: Nick Bowler <nbowler@draconx.ca>
>
> Esben (author of the culprit) and Alexandre (who merged it): do you
> still have reviewing/applying the patch at the start of this thread (
> https://lore.kernel.org/all/BN0PR08MB6951415A751F236375A2945683D1A@BN0PR08MB6951.namprd08.prod.outlook.com/#t)
> on your todo list?
>
> Sorry for nagging, would just be good to finally get this 6.18-rc1
> regression fixed in mainline so the fix can be backported to stable,
> too. And due to the holidays I thought a quick reminder might be wise.
>
> Or was this fixed somehow and I just missed it?

Sorry for introducing this error. The fix looks good to me. I have added
my Reviewed-by to it.

Am I correct in that it also fixes the problem reported by Nick Bowler?

/Esben

Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error

[Thorsten Leemhuis]

On 1/9/26 13:54, Esben Haabendal wrote:
> "Thorsten Leemhuis" <regressions@leemhuis.info> writes:
> 
>> On 12/16/25 16:09, Nick Bowler wrote:
>>> On Tue, Dec 16, 2025 at 11:51:30AM +0100, Thorsten Leemhuis wrote:
>>>> Lo! FWIW, Nick Bowler (now CCed) reported that below patch fixes a
>>>> regression for him caused by the commit from Esben (now also CCed) the
>>>> Fixes: tag mentions. See "hwclock busted w/ M48T59 RTC (regression)" for
>>>> details:
>>>> https://lore.kernel.org/all/2t6bhs4udbu55ctbemkhlluchz2exrwown7kmu2gss6zukaxdm@ughygemahmem/
>>>> and
>>>>
>>>> Nick, could you maybe provide a tested-by tag here? Maybe that would
>>>> motivate someone to get this en route to mainline.
>>>>
>>>> Adding a "Cc: <stable@vger.kernel.org>" would be great, too, as Nick
>>>> encountered this on earlier series, as it was backported all the way to
>>>> 5.15.y
>>>
>>> It was backported to 5.10.y and 5.4.y too, but only after I had reported
>>> this regression back in October (and I guess 5.4.y is EOL now).
>>>
>>> Tested-by: Nick Bowler <nbowler@draconx.ca>
>>
>> Esben (author of the culprit) and Alexandre (who merged it): do you
>> still have reviewing/applying the patch at the start of this thread (
>> https://lore.kernel.org/all/BN0PR08MB6951415A751F236375A2945683D1A@BN0PR08MB6951.namprd08.prod.outlook.com/#t)
>> on your todo list?
>>
>> Sorry for nagging, would just be good to finally get this 6.18-rc1
>> regression fixed in mainline so the fix can be backported to stable,
>> too. And due to the holidays I thought a quick reminder might be wise.
>>
>> Or was this fixed somehow and I just missed it?
> 
> Sorry for introducing this error.

Happens, no worries.

> The fix looks good to me. I have added my Reviewed-by to it.

Thx!

> Am I correct in that it also fixes the problem reported by Nick Bowler?

Yes, see the "Tested-by" quoted above.

Ciao, Thorsten

Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error

[Esben Haabendal]

On Tuesday, November 25th, 2025 at 18:35, Anthony Pighin (Nokia) <anthony.pighin@nokia.com> wrote:

> Commit 795cda8338ea ("rtc: interface: Fix long-standing race when setting
> alarm") should not discard any errors from the preceding validations.
> 
> Prior to that commit, if the alarm feature was disabled, or the
> set_alarm failed, a meaningful error code would be returned to the
> caller for further action.
> 
> After, more often than not, the __rtc_read_time will cause a success
> return code instead, misleading the caller.
> 
> An example of this is when timer_enqueue is called for a rtc-abx080x
> device. Since that driver does not clear the alarm feature bit, but
> instead relies on the set_alarm operation to return invalid, the discard
> of the return code causes very different behaviour; i.e.
> hwclock: select() to /dev/rtc0 to wait for clock tick timed out
> 
> Fixes: 795cda8338ea ("rtc: interface: Fix long-standing race when setting alarm")
> Signed-off-by: Anthony Pighin anthony.pighin@nokia.com
> 
> ---
> drivers/rtc/interface.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index b8b298efd9a9..1906f4884a83 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -457,7 +457,7 @@ static int __rtc_set_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm)
> * are in, we can return -ETIME to signal that the timer has already
> * expired, which is true in both cases.
> */
> - if ((scheduled - now) <= 1) {
> + if (!err && (scheduled - now) <= 1) {
> err = __rtc_read_time(rtc, &tm);
> if (err)
> return err;
> --
> 2.43.0

Sorry about the slow response. And thanks for the fix :)
Reviewed-by: Esben Haabendal <esben@geanix.com>

Re: [PATCH] rtc: interface: Alarm race handling should not discard preceding error

[Alexandre Belloni]

On Tue, 25 Nov 2025 17:35:19 +0000, Anthony Pighin (Nokia) wrote:
> Commit 795cda8338ea ("rtc: interface: Fix long-standing race when setting
> alarm") should not discard any errors from the preceding validations.
> 
> Prior to that commit, if the alarm feature was disabled, or the
> set_alarm failed, a meaningful error code would be returned to the
> caller for further action.
> 
> [...]

Applied, after fixing the patch that has been mangled by outlook...

Also, you had this checkpatch error:
CHECK: From:/Signed-off-by: email comments mismatch: 'From: Anthony Pighin (Nokia) <anthony.pighin@nokia.com>' != 'Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>'


[1/1] rtc: interface: Alarm race handling should not discard preceding error
      https://git.kernel.org/abelloni/c/c6cf26c15ce7

Best regards,

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