• Nicolai Stange's avatar
    debugfs: prevent access to possibly dead file_operations at file open · 9fd4dcec
    Nicolai Stange authored
    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
    ->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 ->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 ->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 ->f_op to the original
      file_operations and forwards the ongoing open() call to the original
      ->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 ->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: default avatarNicolai Stange <nicstange@gmail.com>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    9fd4dcec
inode.c 20.9 KB