[LTP] [PATCH v2 1/2] lib: tst_rtctime: close RTC fd on the ioctl() error path

[LTP] [PATCH v2 1/2] lib: tst_rtctime: close RTC fd on the ioctl() error path

[Kuba Pawlak via ltp]

tst_rtc_ioctl() opens the RTC device with SAFE_OPEN() but returns -1 on
an ioctl() failure without closing the descriptor, leaking it on every
failed call. The RTC character device is exclusive-open, so a leaked fd
makes the next tst_rtc_ioctl() open fail with EBUSY.

This was mostly latent because the failure path was rarely taken, but it
is now hit routinely on RTCs that reject RTC_SET_TIME (e.g. Qualcomm PMIC
RTCs returning -ENODEV). Close the fd on the error path and preserve
errno so callers can still inspect the ioctl() failure reason.

Signed-off-by: Kuba Pawlak <kuba.pawlak@canonical.com>
---
 lib/tst_rtctime.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/tst_rtctime.c b/lib/tst_rtctime.c
index c62ac73..cc0cb4e 100644
--- a/lib/tst_rtctime.c
+++ b/lib/tst_rtctime.c
@@ -7,6 +7,7 @@
 
 #include <stdbool.h>
 #include <limits.h>
+#include <errno.h>
 #define TST_NO_DEFAULT_MAIN
 #include "tst_test.h"
 #include "tst_rtctime.h"
@@ -117,11 +118,15 @@ int tst_rtc_ioctl(const char *rtc_dev, unsigned long request,
 
 	ret = ioctl(rtc_fd, request, rtc_tm);
 
-	if (ret != 0)
-		return -1;
+	if (ret != 0) {
+		int saved_errno = errno;
 
-	if (rtc_fd > 0)
 		SAFE_CLOSE(rtc_fd);
+		errno = saved_errno;
+		return -1;
+	}
+
+	SAFE_CLOSE(rtc_fd);
 
 	return 0;
 }
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/2] rtc02: skip (TCONF) on read-only RTCs that reject RTC_SET_TIME

[Kuba Pawlak via ltp]

Some RTC hardware does not allow setting the time from the host. For
example Qualcomm PMIC RTCs (rtc-pm8xxx) that are configured without the
allow-set-time DT property and without an nvmem/UEFI offset return
-ENODEV from RTC_SET_TIME; the counter free-runs from the epoch and is
only usable as an uptime source. On such devices rtc02 currently reports
TFAIL, which is a hardware limitation rather than a kernel defect.

Probe whether the RTC can be set in the setup phase, writing back the
value just read so a writable RTC is left unchanged. When RTC_SET_TIME
fails with ENODEV, skip the whole test with TCONF. Only ENODEV is treated
as "not supported": wider errnos such as EINVAL are used broadly by the
kernel and could mask a genuine failure, so anything else still results
in TBROK.

Doing the check in setup, before tst_rtc_clock_save(), also avoids the
cleanup path (tst_rtc_clock_restore()) issuing RTC_SET_TIME and turning
the run into a TBROK on such devices. If the probe succeeds the test body
runs as before, so no additional handling is needed there.

Signed-off-by: Kuba Pawlak <kuba.pawlak@canonical.com>
---
 testcases/kernel/device-drivers/rtc/rtc02.c | 22 +++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/testcases/kernel/device-drivers/rtc/rtc02.c b/testcases/kernel/device-drivers/rtc/rtc02.c
index 34509ab..96e1084 100644
--- a/testcases/kernel/device-drivers/rtc/rtc02.c
+++ b/testcases/kernel/device-drivers/rtc/rtc02.c
@@ -16,6 +16,7 @@
  */
 
 #include <stdio.h>
+#include <errno.h>
 #include "tst_rtctime.h"
 #include "tst_wallclock.h"
 #include "tst_test.h"
@@ -133,10 +134,31 @@ static void set_rtc_test(void)
 static void rtc_setup(void)
 {
 	int exists = access(rtc_dev, F_OK);
+	struct rtc_time probe_tm;
 
 	if (exists < 0)
 		tst_brk(TCONF, "RTC device %s not available", rtc_dev);
 
+	/*
+	 * Skip on RTCs that do not allow setting the time, e.g. Qualcomm PMIC
+	 * RTCs without allow-set-time or an nvmem offset, where RTC_SET_TIME
+	 * returns -ENODEV. Probe with the current time so a writable RTC is
+	 * left unchanged, and skip here (before tst_rtc_clock_save()) so the
+	 * cleanup restore does not fail on such devices.
+	 *
+	 * Only ENODEV is treated as "not supported" here. Wider errnos such as
+	 * EINVAL are used broadly by the kernel and could mask a genuine
+	 * failure, so let anything else fall through to TBROK.
+	 */
+	if (tst_rtc_gettime(rtc_dev, &probe_tm))
+		tst_brk(TBROK | TERRNO, "ioctl() RTC_RD_TIME");
+	if (tst_rtc_settime(rtc_dev, &probe_tm)) {
+		if (errno == ENODEV)
+			tst_brk(TCONF | TERRNO,
+				"RTC does not support setting the time");
+		tst_brk(TBROK | TERRNO, "ioctl() RTC_SET_TIME");
+	}
+
 	tst_rtc_clock_save(rtc_dev);
 }
 
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/2] rtc02: skip (TCONF) on read-only RTCs that reject RTC_SET_TIME

