lkml.org 
[lkml]   [2018]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings
Date
Julia Lawall <julia.lawall@lip6.fr> writes:

> On Thu, 29 Mar 2018, Fabio Estevam wrote:
>
>> Hi Julia,
>>
>> On Thu, Mar 29, 2018 at 4:12 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> > Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
>> > for debugfs files.
>> >
>> > Semantic patch information:
>> > Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
>> > imposes some significant overhead as compared to
>> > DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>>
>> Just curious: could you please expand on what "imposes some
>> significant overhead" means?
>
> I don't know. I didn't write this rule. Nicolai, can you explain?

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,

Nicolai


--
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
HRB 21284 (AG Nürnberg)

\
 
 \ /
  Last update: 2018-03-30 08:15    [W:0.076 / U:0.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site