<feed xmlns='http://www.w3.org/2005/Atom'>
<title>linux/fs/debugfs/inode.c, branch v4.7</title>
<subtitle>Mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
</subtitle>
<id>https://git.shady.money/linux/atom?h=v4.7</id>
<link rel='self' href='https://git.shady.money/linux/atom?h=v4.7'/>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/'/>
<updated>2016-04-18T19:28:28Z</updated>
<entry>
<title>Merge 4.6-rc4 into driver-core-next</title>
<updated>2016-04-18T19:28:28Z</updated>
<author>
<name>Greg Kroah-Hartman</name>
<email>gregkh@linuxfoundation.org</email>
</author>
<published>2016-04-18T19:28:28Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=5614e7725856ea383f458377980298111439e0fb'/>
<id>urn:sha1:5614e7725856ea383f458377980298111439e0fb</id>
<content type='text'>
We want those fixes in here as well.

Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>debugfs: Make automount point inodes permanently empty</title>
<updated>2016-04-12T22:01:53Z</updated>
<author>
<name>Seth Forshee</name>
<email>seth.forshee@canonical.com</email>
</author>
<published>2016-03-09T15:18:07Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=87243deb88671f70def4c52dfa7ca7830707bd31'/>
<id>urn:sha1:87243deb88671f70def4c52dfa7ca7830707bd31</id>
<content type='text'>
Starting with 4.1 the tracing subsystem has its own filesystem
which is automounted in the tracing subdirectory of debugfs.
Prior to this debugfs could be bind mounted in a cloned mount
namespace, but if tracefs has been mounted under debugfs this
now fails because there is a locked child mount. This creates
a regression for container software which bind mounts debugfs
to satisfy the assumption of some userspace software.

In other pseudo filesystems such as proc and sysfs we're already
creating mountpoints like this in such a way that no dirents can
be created in the directories, allowing them to be exceptions to
some MNT_LOCKED tests. In fact we're already do this for the
tracefs mountpoint in sysfs.

Do the same in debugfs_create_automount(), since the intention
here is clearly to create a mountpoint. This fixes the regression,
as locked child mounts on permanently empty directories do not
cause a bind mount to fail.

Cc: stable@vger.kernel.org # v4.1+
Signed-off-by: Seth Forshee &lt;seth.forshee@canonical.com&gt;
Acked-by: Serge Hallyn &lt;serge.hallyn@canonical.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>debugfs: add support for self-protecting attribute file fops</title>
<updated>2016-04-12T21:14:21Z</updated>
<author>
<name>Nicolai Stange</name>
<email>nicstange@gmail.com</email>
</author>
<published>2016-03-22T13:11:15Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=c64688081490321f2d23a292ef24e60bb321f3f1'/>
<id>urn:sha1:c64688081490321f2d23a292ef24e60bb321f3f1</id>
<content type='text'>
In order to protect them against file removal issues, debugfs_create_file()
creates a lifetime managing proxy around each struct file_operations
handed in.

In cases where this struct file_operations is able to manage file lifetime
by itself already, the proxy created by debugfs is a waste of resources.

The most common class of struct file_operations given to debugfs are those
defined by means of the DEFINE_SIMPLE_ATTRIBUTE() macro.

Introduce a DEFINE_DEBUGFS_ATTRIBUTE() macro to allow any
struct file_operations of this class to be easily made file lifetime aware
and thus, to be operated unproxied.

Specifically, introduce debugfs_attr_read() and debugfs_attr_write()
which wrap simple_attr_read() and simple_attr_write() under the protection
of a debugfs_use_file_start()/debugfs_use_file_finish() pair.

Make DEFINE_DEBUGFS_ATTRIBUTE() set the defined struct file_operations'
-&gt;read() and -&gt;write() members to these wrappers.

Export debugfs_create_file_unsafe() in order to allow debugfs users to
create their files in non-proxying operation mode.

Signed-off-by: Nicolai Stange &lt;nicstange@gmail.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>debugfs: prevent access to removed files' private data</title>
<updated>2016-04-12T21:14:21Z</updated>
<author>
<name>Nicolai Stange</name>
<email>nicstange@gmail.com</email>
</author>
<published>2016-03-22T13:11:14Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=49d200deaa680501f19a247b1fffb29301e51d2b'/>
<id>urn:sha1:49d200deaa680501f19a247b1fffb29301e51d2b</id>
<content type='text'>
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.

