* [PATCH v3 1/8] debugfs: add support for more elaborate ->d_fsdata
2017-10-30 23:15 ` [PATCH v3 0/8] debugfs: per-file removal protection Nicolai Stange
@ 2017-10-30 23:15 ` Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 2/8] debugfs: implement per-file removal protection Nicolai Stange
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Nicolai Stange @ 2017-10-30 23:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Paul E.McKenney, Tyler Hall, Mike Marciniszyn,
Dennis Dalessandro, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel, Nicolai Stange
Currently, the user provided fops, "real_fops", are stored directly into
->d_fsdata.
In order to be able to store more per-file state and thus prepare for more
granular file removal protection, wrap the real_fops into a dynamically
allocated container struct, debugfs_fsdata.
A struct debugfs_fsdata gets allocated at file creation and freed from the
newly intoduced ->d_release().
Finally, move the implementation of debugfs_real_fops() out of the public
debugfs header such that struct debugfs_fsdata's declaration can be kept
private.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
fs/debugfs/file.c | 12 ++++++++++++
fs/debugfs/inode.c | 22 +++++++++++++++++++---
fs/debugfs/internal.h | 4 ++++
include/linux/debugfs.h | 20 +++-----------------
4 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 6dabc4a10396..b6f5ddab66bf 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -97,6 +97,18 @@ EXPORT_SYMBOL_GPL(debugfs_use_file_finish);
#define F_DENTRY(filp) ((filp)->f_path.dentry)
+const struct file_operations *debugfs_real_fops(const struct file *filp)
+ __must_hold(&debugfs_srcu)
+{
+ struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata;
+ /*
+ * Neither the pointer to the struct file_operations, nor its
+ * contents ever change -- srcu_dereference() is not needed here.
+ */
+ return fsd->real_fops;
+}
+EXPORT_SYMBOL_GPL(debugfs_real_fops);
+
static int open_proxy_open(struct inode *inode, struct file *filp)
{
const struct dentry *dentry = F_DENTRY(filp);
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index c59f015f386e..a9c3d3e9af39 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -185,6 +185,11 @@ static const struct super_operations debugfs_super_operations = {
.evict_inode = debugfs_evict_inode,
};
+static void debugfs_release_dentry(struct dentry *dentry)
+{
+ kfree(dentry->d_fsdata);
+}
+
static struct vfsmount *debugfs_automount(struct path *path)
{
debugfs_automount_t f;
@@ -194,6 +199,7 @@ static struct vfsmount *debugfs_automount(struct path *path)
static const struct dentry_operations debugfs_dops = {
.d_delete = always_delete_dentry,
+ .d_release = debugfs_release_dentry,
.d_automount = debugfs_automount,
};
@@ -341,24 +347,34 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
{
struct dentry *dentry;
struct inode *inode;
+ struct debugfs_fsdata *fsd;
+
+ fsd = kmalloc(sizeof(*fsd), GFP_KERNEL);
+ if (!fsd)
+ return NULL;
if (!(mode & S_IFMT))
mode |= S_IFREG;
BUG_ON(!S_ISREG(mode));
dentry = start_creating(name, parent);
- if (IS_ERR(dentry))
+ if (IS_ERR(dentry)) {
+ kfree(fsd);
return NULL;
+ }
inode = debugfs_get_inode(dentry->d_sb);
- if (unlikely(!inode))
+ if (unlikely(!inode)) {
+ kfree(fsd);
return failed_creating(dentry);
+ }
inode->i_mode = mode;
inode->i_private = data;
inode->i_fop = proxy_fops;
- dentry->d_fsdata = (void *)real_fops;
+ fsd->real_fops = real_fops;
+ dentry->d_fsdata = fsd;
d_instantiate(dentry, inode);
fsnotify_create(d_inode(dentry->d_parent), dentry);
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index b3e8443a1f47..512601eed3ce 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -19,4 +19,8 @@ extern const struct file_operations debugfs_noop_file_operations;
extern const struct file_operations debugfs_open_proxy_file_operations;
extern const struct file_operations debugfs_full_proxy_file_operations;
+struct debugfs_fsdata {
+ const struct file_operations *real_fops;
+};
+
#endif /* _DEBUGFS_INTERNAL_H_ */
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index b93efc8feecd..cbee5f4a02a3 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -45,23 +45,6 @@ extern struct dentry *arch_debugfs_dir;
extern struct srcu_struct debugfs_srcu;
-/**
- * debugfs_real_fops - getter for the real file operation
- * @filp: a pointer to a struct file
- *
- * Must only be called under the protection established by
- * debugfs_use_file_start().
- */
-static inline const struct file_operations *debugfs_real_fops(const struct file *filp)
- __must_hold(&debugfs_srcu)
-{
- /*
- * Neither the pointer to the struct file_operations, nor its
- * contents ever change -- srcu_dereference() is not needed here.
- */
- return filp->f_path.dentry->d_fsdata;
-}
-
#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \
static int __fops ## _open(struct inode *inode, struct file *file) \
{ \
@@ -112,6 +95,9 @@ int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx)
void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu);
+const struct file_operations *debugfs_real_fops(const struct file *filp)
+ __must_hold(&debugfs_srcu);
+
ssize_t debugfs_attr_read(struct file *file, char __user *buf,
size_t len, loff_t *ppos);
ssize_t debugfs_attr_write(struct file *file, const char __user *buf,
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 2/8] debugfs: implement per-file removal protection
2017-10-30 23:15 ` [PATCH v3 0/8] debugfs: per-file removal protection Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 1/8] debugfs: add support for more elaborate ->d_fsdata Nicolai Stange
@ 2017-10-30 23:15 ` Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 3/8] debugfs: debugfs_real_fops(): drop __must_hold sparse annotation Nicolai Stange
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Nicolai Stange @ 2017-10-30 23:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Paul E.McKenney, Tyler Hall, Mike Marciniszyn,
Dennis Dalessandro, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel, Nicolai Stange
Since commit 49d200deaa68 ("debugfs: prevent access to removed files'
private data"), accesses to a file's private data are protected from
concurrent removal by covering all file_operations with a SRCU read section
and sychronizing with those before returning from debugfs_remove() by means
of synchronize_srcu().
As pointed out by Johannes Berg, there are debugfs files with forever
blocking file_operations. Their corresponding SRCU read side sections would
block any debugfs_remove() forever as well, even unrelated ones. This
results in a livelock. Because a remover can't cancel any indefinite
blocking within foreign files, this is a problem.
Resolve this by introducing support for more granular protection on a
per-file basis.
This is implemented by introducing an 'active_users' refcount_t to the
per-file struct debugfs_fsdata state. At file creation time, it is set to
one and a debugfs_remove() will drop that initial reference. The new
debugfs_file_get() and debugfs_file_put(), intended to be used in place of
former debugfs_use_file_start() and debugfs_use_file_finish(), increment
and decrement it respectively. Once the count drops to zero,
debugfs_file_put() will signal a completion which is possibly being waited
for from debugfs_remove().
Thus, as long as there is a debugfs_file_get() not yet matched by a
corresponding debugfs_file_put() around, debugfs_remove() will block.
Actual users of debugfs_use_file_start() and -finish() will get converted
to the new debugfs_file_get() and debugfs_file_put() by followup patches.
Fixes: 49d200deaa68 ("debugfs: prevent access to removed files' private
data")
Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
fs/debugfs/file.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/debugfs/inode.c | 29 +++++++++++++++++++++++------
fs/debugfs/internal.h | 2 ++
include/linux/debugfs.h | 11 +++++++++++
4 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index b6f5ddab66bf..6644bfdea2f8 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -109,6 +109,54 @@ const struct file_operations *debugfs_real_fops(const struct file *filp)
}
EXPORT_SYMBOL_GPL(debugfs_real_fops);
+/**
+ * debugfs_file_get - mark the beginning of file data access
+ * @dentry: the dentry object whose data is being accessed.
+ *
+ * Up to a matching call to debugfs_file_put(), any successive call
+ * into the file removing functions debugfs_remove() and
+ * debugfs_remove_recursive() will block. Since associated private
+ * file data may only get freed after a successful return of any of
+ * the removal functions, you may safely access it after a successful
+ * call to debugfs_file_get() without worrying about lifetime issues.
+ *
+ * If -%EIO is returned, the file has already been removed and thus,
+ * it is not safe to access any of its data. If, on the other hand,
+ * it is allowed to access the file data, zero is returned.
+ */
+int debugfs_file_get(struct dentry *dentry)
+{
+ struct debugfs_fsdata *fsd = dentry->d_fsdata;
+
+ /* Avoid starvation of removers. */
+ if (d_unlinked(dentry))
+ return -EIO;
+
+ if (!refcount_inc_not_zero(&fsd->active_users))
+ return -EIO;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(debugfs_file_get);
+
+/**
+ * debugfs_file_put - mark the end of file data access
+ * @dentry: the dentry object formerly passed to
+ * debugfs_file_get().
+ *
+ * Allow any ongoing concurrent call into debugfs_remove() or
+ * debugfs_remove_recursive() blocked by a former call to
+ * debugfs_file_get() to proceed and return to its caller.
+ */
+void debugfs_file_put(struct dentry *dentry)
+{
+ struct debugfs_fsdata *fsd = dentry->d_fsdata;
+
+ if (refcount_dec_and_test(&fsd->active_users))
+ complete(&fsd->active_users_drained);
+}
+EXPORT_SYMBOL_GPL(debugfs_file_put);
+
static int open_proxy_open(struct inode *inode, struct file *filp)
{
const struct dentry *dentry = F_DENTRY(filp);
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index a9c3d3e9af39..6449eb935540 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -374,6 +374,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
inode->i_fop = proxy_fops;
fsd->real_fops = real_fops;
+ refcount_set(&fsd->active_users, 1);
dentry->d_fsdata = fsd;
d_instantiate(dentry, inode);
@@ -631,18 +632,34 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
}
EXPORT_SYMBOL_GPL(debugfs_create_symlink);
+static void __debugfs_remove_file(struct dentry *dentry, struct dentry *parent)
+{
+ struct debugfs_fsdata *fsd;
+
+ simple_unlink(d_inode(parent), dentry);
+ d_delete(dentry);
+ fsd = dentry->d_fsdata;
+ init_completion(&fsd->active_users_drained);
+ if (!refcount_dec_and_test(&fsd->active_users))
+ wait_for_completion(&fsd->active_users_drained);
+}
+
static int __debugfs_remove(struct dentry *dentry, struct dentry *parent)
{
int ret = 0;
if (simple_positive(dentry)) {
dget(dentry);
- if (d_is_dir(dentry))
- ret = simple_rmdir(d_inode(parent), dentry);
- else
- simple_unlink(d_inode(parent), dentry);
- if (!ret)
- d_delete(dentry);
+ if (!d_is_reg(dentry)) {
+ if (d_is_dir(dentry))
+ ret = simple_rmdir(d_inode(parent), dentry);
+ else
+ simple_unlink(d_inode(parent), dentry);
+ if (!ret)
+ d_delete(dentry);
+ } else {
+ __debugfs_remove_file(dentry, parent);
+ }
dput(dentry);
}
return ret;
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 512601eed3ce..0eea99432840 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -21,6 +21,8 @@ extern const struct file_operations debugfs_full_proxy_file_operations;
struct debugfs_fsdata {
const struct file_operations *real_fops;
+ refcount_t active_users;
+ struct completion active_users_drained;
};
#endif /* _DEBUGFS_INTERNAL_H_ */
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index cbee5f4a02a3..3b914d588148 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -98,6 +98,9 @@ void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu);
const struct file_operations *debugfs_real_fops(const struct file *filp)
__must_hold(&debugfs_srcu);
+int debugfs_file_get(struct dentry *dentry);
+void debugfs_file_put(struct dentry *dentry);
+
ssize_t debugfs_attr_read(struct file *file, char __user *buf,
size_t len, loff_t *ppos);
ssize_t debugfs_attr_write(struct file *file, const char __user *buf,
@@ -236,6 +239,14 @@ static inline void debugfs_use_file_finish(int srcu_idx)
__releases(&debugfs_srcu)
{ }
+static inline int debugfs_file_get(struct dentry *dentry)
+{
+ return 0;
+}
+
+static inline void debugfs_file_put(struct dentry *dentry)
+{ }
+
static inline ssize_t debugfs_attr_read(struct file *file, char __user *buf,
size_t len, loff_t *ppos)
{
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 3/8] debugfs: debugfs_real_fops(): drop __must_hold sparse annotation
2017-10-30 23:15 ` [PATCH v3 0/8] debugfs: per-file removal protection Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 1/8] debugfs: add support for more elaborate ->d_fsdata Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 2/8] debugfs: implement per-file removal protection Nicolai Stange
@ 2017-10-30 23:15 ` Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 4/8] debugfs: convert to debugfs_file_get() and -put() Nicolai Stange
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Nicolai Stange @ 2017-10-30 23:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Paul E.McKenney, Tyler Hall, Mike Marciniszyn,
Dennis Dalessandro, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel, Nicolai Stange
Currently, debugfs_real_fops() is annotated with a
__must_hold(&debugfs_srcu) sparse annotation.
With the conversion of the SRCU based protection of users against
concurrent file removals to a per-file refcount based scheme, this becomes
wrong.
Drop this annotation.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
fs/debugfs/file.c | 6 +-----
include/linux/debugfs.h | 3 +--
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 6644bfdea2f8..08511678b782 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -98,13 +98,9 @@ EXPORT_SYMBOL_GPL(debugfs_use_file_finish);
#define F_DENTRY(filp) ((filp)->f_path.dentry)
const struct file_operations *debugfs_real_fops(const struct file *filp)
- __must_hold(&debugfs_srcu)
{
struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata;
- /*
- * Neither the pointer to the struct file_operations, nor its
- * contents ever change -- srcu_dereference() is not needed here.
- */
+
return fsd->real_fops;
}
EXPORT_SYMBOL_GPL(debugfs_real_fops);
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 3b914d588148..c5eda259b9d6 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -95,8 +95,7 @@ int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx)
void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu);
-const struct file_operations *debugfs_real_fops(const struct file *filp)
- __must_hold(&debugfs_srcu);
+const struct file_operations *debugfs_real_fops(const struct file *filp);
int debugfs_file_get(struct dentry *dentry);
void debugfs_file_put(struct dentry *dentry);
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 4/8] debugfs: convert to debugfs_file_get() and -put()
2017-10-30 23:15 ` [PATCH v3 0/8] debugfs: per-file removal protection Nicolai Stange
` (2 preceding siblings ...)
2017-10-30 23:15 ` [PATCH v3 3/8] debugfs: debugfs_real_fops(): drop __must_hold sparse annotation Nicolai Stange
@ 2017-10-30 23:15 ` Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 5/8] IB/hfi1: " Nicolai Stange
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Nicolai Stange @ 2017-10-30 23:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Paul E.McKenney, Tyler Hall, Mike Marciniszyn,
Dennis Dalessandro, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel, Nicolai Stange
Convert all calls to the now obsolete debugfs_use_file_start() and
debugfs_use_file_finish() from the debugfs core itself to the new
debugfs_file_get() and debugfs_file_put() API.
Fixes: 49d200deaa68 ("debugfs: prevent access to removed files' private
data")
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
fs/debugfs/file.c | 106 ++++++++++++++++++++++++++----------------------------
1 file changed, 50 insertions(+), 56 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 08511678b782..d3a972b45ff0 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -155,15 +155,12 @@ EXPORT_SYMBOL_GPL(debugfs_file_put);
static int open_proxy_open(struct inode *inode, struct file *filp)
{
- const struct dentry *dentry = F_DENTRY(filp);
+ struct dentry *dentry = F_DENTRY(filp);
const struct file_operations *real_fops = NULL;
- int srcu_idx, r;
+ int r = 0;
- r = debugfs_use_file_start(dentry, &srcu_idx);
- if (r) {
- r = -ENOENT;
- goto out;
- }
+ if (debugfs_file_get(dentry))
+ return -ENOENT;
real_fops = debugfs_real_fops(filp);
real_fops = fops_get(real_fops);
@@ -180,7 +177,7 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
r = real_fops->open(inode, filp);
out:
- debugfs_use_file_finish(srcu_idx);
+ debugfs_file_put(dentry);
return r;
}
@@ -194,16 +191,16 @@ const struct file_operations debugfs_open_proxy_file_operations = {
#define FULL_PROXY_FUNC(name, ret_type, filp, proto, args) \
static ret_type full_proxy_ ## name(proto) \
{ \
- const struct dentry *dentry = F_DENTRY(filp); \
+ struct dentry *dentry = F_DENTRY(filp); \
const struct file_operations *real_fops = \
debugfs_real_fops(filp); \
- int srcu_idx; \
ret_type r; \
\
- r = debugfs_use_file_start(dentry, &srcu_idx); \
- if (likely(!r)) \
- r = real_fops->name(args); \
- debugfs_use_file_finish(srcu_idx); \
+ r = debugfs_file_get(dentry); \
+ if (unlikely(r)) \
+ return r; \
+ r = real_fops->name(args); \
+ debugfs_file_put(dentry); \
return r; \
}
@@ -228,18 +225,15 @@ FULL_PROXY_FUNC(unlocked_ioctl, long, filp,
static unsigned int full_proxy_poll(struct file *filp,
struct poll_table_struct *wait)
{
- const struct dentry *dentry = F_DENTRY(filp);
const struct file_operations *real_fops = debugfs_real_fops(filp);
- int srcu_idx;
+ struct dentry *dentry = F_DENTRY(filp);
unsigned int r = 0;
- if (debugfs_use_file_start(dentry, &srcu_idx)) {
- debugfs_use_file_finish(srcu_idx);
+ if (debugfs_file_get(dentry))
return POLLHUP;
- }
r = real_fops->poll(filp, wait);
- debugfs_use_file_finish(srcu_idx);
+ debugfs_file_put(dentry);
return r;
}
@@ -283,16 +277,13 @@ static void __full_proxy_fops_init(struct file_operations *proxy_fops,
static int full_proxy_open(struct inode *inode, struct file *filp)
{
- const struct dentry *dentry = F_DENTRY(filp);
+ struct dentry *dentry = F_DENTRY(filp);
const struct file_operations *real_fops = NULL;
struct file_operations *proxy_fops = NULL;
- int srcu_idx, r;
+ int r = 0;
- r = debugfs_use_file_start(dentry, &srcu_idx);
- if (r) {
- r = -ENOENT;
- goto out;
- }
+ if (debugfs_file_get(dentry))
+ return -ENOENT;
real_fops = debugfs_real_fops(filp);
real_fops = fops_get(real_fops);
@@ -330,7 +321,7 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
kfree(proxy_fops);
fops_put(real_fops);
out:
- debugfs_use_file_finish(srcu_idx);
+ debugfs_file_put(dentry);
return r;
}
@@ -341,13 +332,14 @@ const struct file_operations debugfs_full_proxy_file_operations = {
ssize_t debugfs_attr_read(struct file *file, char __user *buf,
size_t len, loff_t *ppos)
{
+ struct dentry *dentry = F_DENTRY(file);
ssize_t ret;
- int srcu_idx;
- ret = debugfs_use_file_start(F_DENTRY(file), &srcu_idx);
- if (likely(!ret))
- ret = simple_attr_read(file, buf, len, ppos);
- debugfs_use_file_finish(srcu_idx);
+ ret = debugfs_file_get(dentry);
+ if (unlikely(ret))
+ return ret;
+ ret = simple_attr_read(file, buf, len, ppos);
+ debugfs_file_put(dentry);
return ret;
}
EXPORT_SYMBOL_GPL(debugfs_attr_read);
@@ -355,13 +347,14 @@ EXPORT_SYMBOL_GPL(debugfs_attr_read);
ssize_t debugfs_attr_write(struct file *file, const char __user *buf,
size_t len, loff_t *ppos)
{
+ struct dentry *dentry = F_DENTRY(file);
ssize_t ret;
- int srcu_idx;
- ret = debugfs_use_file_start(F_DENTRY(file), &srcu_idx);
- if (likely(!ret))
- ret = simple_attr_write(file, buf, len, ppos);
- debugfs_use_file_finish(srcu_idx);
+ ret = debugfs_file_get(dentry);
+ if (unlikely(ret))
+ return ret;
+ ret = simple_attr_write(file, buf, len, ppos);
+ debugfs_file_put(dentry);
return ret;
}
EXPORT_SYMBOL_GPL(debugfs_attr_write);
@@ -795,14 +788,14 @@ ssize_t debugfs_read_file_bool(struct file *file, char __user *user_buf,
{
char buf[3];
bool val;
- int r, srcu_idx;
+ int r;
+ struct dentry *dentry = F_DENTRY(file);
- r = debugfs_use_file_start(F_DENTRY(file), &srcu_idx);
- if (likely(!r))
- val = *(bool *)file->private_data;
- debugfs_use_file_finish(srcu_idx);
- if (r)
+ r = debugfs_file_get(dentry);
+ if (unlikely(r))
return r;
+ val = *(bool *)file->private_data;
+ debugfs_file_put(dentry);
if (val)
buf[0] = 'Y';
@@ -820,8 +813,9 @@ ssize_t debugfs_write_file_bool(struct file *file, const char __user *user_buf,
char buf[32];
size_t buf_size;
bool bv;
- int r, srcu_idx;
+ int r;
bool *val = file->private_data;
+ struct dentry *dentry = F_DENTRY(file);
buf_size = min(count, (sizeof(buf)-1));
if (copy_from_user(buf, user_buf, buf_size))
@@ -829,12 +823,11 @@ ssize_t debugfs_write_file_bool(struct file *file, const char __user *user_buf,
buf[buf_size] = '\0';
if (strtobool(buf, &bv) == 0) {
- r = debugfs_use_file_start(F_DENTRY(file), &srcu_idx);
- if (likely(!r))
- *val = bv;
- debugfs_use_file_finish(srcu_idx);
- if (r)
+ r = debugfs_file_get(dentry);
+ if (unlikely(r))
return r;
+ *val = bv;
+ debugfs_file_put(dentry);
}
return count;
@@ -896,14 +889,15 @@ static ssize_t read_file_blob(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
struct debugfs_blob_wrapper *blob = file->private_data;
+ struct dentry *dentry = F_DENTRY(file);
ssize_t r;
- int srcu_idx;
- r = debugfs_use_file_start(F_DENTRY(file), &srcu_idx);
- if (likely(!r))
- r = simple_read_from_buffer(user_buf, count, ppos, blob->data,
- blob->size);
- debugfs_use_file_finish(srcu_idx);
+ r = debugfs_file_get(dentry);
+ if (unlikely(r))
+ return r;
+ r = simple_read_from_buffer(user_buf, count, ppos, blob->data,
+ blob->size);
+ debugfs_file_put(dentry);
return r;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 5/8] IB/hfi1: convert to debugfs_file_get() and -put()
2017-10-30 23:15 ` [PATCH v3 0/8] debugfs: per-file removal protection Nicolai Stange
` (3 preceding siblings ...)
2017-10-30 23:15 ` [PATCH v3 4/8] debugfs: convert to debugfs_file_get() and -put() Nicolai Stange
@ 2017-10-30 23:15 ` Nicolai Stange
[not found] ` <20171030231554.6200-6-nicstange-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-10-30 23:15 ` [PATCH v3 6/8] debugfs: purge obsolete SRCU based removal protection Nicolai Stange
` (2 subsequent siblings)
7 siblings, 1 reply; 10+ messages in thread
From: Nicolai Stange @ 2017-10-30 23:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Paul E.McKenney, Tyler Hall, Mike Marciniszyn,
Dennis Dalessandro, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel, Nicolai Stange
Convert all calls to the now obsolete debugfs_use_file_start() and
debugfs_use_file_finish() to the new debugfs_file_get() and
debugfs_file_put() API.
Fixes: 49d200deaa68 ("debugfs: prevent access to removed files' private
data")
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
drivers/infiniband/hw/hfi1/debugfs.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
index 24128f4e5748..ca03d8b23851 100644
--- a/drivers/infiniband/hw/hfi1/debugfs.c
+++ b/drivers/infiniband/hw/hfi1/debugfs.c
@@ -71,13 +71,13 @@ static ssize_t hfi1_seq_read(
loff_t *ppos)
{
struct dentry *d = file->f_path.dentry;
- int srcu_idx;
ssize_t r;
- r = debugfs_use_file_start(d, &srcu_idx);
- if (likely(!r))
- r = seq_read(file, buf, size, ppos);
- debugfs_use_file_finish(srcu_idx);
+ r = debugfs_file_get(d);
+ if (unlikely(r))
+ return r;
+ r = seq_read(file, buf, size, ppos);
+ debugfs_file_put(d);
return r;
}
@@ -87,13 +87,13 @@ static loff_t hfi1_seq_lseek(
int whence)
{
struct dentry *d = file->f_path.dentry;
- int srcu_idx;
loff_t r;
- r = debugfs_use_file_start(d, &srcu_idx);
- if (likely(!r))
- r = seq_lseek(file, offset, whence);
- debugfs_use_file_finish(srcu_idx);
+ r = debugfs_file_get(d);
+ if (unlikely(r))
+ return r;
+ r = seq_lseek(file, offset, whence);
+ debugfs_file_put(d);
return r;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 6/8] debugfs: purge obsolete SRCU based removal protection
2017-10-30 23:15 ` [PATCH v3 0/8] debugfs: per-file removal protection Nicolai Stange
` (4 preceding siblings ...)
2017-10-30 23:15 ` [PATCH v3 5/8] IB/hfi1: " Nicolai Stange
@ 2017-10-30 23:15 ` Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 7/8] debugfs: call debugfs_real_fops() only after debugfs_file_get() Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 8/8] debugfs: defer debugfs_fsdata allocation to first usage Nicolai Stange
7 siblings, 0 replies; 10+ messages in thread
From: Nicolai Stange @ 2017-10-30 23:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Paul E.McKenney, Tyler Hall, Mike Marciniszyn,
Dennis Dalessandro, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel, Nicolai Stange
Purge the SRCU based file removal race protection in favour of the new,
refcount based debugfs_file_get()/debugfs_file_put() API.
Fixes: 49d200deaa68 ("debugfs: prevent access to removed files' private
data")
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
fs/debugfs/file.c | 48 ------------------------------------------------
fs/debugfs/inode.c | 7 -------
include/linux/debugfs.h | 19 -------------------
lib/Kconfig.debug | 1 -
4 files changed, 75 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index d3a972b45ff0..53f5c9a2af88 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -22,7 +22,6 @@
#include <linux/slab.h>
#include <linux/atomic.h>
#include <linux/device.h>
-#include <linux/srcu.h>
#include <asm/poll.h>
#include "internal.h"
@@ -48,53 +47,6 @@ const struct file_operations debugfs_noop_file_operations = {
.llseek = noop_llseek,
};
-/**
- * debugfs_use_file_start - mark the beginning of file data access
- * @dentry: the dentry object whose data is being accessed.
- * @srcu_idx: a pointer to some memory to store a SRCU index in.
- *
- * Up to a matching call to debugfs_use_file_finish(), any
- * successive call into the file removing functions debugfs_remove()
- * and debugfs_remove_recursive() will block. Since associated private
- * file data may only get freed after a successful return of any of
- * the removal functions, you may safely access it after a successful
- * call to debugfs_use_file_start() without worrying about
- * lifetime issues.
- *
- * If -%EIO is returned, the file has already been removed and thus,
- * it is not safe to access any of its data. If, on the other hand,
- * it is allowed to access the file data, zero is returned.
- *
- * Regardless of the return code, any call to
- * debugfs_use_file_start() must be followed by a matching call
- * to debugfs_use_file_finish().
- */
-int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx)
- __acquires(&debugfs_srcu)
-{
- *srcu_idx = srcu_read_lock(&debugfs_srcu);
- barrier();
- if (d_unlinked(dentry))
- return -EIO;
- return 0;
-}
-EXPORT_SYMBOL_GPL(debugfs_use_file_start);
-
-/**
- * debugfs_use_file_finish - mark the end of file data access
- * @srcu_idx: the SRCU index "created" by a former call to
- * debugfs_use_file_start().
- *
- * Allow any ongoing concurrent call into debugfs_remove() or
- * debugfs_remove_recursive() blocked by a former call to
- * debugfs_use_file_start() to proceed and return to its caller.
- */
-void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu)
-{
- srcu_read_unlock(&debugfs_srcu, srcu_idx);
-}
-EXPORT_SYMBOL_GPL(debugfs_use_file_finish);
-
#define F_DENTRY(filp) ((filp)->f_path.dentry)
const struct file_operations *debugfs_real_fops(const struct file *filp)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 6449eb935540..f587aded46b5 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -27,14 +27,11 @@
#include <linux/parser.h>
#include <linux/magic.h>
#include <linux/slab.h>
-#include <linux/srcu.h>
#include "internal.h"
#define DEBUGFS_DEFAULT_MODE 0700
-DEFINE_SRCU(debugfs_srcu);
-
static struct vfsmount *debugfs_mount;
static int debugfs_mount_count;
static bool debugfs_registered;
@@ -693,8 +690,6 @@ void debugfs_remove(struct dentry *dentry)
inode_unlock(d_inode(parent));
if (!ret)
simple_release_fs(&debugfs_mount, &debugfs_mount_count);
-
- synchronize_srcu(&debugfs_srcu);
}
EXPORT_SYMBOL_GPL(debugfs_remove);
@@ -768,8 +763,6 @@ void debugfs_remove_recursive(struct dentry *dentry)
if (!__debugfs_remove(child, parent))
simple_release_fs(&debugfs_mount, &debugfs_mount_count);
inode_unlock(d_inode(parent));
-
- synchronize_srcu(&debugfs_srcu);
}
EXPORT_SYMBOL_GPL(debugfs_remove_recursive);
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index c5eda259b9d6..9f821a43bb0d 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -23,7 +23,6 @@
struct device;
struct file_operations;
-struct srcu_struct;
struct debugfs_blob_wrapper {
void *data;
@@ -43,8 +42,6 @@ struct debugfs_regset32 {
extern struct dentry *arch_debugfs_dir;
-extern struct srcu_struct debugfs_srcu;
-
#define DEFINE_DEBUGFS_ATTRIBUTE(__fops, __get, __set, __fmt) \
static int __fops ## _open(struct inode *inode, struct file *file) \
{ \
@@ -90,11 +87,6 @@ struct dentry *debugfs_create_automount(const char *name,
void debugfs_remove(struct dentry *dentry);
void debugfs_remove_recursive(struct dentry *dentry);
-int debugfs_use_file_start(const struct dentry *dentry, int *srcu_idx)
- __acquires(&debugfs_srcu);
-
-void debugfs_use_file_finish(int srcu_idx) __releases(&debugfs_srcu);
-
const struct file_operations *debugfs_real_fops(const struct file *filp);
int debugfs_file_get(struct dentry *dentry);
@@ -227,17 +219,6 @@ static inline void debugfs_remove(struct dentry *dentry)
static inline void debugfs_remove_recursive(struct dentry *dentry)
{ }
-static inline int debugfs_use_file_start(const struct dentry *dentry,
- int *srcu_idx)
- __acquires(&debugfs_srcu)
-{
- return 0;
-}
-
-static inline void debugfs_use_file_finish(int srcu_idx)
- __releases(&debugfs_srcu)
-{ }
-
static inline int debugfs_file_get(struct dentry *dentry)
{
return 0;
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 03e2ac7cceb6..f6140d5a50f1 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -326,7 +326,6 @@ config PAGE_OWNER
config DEBUG_FS
bool "Debug Filesystem"
- select SRCU
help
debugfs is a virtual file system that kernel developers use to put
debugging files into. Enable this option to be able to read and
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 7/8] debugfs: call debugfs_real_fops() only after debugfs_file_get()
2017-10-30 23:15 ` [PATCH v3 0/8] debugfs: per-file removal protection Nicolai Stange
` (5 preceding siblings ...)
2017-10-30 23:15 ` [PATCH v3 6/8] debugfs: purge obsolete SRCU based removal protection Nicolai Stange
@ 2017-10-30 23:15 ` Nicolai Stange
2017-10-30 23:15 ` [PATCH v3 8/8] debugfs: defer debugfs_fsdata allocation to first usage Nicolai Stange
7 siblings, 0 replies; 10+ messages in thread
From: Nicolai Stange @ 2017-10-30 23:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Paul E.McKenney, Tyler Hall, Mike Marciniszyn,
Dennis Dalessandro, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel, Nicolai Stange
The current implementation of debugfs_real_fops() relies on a
debugfs_fsdata instance to be installed at ->d_fsdata.
With future patches introducing lazy allocation of these, this requirement
will be guaranteed to be fullfilled only inbetween a
debugfs_file_get()/debugfs_file_put() pair.
The full proxies' fops implemented by debugfs happen to be the only
offenders. Fix them up by moving their debugfs_real_fops() calls past those
to debugfs_file_get().
full_proxy_release() is special as it doesn't invoke debugfs_file_get() at
all. Leave it alone for now.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
fs/debugfs/file.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 53f5c9a2af88..bc3549c95574 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -144,13 +144,13 @@ const struct file_operations debugfs_open_proxy_file_operations = {
static ret_type full_proxy_ ## name(proto) \
{ \
struct dentry *dentry = F_DENTRY(filp); \
- const struct file_operations *real_fops = \
- debugfs_real_fops(filp); \
+ const struct file_operations *real_fops; \
ret_type r; \
\
r = debugfs_file_get(dentry); \
if (unlikely(r)) \
return r; \
+ real_fops = debugfs_real_fops(filp); \
r = real_fops->name(args); \
debugfs_file_put(dentry); \
return r; \
@@ -177,13 +177,14 @@ FULL_PROXY_FUNC(unlocked_ioctl, long, filp,
static unsigned int full_proxy_poll(struct file *filp,
struct poll_table_struct *wait)
{
- const struct file_operations *real_fops = debugfs_real_fops(filp);
struct dentry *dentry = F_DENTRY(filp);
unsigned int r = 0;
+ const struct file_operations *real_fops;
if (debugfs_file_get(dentry))
return POLLHUP;
+ real_fops = debugfs_real_fops(filp);
r = real_fops->poll(filp, wait);
debugfs_file_put(dentry);
return r;
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 8/8] debugfs: defer debugfs_fsdata allocation to first usage
2017-10-30 23:15 ` [PATCH v3 0/8] debugfs: per-file removal protection Nicolai Stange
` (6 preceding siblings ...)
2017-10-30 23:15 ` [PATCH v3 7/8] debugfs: call debugfs_real_fops() only after debugfs_file_get() Nicolai Stange
@ 2017-10-30 23:15 ` Nicolai Stange
7 siblings, 0 replies; 10+ messages in thread
From: Nicolai Stange @ 2017-10-30 23:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Paul E.McKenney, Tyler Hall, Mike Marciniszyn,
Dennis Dalessandro, Doug Ledford, Sean Hefty, Hal Rosenstock,
linux-rdma, linux-kernel, Nicolai Stange
Currently, __debugfs_create_file allocates one struct debugfs_fsdata
instance for every file created. However, there are potentially many
debugfs file around, most of which are never touched by userspace.
Thus, defer the allocations to the first usage, i.e. to the first
debugfs_file_get().
A dentry's ->d_fsdata starts out to point to the "real", user provided
fops. After a debugfs_fsdata instance has been allocated (and the real
fops pointer has been moved over into its ->real_fops member),
->d_fsdata is changed to point to it from then on. The two cases are
distinguished by setting BIT(0) for the real fops case.
struct debugfs_fsdata's foremost purpose is to track active users and to
make debugfs_remove() block until they are done. Since no debugfs_fsdata
instance means no active users, make debugfs_remove() return immediately
in this case.
Take care of possible races between debugfs_file_get() and
debugfs_remove(): either debugfs_remove() must see a debugfs_fsdata
instance and thus wait for possible active users or debugfs_file_get() must
see a dead dentry and return immediately.
Make a dentry's ->d_release(), i.e. debugfs_release_dentry(), check whether
->d_fsdata is actually a debugfs_fsdata instance before kfree()ing it.
Similarly, make debugfs_real_fops() check whether ->d_fsdata is actually
a debugfs_fsdata instance before returning it, otherwise emit a warning.
The set of possible error codes returned from debugfs_file_get() has grown
from -EIO to -EIO and -ENOMEM. Make open_proxy_open() and full_proxy_open()
pass the -ENOMEM onwards to their callers.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
fs/debugfs/file.c | 55 ++++++++++++++++++++++++++++++++++++++++++---------
fs/debugfs/inode.c | 36 +++++++++++++++++----------------
fs/debugfs/internal.h | 8 ++++++++
3 files changed, 73 insertions(+), 26 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index bc3549c95574..65872340e301 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -53,6 +53,15 @@ const struct file_operations *debugfs_real_fops(const struct file *filp)
{
struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata;
+ if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) {
+ /*
+ * Urgh, we've been called w/o a protecting
+ * debugfs_file_get().
+ */
+ WARN_ON(1);
+ return NULL;
+ }
+
return fsd->real_fops;
}
EXPORT_SYMBOL_GPL(debugfs_real_fops);
@@ -74,9 +83,35 @@ EXPORT_SYMBOL_GPL(debugfs_real_fops);
*/
int debugfs_file_get(struct dentry *dentry)
{
- struct debugfs_fsdata *fsd = dentry->d_fsdata;
+ struct debugfs_fsdata *fsd;
+ void *d_fsd;
+
+ d_fsd = READ_ONCE(dentry->d_fsdata);
+ if (!((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) {
+ fsd = d_fsd;
+ } else {
+ fsd = kmalloc(sizeof(*fsd), GFP_KERNEL);
+ if (!fsd)
+ return -ENOMEM;
+
+ fsd->real_fops = (void *)((unsigned long)d_fsd &
+ ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
+ refcount_set(&fsd->active_users, 1);
+ init_completion(&fsd->active_users_drained);
+ if (cmpxchg(&dentry->d_fsdata, d_fsd, fsd) != d_fsd) {
+ kfree(fsd);
+ fsd = READ_ONCE(dentry->d_fsdata);
+ }
+ }
- /* Avoid starvation of removers. */
+ /*
+ * In case of a successful cmpxchg() above, this check is
+ * strictly necessary and must follow it, see the comment in
+ * __debugfs_remove_file().
+ * OTOH, if the cmpxchg() hasn't been executed or wasn't
+ * successful, this serves the purpose of not starving
+ * removers.
+ */
if (d_unlinked(dentry))
return -EIO;
@@ -98,7 +133,7 @@ EXPORT_SYMBOL_GPL(debugfs_file_get);
*/
void debugfs_file_put(struct dentry *dentry)
{
- struct debugfs_fsdata *fsd = dentry->d_fsdata;
+ struct debugfs_fsdata *fsd = READ_ONCE(dentry->d_fsdata);
if (refcount_dec_and_test(&fsd->active_users))
complete(&fsd->active_users_drained);
@@ -109,10 +144,11 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
{
struct dentry *dentry = F_DENTRY(filp);
const struct file_operations *real_fops = NULL;
- int r = 0;
+ int r;
- if (debugfs_file_get(dentry))
- return -ENOENT;
+ r = debugfs_file_get(dentry);
+ if (r)
+ return r == -EIO ? -ENOENT : r;
real_fops = debugfs_real_fops(filp);
real_fops = fops_get(real_fops);
@@ -233,10 +269,11 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
struct dentry *dentry = F_DENTRY(filp);
const struct file_operations *real_fops = NULL;
struct file_operations *proxy_fops = NULL;
- int r = 0;
+ int r;
- if (debugfs_file_get(dentry))
- return -ENOENT;
+ r = debugfs_file_get(dentry);
+ if (r)
+ return r == -EIO ? -ENOENT : r;
real_fops = debugfs_real_fops(filp);
real_fops = fops_get(real_fops);
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index f587aded46b5..9dca4da059b3 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -184,7 +184,10 @@ static const struct super_operations debugfs_super_operations = {
static void debugfs_release_dentry(struct dentry *dentry)
{
- kfree(dentry->d_fsdata);
+ void *fsd = dentry->d_fsdata;
+
+ if (!((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))
+ kfree(dentry->d_fsdata);
}
static struct vfsmount *debugfs_automount(struct path *path)
@@ -344,35 +347,25 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
{
struct dentry *dentry;
struct inode *inode;
- struct debugfs_fsdata *fsd;
-
- fsd = kmalloc(sizeof(*fsd), GFP_KERNEL);
- if (!fsd)
- return NULL;
if (!(mode & S_IFMT))
mode |= S_IFREG;
BUG_ON(!S_ISREG(mode));
dentry = start_creating(name, parent);
- if (IS_ERR(dentry)) {
- kfree(fsd);
+ if (IS_ERR(dentry))
return NULL;
- }
inode = debugfs_get_inode(dentry->d_sb);
- if (unlikely(!inode)) {
- kfree(fsd);
+ if (unlikely(!inode))
return failed_creating(dentry);
- }
inode->i_mode = mode;
inode->i_private = data;
inode->i_fop = proxy_fops;
- fsd->real_fops = real_fops;
- refcount_set(&fsd->active_users, 1);
- dentry->d_fsdata = fsd;
+ dentry->d_fsdata = (void *)((unsigned long)real_fops |
+ DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
d_instantiate(dentry, inode);
fsnotify_create(d_inode(dentry->d_parent), dentry);
@@ -635,8 +628,17 @@ static void __debugfs_remove_file(struct dentry *dentry, struct dentry *parent)
simple_unlink(d_inode(parent), dentry);
d_delete(dentry);
- fsd = dentry->d_fsdata;
- init_completion(&fsd->active_users_drained);
+
+ /*
+ * Paired with the closing smp_mb() implied by a successful
+ * cmpxchg() in debugfs_file_get(): either
+ * debugfs_file_get() must see a dead dentry or we must see a
+ * debugfs_fsdata instance at ->d_fsdata here (or both).
+ */
+ smp_mb();
+ fsd = READ_ONCE(dentry->d_fsdata);
+ if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
+ return;
if (!refcount_dec_and_test(&fsd->active_users))
wait_for_completion(&fsd->active_users_drained);
}
diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h
index 0eea99432840..cb1e8139c398 100644
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -25,4 +25,12 @@ struct debugfs_fsdata {
struct completion active_users_drained;
};
+/*
+ * A dentry's ->d_fsdata either points to the real fops or to a
+ * dynamically allocated debugfs_fsdata instance.
+ * In order to distinguish between these two cases, a real fops
+ * pointer gets its lowest bit set.
+ */
+#define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
+
#endif /* _DEBUGFS_INTERNAL_H_ */
--
2.13.6
^ permalink raw reply related [flat|nested] 10+ messages in thread