linux-kernel - Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple

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:   Fri, 30 Mar 2018 12:33:27 -0300
From:   Fabio Estevam <festevam@...il.com>
To:     Julia Lawall <julia.lawall@...6.fr>
Cc:     Nicolai Stange <nstange@...e.de>,
        Francisco Jerez <currojerez@...eup.net>,
        linux-pm@...r.kernel.org, Viresh Kumar <viresh.kumar@...aro.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        linux-kernel <linux-kernel@...r.kernel.org>, kbuild-all@...org,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        0day robot <fengguang.wu@...el.com>,
        Len Brown <lenb@...nel.org>
Subject: Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix
 debugfs_simple_attr.cocci warnings

On Fri, Mar 30, 2018 at 3:22 AM, Julia Lawall <julia.lawall@...6.fr> wrote:

>> From commit 49d200deaa68 ("debugfs: prevent access to removed files' private
>> data"):
>>
>>     Upon return of debugfs_remove()/debugfs_remove_recursive(), it might
>>     still be attempted to access associated private file data through
>>     previously opened struct file objects. If that data has been freed by
>>     the caller of debugfs_remove*() in the meanwhile, the reading/writing
>>     process would either encounter a fault or, if the memory address in
>>     question has been reassigned again, unrelated data structures could get
>>     overwritten.
>>     [...]
>>     Currently, there are ~1000 call sites of debugfs_create_file() spread
>>     throughout the whole tree and touching all of those struct file_operations
>>     in order to make them file removal aware by means of checking the result of
>>     debugfs_use_file_start() from within their methods is unfeasible.
>>
>>     Instead, wrap the struct file_operations by a lifetime managing proxy at
>>     file open [...]
>>
>> The additional overhead comes in terms of additional memory needed: for
>> debugs files created through debugfs_create_file(), one such struct
>> file_operations proxy is allocated for each struct file instantiation,
>> c.f. full_proxy_open().
>>
>> This was needed to "repair" the ~1000 call sites without touching them.
>>
>> New debugfs users should make their file_operations removal aware
>> themselves by means of DEFINE_DEBUGFS_ATTRIBUTE() and signal that fact to
>> the debugfs core by instantiating them through
>> debugfs_create_file_unsafe().
>>
>> See commit c64688081490 ("debugfs: add support for self-protecting
>> attribute file fops") for further information.

Thanks for the detailed explanation, Nicolai!

> Thanks.  Perhaps it would be good to add a reference to this commit in
> the message generated by the semantic patch.

Yes, that would be very helpful indeed.

Thanks