linux-kernel - Re: [PATCH 1/2] selftest: rtc: Add to check rtc alarm status for alarm related test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c0db5bd6-8c6a-4017-911e-f3e01cd522ed@nvidia.com>
Date: Mon, 24 Jun 2024 01:43:43 +0000
From: Joseph Jang <jjang@...dia.com>
To: Alexandre Belloni <alexandre.belloni@...tlin.com>
CC: "shuah@...nel.org" <shuah@...nel.org>, "avagin@...gle.com"
	<avagin@...gle.com>, "amir73il@...il.com" <amir73il@...il.com>,
	"brauner@...nel.org" <brauner@...nel.org>, Matt Ochs <mochs@...dia.com>, Koba
 Ko <kobak@...dia.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-rtc@...r.kernel.org"
	<linux-rtc@...r.kernel.org>, "linux-kselftest@...r.kernel.org"
	<linux-kselftest@...r.kernel.org>, "linux-tegra@...r.kernel.org"
	<linux-tegra@...r.kernel.org>
Subject: Re: [PATCH 1/2] selftest: rtc: Add to check rtc alarm status for
 alarm related test



On 2024/6/21 3:36 AM, Alexandre Belloni wrote:
> On 23/05/2024 18:38:06-0700, Joseph Jang wrote:
>> In alarm_wkalm_set and alarm_wkalm_set_minute test, they use different
>> ioctl (RTC_ALM_SET/RTC_WKALM_SET) for alarm feature detection. They will
>> skip testing if RTC_ALM_SET/RTC_WKALM_SET ioctl returns an EINVAL error
>> code. This design may miss detecting real problems when the
>> efi.set_wakeup_time() return errors and then RTC_ALM_SET/RTC_WKALM_SET
>> ioctl returns an EINVAL error code with RTC_FEATURE_ALARM enabled.
>>
>> In order to make rtctest more explicit and robust, we propose to use
>> RTC_PARAM_GET ioctl interface to check rtc alarm feature state before
>> running alarm related tests. If the kernel does not support RTC_PARAM_GET
>> ioctl interface, we will fallback to check the error number of
>> (RTC_ALM_SET/RTC_WKALM_SET) ioctl call for alarm feature detection.
>>
>> Requires commit 101ca8d05913b ("rtc: efi: Enable SET/GET WAKEUP services
>> as optional")
>>
>> Reviewed-by: Koba Ko <kobak@...dia.com>
>> Reviewed-by: Matthew R. Ochs <mochs@...dia.com>
>> Signed-off-by: Joseph Jang <jjang@...dia.com>
>> ---
>>   tools/testing/selftests/rtc/Makefile  |  2 +-
>>   tools/testing/selftests/rtc/rtctest.c | 64 +++++++++++++++++++++++++++
>>   2 files changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/rtc/Makefile b/tools/testing/selftests/rtc/Makefile
>> index 55198ecc04db..6e3a98fb24ba 100644
>> --- a/tools/testing/selftests/rtc/Makefile
>> +++ b/tools/testing/selftests/rtc/Makefile
>> @@ -1,5 +1,5 @@
>>   # SPDX-License-Identifier: GPL-2.0
>> -CFLAGS += -O3 -Wl,-no-as-needed -Wall
>> +CFLAGS += -O3 -Wl,-no-as-needed -Wall -I../../../../usr/include/
> 
> Is this change actually needed?

If we didn't include "-I../../../../usr/include/" in rtctest Makefile,
we may encounter build errors like the following because rtctest default
look at the header file from /usr/include/linux/rtc.h which miss the
definition of struct rtc_param, RTC_PARAM_FEATURES and RTC_PARAM_GET.

rtctest.c: In function ‘get_rtc_alarm_state’:
rtctest.c:94:15: error: variable ‘param’ has initializer but incomplete
type
    94 |        struct rtc_param param = { 0 };
       |               ^~~~~~~~~
rtctest.c:94:35: warning: excess elements in struct initializer
    94 |        struct rtc_param param = { 0 };
       |                                   ^
rtctest.c:94:35: note: (near initialization for ‘param’)
rtctest.c:94:25: error: storage size of ‘param’ isn’t known
    94 |        struct rtc_param param = { 0 };
       |                         ^~~~~
rtctest.c:98:22: error: ‘RTC_PARAM_FEATURES’ undeclared (first use in
this function)
    98 |        param.param = RTC_PARAM_FEATURES;
       |                      ^~~~~~~~~~~~~~~~~~
rtctest.c:98:22: note: each undeclared identifier is reported only once
for each function it appears in
rtctest.c:100:23: error: ‘RTC_PARAM_GET’ undeclared (first use in this
function); did you mean ‘RTC_ALM_SET’?
   100 |        rc = ioctl(fd, RTC_PARAM_GET, &param);
       |                       ^~~~~~~~~~~~~
       |                       RTC_ALM_SET

After adding "-I../../../../usr/include/", the rtctest will look at 
linux kernel source header files from
<Linux root directory>/usr/include/linux/rtc.h to find the definition of
struct rtc_param, RTC_PARAM_FEATURES and RTC_PARAM_GET and fix the
rtctest build errors.


Thank you,
Joseph.