Re: [PATCH v2] rtc: efi: restore alarm support with runtime capability probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Zero-initializing `eft` in `efi_read_alarm()` guarantees an `-EIO` failure when reading disabled alarms, breaking user-space reads.
--

commit f9829a03fcbfff469b82ea3dd308ff1029bfea0e
Author: Johnny.Fan <Johnny.Fan@cixtech.com>
rtc: efi: restore alarm support with runtime capability probe

This commit restores the EFI RTC alarm functionality and procfs output that
was previously removed. It introduces a runtime probe of the wakeup
capability to avoid exposing a broken alarm if the service is unsupported.

> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index b4f44999ef0f4..5276870b410e0 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
[ ... ]
> @@ -112,6 +113,35 @@ convert_from_efi_time(efi_time_t *eft, struct rtc_time *wtime)
>  	return true;
>  }
>  
> +static int efi_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> +	efi_time_t eft;
> +	efi_status_t status;
> +
> +	memset(&eft, 0, sizeof(eft));
> +	status = efi.get_wakeup_time((efi_bool_t *)&wkalrm->enabled,
> +				     (efi_bool_t *)&wkalrm->pending, &eft);
> +	if (status != EFI_SUCCESS)
> +		return -EINVAL;
> +
> +	if (!convert_from_efi_time(&eft, &wkalrm->time))
> +		return -EIO;

[Severity: High]
Does this return -EIO for disabled alarms?

When a user queries the RTC alarm via the standard ioctl interface,
efi_read_alarm() zero-initializes eft before calling efi.get_wakeup_time().

Per the UEFI specification, if the alarm is disabled, the time output is
undefined and the firmware may leave eft unmodified.

Because eft is all zeroes, its day field is 0. The convert_from_efi_time()
function strictly validates the time, rejecting day == 0, and returns false:

drivers/rtc/rtc-efi.c:convert_from_efi_time() {
    ...
    if (!eft->day || eft->day > 31)
        return false;
    ...
}

Will this cause efi_read_alarm() to return -EIO to the subsystem, propagating
the error to userspace instead of correctly reporting the alarm as disabled,
and potentially breaking expected functionality for standard userspace tools
like rtcwake and hwclock?

> +
> +	return rtc_valid_tm(&wkalrm->time);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831113858.759210-1-johnny.fan@cixtech.com?part=1