However, since debugfs files are seldomly removed, usually from module
exit handlers only, the impact is very low.

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:
- In debugfs_create_file(), the original fops handed in has got stashed
  away in -&gt;d_fsdata already.
- In debugfs_create_file(), install a proxy file_operations factory,
  debugfs_full_proxy_file_operations, at -&gt;i_fop.

This proxy factory has got an -&gt;open() method only. It carries out some
lifetime checks and if successful, dynamically allocates and sets up a new
struct file_operations proxy at -&gt;f_op. Afterwards, it forwards to the
-&gt;open() of the original struct file_operations in -&gt;d_fsdata, if any.

The dynamically set up proxy at -&gt;f_op has got a lifetime managing wrapper
set for each of the methods defined in the original struct file_operations
in -&gt;d_fsdata.

Its -&gt;release()er frees the proxy again and forwards to the original
-&gt;release(), if any.

In order not to mislead the VFS layer, it is strictly necessary to leave
those fields blank in the proxy that have been NULL in the original
struct file_operations also, i.e. aren't supported. This is why there is a
need for dynamically allocated proxies. The choice made not to allocate a
proxy instance for every dentry at file creation, but for every
struct file object instantiated thereof is justified by the expected usage
pattern of debugfs, namely that in general very few files get opened more
than once at a time.

The wrapper methods set in the struct file_operations implement lifetime
managing by means of the SRCU protection facilities already in place for
debugfs:
They set up a SRCU read side critical section and check whether the dentry
is still alive by means of debugfs_use_file_start(). If so, they forward
the call to the original struct file_operation stored in -&gt;d_fsdata, still
under the protection of the SRCU read side critical section.
This SRCU read side critical section prevents any pending debugfs_remove()
and friends to return to their callers. Since a file's private data must
only be freed after the return of debugfs_remove(), the ongoing proxied
call is guarded against any file removal race.

If, on the other hand, the initial call to debugfs_use_file_start() detects
that the dentry is dead, the wrapper simply returns -EIO and does not
forward the call. Note that the -&gt;poll() wrapper is special in that its
signature does not allow for the return of arbitrary -EXXX values and thus,
POLLHUP is returned here.

In order not to pollute debugfs with wrapper definitions that aren't ever
needed, I chose not to define a wrapper for every struct file_operations
method possible. Instead, a wrapper is defined only for the subset of
methods which are actually set by any debugfs users.
Currently, these are:

  -&gt;llseek()
  -&gt;read()
  -&gt;write()
  -&gt;unlocked_ioctl()
  -&gt;poll()

The -&gt;release() wrapper is special in that it does not protect the original
-&gt;release() in any way from dead files in order not to leak resources.
Thus, any -&gt;release() handed to debugfs must implement file lifetime
management manually, if needed.
For only 33 out of a total of 434 releasers handed in to debugfs, it could
not be verified immediately whether they access data structures that might
have been freed upon a debugfs_remove() return in the meanwhile.

Export debugfs_use_file_start() and debugfs_use_file_finish() in order to
allow any -&gt;release() to manually implement file lifetime management.

For a set of common cases of struct file_operations implemented by the
debugfs_core itself, future patches will incorporate file lifetime
management directly within those in order to allow for their unproxied
operation. Rename the original, non-proxying "debugfs_create_file()" to
"debugfs_create_file_unsafe()" and keep it for future internal use by
debugfs itself. Factor out code common to both into the new
__debugfs_create_file().

Signed-off-by: Nicolai Stange &lt;nicstange@gmail.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>debugfs: prevent access to possibly dead file_operations at file open</title>
<updated>2016-04-12T21:14:21Z</updated>
<author>
<name>Nicolai Stange</name>
<email>nicstange@gmail.com</email>
</author>
<published>2016-03-22T13:11:13Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=9fd4dcece43a53e5a9e65a973df5693702ee6401'/>
<id>urn:sha1:9fd4dcece43a53e5a9e65a973df5693702ee6401</id>
<content type='text'>
Nothing prevents a dentry found by path lookup before a return of
__debugfs_remove() to actually get opened after that return. Now, after
the return of __debugfs_remove(), there are no guarantees whatsoever
regarding the memory the corresponding inode's file_operations object
had been kept in.

Since __debugfs_remove() is seldomly invoked, usually from module exit
handlers only, the race is hard to trigger and the impact is very low.