[Andrea Cervesato via ltp]

Hi Kuba,

> +	/*
> +	 * Skip on RTCs that do not allow setting the time, e.g. Qualcomm PMIC
> +	 * RTCs without allow-set-time or an nvmem offset, where RTC_SET_TIME
> +	 * returns -ENODEV. Probe with the current time so a writable RTC is
> +	 * left unchanged, and skip here (before tst_rtc_clock_save()) so the
> +	 * cleanup restore does not fail on such devices.
> +	 *
> +	 * Only ENODEV is treated as "not supported" here. Wider errnos such as
> +	 * EINVAL are used broadly by the kernel and could mask a genuine
> +	 * failure, so let anything else fall through to TBROK.
> +	 */

Please don't add more info than what we already need. The git commit
already explains why we have this change and the change talks by itself:
we need to verify if RTC is enabled or not.

> +	if (tst_rtc_gettime(rtc_dev, &probe_tm))
> +		tst_brk(TBROK | TERRNO, "ioctl() RTC_RD_TIME");

Space here.

> +	if (tst_rtc_settime(rtc_dev, &probe_tm)) {
> +		if (errno == ENODEV)
> +			tst_brk(TCONF | TERRNO,
> +				"RTC does not support setting the time");

Move it in one line.

> +		tst_brk(TBROK | TERRNO, "ioctl() RTC_SET_TIME");
> +	}
> +
>  	tst_rtc_clock_save(rtc_dev);
>  }

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] lib: tst_rtctime: close RTC fd on the ioctl() error path

[linuxtestproject.agent]

Hi Kuba,

On Fri, 17 Jul 2026 12:19:59 +0200, Kuba Pawlak wrote:
> lib: tst_rtctime: close RTC fd on the ioctl() error path

Verdict - Reviewed

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/2] rtc02: handle RTCs that do not allow setting the time

[Kuba Pawlak via ltp]

Note: I forgot to include a cover letter when sending this v2 series.
Adding the intended cover-letter text here for context.

This series makes LTP's rtc02 cope with RTC hardware that does not allow
setting the time (e.g. Qualcomm PMIC RTCs, rtc-pm8xxx, which return -ENODEV
from RTC_SET_TIME when configured without allow-set-time or an nvmem/UEFI
offset). On such devices rtc02 currently fails a valid, working system.

Patch 1 fixes a pre-existing fd leak in tst_rtc_ioctl() that the new probe
in patch 2 would otherwise hit on every run (the RTC chardev is
exclusive-open, so the leak causes a follow-up EBUSY). Patch 2 probes
set-time support in setup and skips with TCONF when RTC_SET_TIME returns
ENODEV.

Changes since v1:
  - Narrowed the skip condition from a broad errno set to ENODEV only, as
    EINVAL and others are used too widely by the kernel to treat as
    "not supported" (per review feedback).
  - Added patch 1 to fix the pre-existing fd leak on the ioctl() error
    path, which the probe now exercises routinely.

Tested on arm64:
  - QCS8300 (read-only RTC): rtc02 now TCONF (skipped), was TFAIL.
  - X1E80100 (writable RTC): rtc02 still TPASS.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] lib: tst_rtctime: close RTC fd on the ioctl() error path

[Andrea Cervesato via ltp]

Hi Kuba,

>  	ret = ioctl(rtc_fd, request, rtc_tm);
>  
> -	if (ret != 0)
> -		return -1;
> +	if (ret != 0) {
> +		int saved_errno = errno;
>  
> -	if (rtc_fd > 0)
>  		SAFE_CLOSE(rtc_fd);
> +		errno = saved_errno;
> +		return -1;
> +	}
> +
> +	SAFE_CLOSE(rtc_fd);
>  
>  	return 0;
>  }

Why do we need to store the errno? Simply call SAFE_CLOSE() before
returning -1. If SAFE_CLOSE() fails, test will break anyway..

--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp