From: Nicolai Stange <nstange@suse.de>
Patch-mainline: Never, kABI
Subject: kABI: restore ->d_fsdata semantics for debugfs_real_fops()
References: bsc#1159198 bsc#1109911
Upstream commit 7c8d469877b1 ("debugfs: add support for more elaborate
->d_fsdata"), imported as
patches.suse/debugfs-add-support-for-more-elaborate-d_fsdata.patch,
changed the semantics of S_IFREG debugfs files' ->d_fsdata, which breaks
kABI. Before that change, ->d_fsdata used to store a pointer to the
original, user provided file_operations and the public, inline
debugfs_real_fops() would merely retrieve this pointer value.
After the aforementioned patch, ->d_fsdata was made to point to an
intermediate struct debugfs_fsdata instance, which contains, among others,
a member for referring to said file_operations. (Actully, with later
upstream commit 7d39bc50c47b ("debugfs: defer debugfs_fsdata allocation to
first usage"), things became a bit more complicated: ->d_fsdata would
either point to the file_operations directly or to such a struct
debugfs_fsdata instance wrapping it. The two cases are disambiguated by
by testing ->d_fsdata lsb).
To retain kABI, restore the old semantics of ->d_fsdata and make
__debugfs_create_file() initialize it with a pointer to the user provided
file_operations again.
The implication is that some other means of associating a pointers of the
the new semantics with S_IFREG debugfs files is needed. Abuse the
associated struct inode's ->i_link member for that. This works, because
- with debugfs, the association dentry <-> inode is always 1:1,
- for debugfs S_IFREG files, ->i_link as well as the other members from
the enclosing union are unused and
- the inode_operations installed for S_IFREG files,
debugfs_file_inode_operations, has its ->get_link() unset and thus, the
VFS layer won't ever look at the inode's ->i_link.
Introduce helper macros kabi_debugfs_d_fsdata(dentry) and
__kabi_debugfs_d_fsdata(inode) which both provide access to the associated
inode's ->i_link rather than the dentry's ->d_fsdata. Replace all direct
accesses to ->d_fsdata from the debugfs core by invocations of these as
appropriate.
Finally move the cleanup of the debugfs_fsdata instance, if any, from
debugfs_release_dentry() to debugfs_free_inode(): when the VFS invokes
the former, the dentry isn't linked to the inode anymore.
Signed-off-by: Nicolai Stange <nstange@suse.de>
---
fs/debugfs/file.c | 10 +++++-----
fs/debugfs/inode.c | 25 ++++++++++++++++++-------
fs/debugfs/internal.h | 19 +++++++++++++++++++
3 files changed, 42 insertions(+), 12 deletions(-)
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -51,7 +51,7 @@ const struct file_operations debugfs_noo
const struct file_operations *debugfs_real_fops(const struct file *filp)
{
- struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata;
+ struct debugfs_fsdata *fsd = kabi_debugfs_d_fsdata(F_DENTRY(filp));
if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) {
/*
@@ -86,7 +86,7 @@ int debugfs_file_get(struct dentry *dent
struct debugfs_fsdata *fsd;
void *d_fsd;
- d_fsd = READ_ONCE(dentry->d_fsdata);
+ d_fsd = READ_ONCE(kabi_debugfs_d_fsdata(dentry));
if (!((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) {
fsd = d_fsd;
} else {
@@ -98,9 +98,9 @@ int debugfs_file_get(struct dentry *dent
~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) {
+ if (cmpxchg(&kabi_debugfs_d_fsdata(dentry), d_fsd, fsd) != d_fsd) {
kfree(fsd);
- fsd = READ_ONCE(dentry->d_fsdata);
+ fsd = READ_ONCE(kabi_debugfs_d_fsdata(dentry));
}
}
@@ -133,7 +133,7 @@ EXPORT_SYMBOL_GPL(debugfs_file_get);
*/
void debugfs_file_put(struct dentry *dentry)
{
- struct debugfs_fsdata *fsd = READ_ONCE(dentry->d_fsdata);
+ struct debugfs_fsdata *fsd = READ_ONCE(kabi_debugfs_d_fsdata(dentry));
if (refcount_dec_and_test(&fsd->active_users))
complete(&fsd->active_users_drained);
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -170,8 +170,17 @@ static int debugfs_show_options(struct s
static void debugfs_i_callback(struct rcu_head *head)
{
struct inode *inode = container_of(head, struct inode, i_rcu);
- if (S_ISLNK(inode->i_mode))
+ if (S_ISLNK(inode->i_mode)) {
kfree(inode->i_link);
+ } else if (S_ISREG(inode->i_mode)) {
+ /*
+ * kABI workaround uses ->i_link in place of ->d->fs_data
+ * for regular files.
+ */
+ void *fsd = __kabi_debugfs_d_fsdata(inode);
+ if (!((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))
+ kfree(fsd);
+ }
free_inode_nonrcu(inode);
}
@@ -189,10 +198,11 @@ static const struct super_operations deb
static void debugfs_release_dentry(struct dentry *dentry)
{
- void *fsd = dentry->d_fsdata;
-
- if (!((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))
- kfree(dentry->d_fsdata);
+ /*
+ * Upstream does a kfree() on ->d_fsdata here. However,
+ * for working around kABI, ->i_link is used instead.
+ * The corresponding cleanup is found in debugfs_i_callback().
+ */
}
static struct vfsmount *debugfs_automount(struct path *path)
@@ -370,7 +380,8 @@ static struct dentry *__debugfs_create_f
inode->i_private = data;
inode->i_fop = proxy_fops;
- dentry->d_fsdata = (void *)((unsigned long)real_fops |
+ dentry->d_fsdata = (void *)real_fops; /* Needed for kABI */
+ __kabi_debugfs_d_fsdata(inode) = (void *)((unsigned long)real_fops |
DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
d_instantiate(dentry, inode);
@@ -639,7 +650,7 @@ static void __debugfs_file_removed(struc
* debugfs_fsdata instance at ->d_fsdata here (or both).
*/
smp_mb();
- fsd = READ_ONCE(dentry->d_fsdata);
+ fsd = READ_ONCE(kabi_debugfs_d_fsdata(dentry));
if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
return;
if (!refcount_dec_and_test(&fsd->active_users))
--- a/fs/debugfs/internal.h
+++ b/fs/debugfs/internal.h
@@ -33,4 +33,23 @@ struct debugfs_fsdata {
*/
#define DEBUGFS_FSDATA_IS_REAL_FOPS_BIT BIT(0)
+/*
+ * The file removal protection series, upstream commit
+ * 7c8d469877b1 ("debugfs: add support for more elaborate ->d_fsdata")
+ * in particular, changed the semantics of S_IFREG files' dentry->d_fsdata.
+ * Before that change, the original, a pointer to the user-provided
+ * file_operations had been stored there and external modules might depend on
+ * this. Especially as debugfs_real_fops() used to be defined as an inline
+ * function in public include/linux/debugfs.h.
+ *
+ * Work around this by abusing the associated inode's ->i_link instead, which
+ * is unused for regular debugfs files. The dereference + cast dance below
+ * is to make the result an lvalue of type void *.
+ */
+#define __kabi_debugfs_d_fsdata(inode) \
+ (*(void **)&(inode)->i_link)
+
+#define kabi_debugfs_d_fsdata(dentry) \
+ __kabi_debugfs_d_fsdata(d_inode(dentry))
+
#endif /* _DEBUGFS_INTERNAL_H_ */