A discussion of the problem outlined above as well as a suggested
solution can be found in the (sub-)thread rooted at

  http://lkml.kernel.org/g/20130401203445.GA20862@ZenIV.linux.org.uk
  ("Yet another pipe related oops.")

Basically, Greg KH suggests to introduce an intermediate fops and
Al Viro points out that a pointer to the original ones may be stored in
-&gt;d_fsdata.

Follow this line of reasoning:
- Add SRCU as a reverse dependency of DEBUG_FS.
- Introduce a srcu_struct object for the debugfs subsystem.
- In debugfs_create_file(), store a pointer to the original
  file_operations object in -&gt;d_fsdata.
- Make debugfs_remove() and debugfs_remove_recursive() wait for a
  SRCU grace period after the dentry has been delete()'d and before they
  return to their callers.
- Introduce an intermediate file_operations object named
  "debugfs_open_proxy_file_operations". It's -&gt;open() functions checks,
  under the protection of a SRCU read lock, whether the dentry is still
  alive, i.e. has not been d_delete()'d and if so, tries to acquire a
  reference on the owning module.
  On success, it sets the file object's -&gt;f_op to the original
  file_operations and forwards the ongoing open() call to the original
  -&gt;open().
- For clarity, rename the former debugfs_file_operations to
  debugfs_noop_file_operations -- they are in no way canonical.

The choice of SRCU over "normal" RCU is justified by the fact, that the
former may also be used to protect -&gt;i_private data from going away
during the execution of a file's readers and writers which may (and do)
sleep.

Finally, introduce the fs/debugfs/internal.h header containing some
declarations internal to the debugfs implementation.

Signed-off-by: Nicolai Stange &lt;nicstange@gmail.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>fs: debugfs: Replace CURRENT_TIME by current_fs_time()</title>
<updated>2016-03-29T17:11:44Z</updated>
<author>
<name>Deepa Dinamani</name>
<email>deepa.kernel@gmail.com</email>
</author>
<published>2016-02-22T15:17:47Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=1b48b530dac3e600d92854d98a2a519243661f6c'/>
<id>urn:sha1:1b48b530dac3e600d92854d98a2a519243661f6c</id>
<content type='text'>
CURRENT_TIME macro is not appropriate for filesystems as it
doesn't use the right granularity for filesystem timestamps.
Use current_fs_time() instead.

Signed-off-by: Deepa Dinamani &lt;deepa.kernel@gmail.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>debugfs: fix inode i_nlink references for automount dentry</title>
<updated>2016-03-29T17:11:44Z</updated>
<author>
<name>Roman Pen</name>
<email>r.peniaev@gmail.com</email>
</author>
<published>2016-02-09T10:30:29Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=a8f324a46fbe5473633af00039e81821be0ce51b'/>
<id>urn:sha1:a8f324a46fbe5473633af00039e81821be0ce51b</id>
<content type='text'>
Directory inodes should start off with i_nlink == 2 (one extra ref
for "." entry).  debugfs_create_automount() increases neither the
i_nlink reference for current inode nor for parent inode.

On attempt to remove the automount dentry, kernel complains:

  [   86.288070] WARNING: CPU: 1 PID: 3616 at fs/inode.c:273 drop_nlink+0x3e/0x50()
  [   86.288461] Modules linked in: debugfs_example2(O-)
  [   86.288745] CPU: 1 PID: 3616 Comm: rmmod Tainted: G           O    4.4.0-rc3-next-20151207+ #135
  [   86.289197] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.2-20150617_082717-anatol 04/01/2014
  [   86.289696]  ffffffff81be05c9 ffff8800b9e6fda0 ffffffff81352e2c 0000000000000000
  [   86.290110]  ffff8800b9e6fdd8 ffffffff81065142 ffff8801399175e8 ffff8800bb78b240
  [   86.290507]  ffff8801399175e8 ffff8800b73d7898 ffff8800b73d7840 ffff8800b9e6fde8
  [   86.290933] Call Trace:
  [   86.291080]  [&lt;ffffffff81352e2c&gt;] dump_stack+0x4e/0x82
  [   86.291340]  [&lt;ffffffff81065142&gt;] warn_slowpath_common+0x82/0xc0
  [   86.291640]  [&lt;ffffffff8106523a&gt;] warn_slowpath_null+0x1a/0x20
  [   86.291932]  [&lt;ffffffff811ae62e&gt;] drop_nlink+0x3e/0x50
  [   86.292208]  [&lt;ffffffff811ba35b&gt;] simple_unlink+0x4b/0x60
  [   86.292481]  [&lt;ffffffff811ba3a7&gt;] simple_rmdir+0x37/0x50
  [   86.292748]  [&lt;ffffffff812d9808&gt;] __debugfs_remove.part.16+0xa8/0xd0
  [   86.293082]  [&lt;ffffffff812d9a0b&gt;] debugfs_remove_recursive+0xdb/0x1c0
  [   86.293406]  [&lt;ffffffffa00004dd&gt;] cleanup_module+0x2d/0x3b [debugfs_example2]
  [   86.293762]  [&lt;ffffffff810d959b&gt;] SyS_delete_module+0x16b/0x220
  [   86.294077]  [&lt;ffffffff818ef857&gt;] entry_SYSCALL_64_fastpath+0x12/0x6a
  [   86.294405] ---[ end trace c9fc53353fe14a36 ]---
  [   86.294639] ------------[ cut here ]------------

