From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolai Stange Subject: Re: [kbuild-all] [PATCH] OPTIONAL: cpufreq/intel_pstate: fix debugfs_simple_attr.cocci warnings Date: Fri, 30 Mar 2018 08:14:40 +0200 Message-ID: <87po3mxf73.fsf@suse.de> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: (Julia Lawall's message of "Thu, 29 Mar 2018 21:44:02 +0200 (CEST)") Sender: linux-kernel-owner@vger.kernel.org To: Fabio Estevam Cc: Julia Lawall , Francisco Jerez , linux-pm@vger.kernel.org, Viresh Kumar , "Rafael J. Wysocki" , linux-kernel , kbuild-all@01.org, Srinivas Pandruvada , 0day robot , Len Brown , Nicolai Stange List-Id: linux-pm@vger.kernel.org Julia Lawall writes: > On Thu, 29 Mar 2018, Fabio Estevam wrote: > >> Hi Julia, >> >> On Thu, Mar 29, 2018 at 4:12 PM, Julia Lawall 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)