From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753761AbbLJMrZ (ORCPT ); Thu, 10 Dec 2015 07:47:25 -0500 Received: from mail-wm0-f50.google.com ([74.125.82.50]:35101 "EHLO mail-wm0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751294AbbLJMrX (ORCPT ); Thu, 10 Dec 2015 07:47:23 -0500 From: Roman Pen Cc: Roman Pen , Greg Kroah-Hartman , linux-kernel@vger.kernel.org Subject: [RFC PATCH 0/3] debugfs: make __debugfs_remove wait for dentry release Date: Thu, 10 Dec 2015 13:47:11 +0100 Message-Id: <1449751634-7887-1-git-send-email-r.peniaev@gmail.com> X-Mailer: git-send-email 2.6.2 To: unlisted-recipients:; (no To-header on input) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello. Recently I observed that all over the sources people expect that debugfs_remove() should behave as a barrier, hence file operations won't be invoked after it is called and it is safe to free remain memory. But by default debugfs_remove() does not guarantee that dentry will be released, and after this call fops->open/read/write can still be invoked if someone holds the reference on dentry because of successfull lookup. For example here is the grep output: *** drivers/block/pktcdvd.c: pkt_debugfs_dev_remove[489] debugfs_remove(pd->dfs_f_info); pkt_debugfs_dev_remove[490] debugfs_remove(pd->dfs_d_root); *** drivers/char/virtio_console.c: unplug_port[1595] debugfs_remove(port->debugfs_file); *** drivers/crypto/qat/qat_common/adf_cfg.c: adf_cfg_dev_remove[187] debugfs_remove(dev_cfg_data->debug); *** drivers/gpu/drm/drm_debugfs.c: drm_debugfs_remove_files[203] debugfs_remove(tmp->dent); .... and more and more and more ... where people do the following sequence: debugfs_remove(dev->debugfs_dentry); kfree(dev); and 'dev' pointer was passed to 'debugfs_create_file("my_dev", , dev, dev_fops)', so any access to 'dev' in file operations can lead to usage-after-free. In this patch __debugfs_remove() waits for last dentry release callback. BUT! I am not sure that nobody tries to remove the dentry from it's own file operation (dentry suicide). And if so - deadlock will happen. Probably, dentry_remove_self() should be implemented for such cases, which is similar to sysfs_remove_file_self(). But for now I do not want to add new function which can be useless in the nearest future. Also I did a fix for automount inodes and increased i_nlink references because of WARNING at fs/inode.c:273 drop_nlink. Cc: Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org Roman Pen (3): debugfs: fix automount inode i_nlink references debugfs: put private data to i_private for automount inode debugfs: make __debugfs_remove wait for dentry release fs/debugfs/inode.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 7 deletions(-) -- 2.6.2