To reproduce the issue it is enough to invoke these lines:

     autom = debugfs_create_automount("automount", NULL, vfsmount_cb, data);
     BUG_ON(IS_ERR_OR_NULL(autom));
     debugfs_remove(autom);

The issue is fixed by increasing inode i_nlink references for current
and parent inodes.

Signed-off-by: Roman Pen &lt;r.peniaev@gmail.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>wrappers for -&gt;i_mutex access</title>
<updated>2016-01-22T23:04:28Z</updated>
<author>
<name>Al Viro</name>
<email>viro@zeniv.linux.org.uk</email>
</author>
<published>2016-01-22T20:40:57Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=5955102c9984fa081b2d570cfac75c97eecf8f3b'/>
<id>urn:sha1:5955102c9984fa081b2d570cfac75c97eecf8f3b</id>
<content type='text'>
parallel to mutex_{lock,unlock,trylock,is_locked,lock_nested},
inode_foo(inode) being mutex_foo(&amp;inode-&gt;i_mutex).

Please, use those for access to -&gt;i_mutex; over the coming cycle
-&gt;i_mutex will become rwsem, with -&gt;lookup() done with it held
only shared.

Signed-off-by: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
</content>
</entry>
<entry>
<title>debugfs: fix refcount imbalance in start_creating</title>
<updated>2015-11-11T07:04:44Z</updated>
<author>
<name>Daniel Borkmann</name>
<email>daniel@iogearbox.net</email>
</author>
<published>2015-11-04T23:01:51Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=0ee9608c89e81a1ccee52ecb58a7ff040e2522d9'/>
<id>urn:sha1:0ee9608c89e81a1ccee52ecb58a7ff040e2522d9</id>
<content type='text'>
In debugfs' start_creating(), we pin the file system to safely access
its root. When we failed to create a file, we unpin the file system via
failed_creating() to release the mount count and eventually the reference
of the vfsmount.

However, when we run into an error during lookup_one_len() when still
in start_creating(), we only release the parent's mutex but not so the
reference on the mount. Looks like it was done in the past, but after
splitting portions of __create_file() into start_creating() and
end_creating() via 190afd81e4a5 ("debugfs: split the beginning and the
end of __create_file() off"), this seemed missed. Noticed during code
review.

Fixes: 190afd81e4a5 ("debugfs: split the beginning and the end of __create_file() off")
Cc: stable@vger.kernel.org # v4.0+
Signed-off-by: Daniel Borkmann &lt;daniel@iogearbox.net&gt;
Signed-off-by: Al Viro &lt;viro@zeniv.linux.org.uk&gt;
</content>
</entry>
<entry>
<title>debugfs: document that debugfs_remove*() accepts NULL and error values</title>
<updated>2015-10-04T10:36:07Z</updated>
<author>
<name>Ulf Magnusson</name>
<email>ulfalizer@gmail.com</email>
</author>
<published>2015-09-07T17:03:15Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/linux/commit/?id=398dc4ad52022c3d585908544b936bdf73640727'/>
<id>urn:sha1:398dc4ad52022c3d585908544b936bdf73640727</id>
<content type='text'>
According to commit a59d6293e537 ("debugfs: change parameter check in
debugfs_remove() functions"), this is meant to make cleanup easier for
callers. In that case it ought to be documented.

Signed-off-by: Ulf Magnusson &lt;ulfalizer@gmail.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
</feed>
