linux-kernel - Re: [PATCH v2 0/4] debugfs: Add simple min/max "files" to debugfs to fix sched debug code.

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] [day] [month] [year] [list]

Date:   Tue, 30 May 2023 18:24:49 -0400
From:   chris hyser <chris.hyser@...cle.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, peterz@...radead.org,
        "Rafael J. Wysocki" <rafael@...nel.org>
Subject: Re: [PATCH v2 0/4] debugfs: Add simple min/max "files" to debugfs to
 fix sched debug code.

On 5/30/23 17:41, chris hyser wrote:
> On 5/30/23 16:18, Greg KH wrote:
>> On Tue, May 30, 2023 at 03:40:08PM -0400, chris hyser wrote:
>>> v2:
>>> Apologies. I sent this the first time without including lkml.
>>>
>>> v1:
>>> This originally started as an attempt to solve a divide by zero issue 
>>> in sched
>>> debug code that was introduced when a sysctl value with non-zero 
>>> checking was
>>> moved to a simple u32 debugfs file. In looking at ways to solve this, 
>>> it was
>>> mentioned I should look at providing general debugfs files with min/max
>>> checking.
>>>
>>> One problem was that a check for greater than zero for say a u8 
>>> succeeds for a
>>> number like 256 (but stores a zero anyway) as the upper bits that 
>>> don't fit into
>>> storage are silently dropped. Therefore values greater than the 
>>> storage capacity
>>> must also fail. Getting an error when what the user wrote is not what 
>>> was
>>> actually stored, seems like a useful requirement for the other simple 
>>> files and
>>> so I moved the check into there.
>>>
>>> To enable easy testing, a test module and test script are provided 
>>> which can
>>> validate the new functions as well as check the new limits on the older
>>> functions. This was stuck under selftests, but it is not currently 
>>> tied into the
>>> testing infrastructure.
>>
>> This is a lot of new infrastructure for a single debugfs file that you
>> could just check for in the file write callback instead.
>>
>> I'm all for cool new features, but wow, this seems like major overkill.
>> Are you _SURE_ you need it all?

I do want to clarify about the size of this. It is 4 new create file 
functions, 8 static get/sets, a new struct and some simple macros. In 
keeping the style of the new code similar to prior, the lines of 
function comments for the new creates() almost equals the lines of code.

This doesn't seem to me to be as much overkill as say the xsigned 
version of these files which consumes 12 struct file_operations simply 
to provide a different string format.

-chrish