lkml.org 
[lkml]   [2018]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings


On Fri, 30 Mar 2018, Nicolai Stange wrote:

> 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. Perhaps it would be good to add a reference to this commit in
the message generated by the semantic patch.

Would it be sufficient to just apply the semantic patch everywhere and
submit the patches?

julia

>
>
> Thanks,
>
> Nicolai
>
>
> --
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton,
> HRB 21284 (AG Nürnberg)
>
\
 
 \ /
  Last update: 2018-03-30 08:23    [W:0.156 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site