linux-kernel - Re: [PATCH 1/2] [RFC] rtc: expose direct access to hardware alarm time in debugfs

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]

Date:   Thu, 31 Mar 2022 21:43:03 +0200
From:   Mateusz Jończyk <mat.jonczyk@...pl>
To:     Greg KH <greg@...ah.com>
Cc:     linux-kernel@...r.kernel.org, linux-rtc@...r.kernel.org,
        linux-kselftest@...r.kernel.org,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH 1/2] [RFC] rtc: expose direct access to hardware alarm
 time in debugfs

W dniu 31.03.2022 o 21:21, Greg KH pisze:
> On Thu, Mar 31, 2022 at 09:06:11PM +0200, Mateusz Jończyk wrote:
>> Before Linux 5.17, there was a problem with the CMOS RTC driver:
>> cmos_read_alarm() and cmos_set_alarm() did not check for the UIP (Update
>> in progress) bit, which could have caused it to sometimes fail silently
>> and read bogus values or do not set the alarm correctly.
>> Luckily, this issue was masked by cmos_read_time() invocations in core
>> RTC code - see https://marc.info/?l=linux-rtc&m=164858416511425&w=4
[snip]
>> diff --git a/drivers/rtc/debugfs.c b/drivers/rtc/debugfs.c
>> new file mode 100644
>> index 000000000000..5ceed5504033
>> --- /dev/null
>> +++ b/drivers/rtc/debugfs.c
>> @@ -0,0 +1,112 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +/*
>> + * Debugfs interface for testing RTC alarms.
>> + */
>> +#include <linux/debugfs.h>
>> +#include <linux/err.h>
>> +#include <linux/rtc.h>
>> +
>> +static struct dentry *rtc_main_debugfs_dir;
>> +
>> +void rtc_debugfs_init(void)
>> +{
>> +	struct dentry *ret = debugfs_create_dir("rtc", NULL);
>> +
>> +	// No error is critical here
> What do you mean by this?

I meant that even if debugfs_create_dir() fails, this does not matter here and the
caller of rtc_debugfs_init() may continue execution.

>> +	if (!IS_ERR(ret))
>> +		rtc_main_debugfs_dir = ret;
> This should not be needed.
>
> Just set the directory and away you go.
>
> But why even save it?  If you need it you can always look it up, right?
OK, I did not know about easy lookup of debugfs directories.
> And when are you removing this directory you created?
Never, as the RTC subsystem cannot be compiled as a module.
This is the main debugfs directory of RTC subsystem, directories for specific devices
(rtc0, etc.) are created below it.
Please correct me if I'm wrong, but I thought that empty debugfs directories do not hurt.


Thanks for reviewing,

Mateusz