From f8f25893a477a4da4414c3e40ddd51d77fac9cfc Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 29 Nov 2024 13:15:37 +0100 Subject: fs: debugfs: differentiate short fops with proxy ops Geert reported that my previous short fops debugfs changes broke m68k, because it only has mandatory alignement of two, so we can't stash the "is it short" information into the pointer (as we already did with the "is it real" bit.) Instead, exploit the fact that debugfs_file_get() called on an already open file will already find that the fsdata is no longer the real fops but rather the allocated data that already distinguishes full/short ops, so only open() needs to be able to distinguish. We can achieve that by using two different open functions. Unfortunately this requires another set of full file ops, increasing the size by 536 bytes (x86-64), but that's still a reasonable trade-off given that only converting some of the wireless stack gained over 28k. This brings the total cost of this to around 1k, for wins of 28k (all x86-64). Reported-and-tested-by: Geert Uytterhoeven Link: https://lore.kernel.org/CAMuHMdWu_9-L2Te101w8hU7H_2yobJFPXSwwUmGHSJfaPWDKiQ@mail.gmail.com Fixes: 8dc6d81c6b2a ("debugfs: add small file operations for most files") Signed-off-by: Johannes Berg Tested-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/20241129121536.30989-2-johannes@sipsolutions.net Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 72 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 22 deletions(-) (limited to 'fs/debugfs/file.c') diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 47dc96dfe386..bdb4f2ca0506 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -64,22 +64,13 @@ 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) +enum dbgfs_get_mode { + DBGFS_GET_ALREADY, + DBGFS_GET_REGULAR, + DBGFS_GET_SHORT, +}; + +static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode) { struct debugfs_fsdata *fsd; void *d_fsd; @@ -96,15 +87,17 @@ int debugfs_file_get(struct dentry *dentry) if (!((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) { fsd = d_fsd; } else { + if (WARN_ON(mode == DBGFS_GET_ALREADY)) + return -EINVAL; + fsd = kmalloc(sizeof(*fsd), GFP_KERNEL); if (!fsd) return -ENOMEM; - if ((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT) { + if (mode == DBGFS_GET_SHORT) { fsd->real_fops = NULL; fsd->short_fops = (void *)((unsigned long)d_fsd & - ~(DEBUGFS_FSDATA_IS_REAL_FOPS_BIT | - DEBUGFS_FSDATA_IS_SHORT_FOPS_BIT)); + ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); } else { fsd->real_fops = (void *)((unsigned long)d_fsd & ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); @@ -138,6 +131,26 @@ int debugfs_file_get(struct dentry *dentry) return 0; } + +/** + * 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) +{ + return __debugfs_file_get(dentry, DBGFS_GET_ALREADY); +} EXPORT_SYMBOL_GPL(debugfs_file_get); /** @@ -424,7 +437,8 @@ static void __full_proxy_fops_init(struct file_operations *proxy_fops, proxy_fops->unlocked_ioctl = full_proxy_unlocked_ioctl; } -static int full_proxy_open(struct inode *inode, struct file *filp) +static int full_proxy_open(struct inode *inode, struct file *filp, + enum dbgfs_get_mode mode) { struct dentry *dentry = F_DENTRY(filp); const struct file_operations *real_fops; @@ -432,7 +446,7 @@ static int full_proxy_open(struct inode *inode, struct file *filp) struct debugfs_fsdata *fsd; int r; - r = debugfs_file_get(dentry); + r = __debugfs_file_get(dentry, mode); if (r) return r == -EIO ? -ENOENT : r; @@ -491,8 +505,22 @@ out: return r; } +static int full_proxy_open_regular(struct inode *inode, struct file *filp) +{ + return full_proxy_open(inode, filp, DBGFS_GET_REGULAR); +} + const struct file_operations debugfs_full_proxy_file_operations = { - .open = full_proxy_open, + .open = full_proxy_open_regular, +}; + +static int full_proxy_open_short(struct inode *inode, struct file *filp) +{ + return full_proxy_open(inode, filp, DBGFS_GET_SHORT); +} + +const struct file_operations debugfs_full_short_proxy_file_operations = { + .open = full_proxy_open_short, }; ssize_t debugfs_attr_read(struct file *file, char __user *buf, -- cgit v1.2.3 From 67510d7e2e5f5bdc020bf9d759aa575cce48c8e1 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Fri, 10 Jan 2025 08:58:14 +0100 Subject: fs: debugfs: fix open proxy for unsafe files In the previous commit referenced below, I had to split the short fops handling into different proxy fops. This necessitated knowing out-of-band whether or not the ops are short or full, when attempting to convert from fops to allocated fsdata. Unfortunately, I only converted full_proxy_open() which is used for the new full_proxy_open_regular() and full_proxy_open_short(), but forgot about the call in open_proxy_open(), used for debugfs_create_file_unsafe(). Fix that, it never has short fops. Fixes: f8f25893a477 ("fs: debugfs: differentiate short fops with proxy ops") Reported-by: Suresh Kumar Kurmi Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-lkp/202501101055.bb8bf3e7-lkp@intel.com Reported-by: Venkat Rao Bagalkote Signed-off-by: Johannes Berg Link: https://lore.kernel.org/r/20250110085826.cd74f3b7a36b.I430c79c82ec3f954c2ff9665753bf6ac9e63eef8@changeid Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/debugfs/file.c') diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index bdb4f2ca0506..16e198a26339 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -280,7 +280,7 @@ static int open_proxy_open(struct inode *inode, struct file *filp) const struct file_operations *real_fops = NULL; int r; - r = debugfs_file_get(dentry); + r = __debugfs_file_get(dentry, DBGFS_GET_REGULAR); if (r) return r == -EIO ? -ENOENT : r; -- cgit v1.2.3 From 41a0ecc0997cd40d913cce18867efd1c34c64e28 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 12 Jan 2025 08:06:47 +0000 Subject: debugfs: get rid of dynamically allocation proxy_ops All it takes is having full_proxy_open() collect the information about available methods and store it in debugfs_fsdata. Wrappers are called only after full_proxy_open() has succeeded calling debugfs_get_file(), so they are guaranteed to have ->d_fsdata already pointing to debugfs_fsdata. As the result, they can check if method is absent and bugger off early, without any atomic operations, etc. - same effect as what we'd have from NULL method. Which makes the entire proxy_fops contents unconditional, making it completely pointless - we can just put those methods (unconditionally) into debugfs_full_proxy_file_operations and forget about dynamic allocation, replace_fops, etc. Signed-off-by: Al Viro Reviewed-by: Christian Brauner Link: https://lore.kernel.org/r/20250112080705.141166-3-viro@zeniv.linux.org.uk Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 113 +++++++++++++++++++++++--------------------------- fs/debugfs/internal.h | 9 ++++ 2 files changed, 61 insertions(+), 61 deletions(-) (limited to 'fs/debugfs/file.c') diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 16e198a26339..eb59b01f5f25 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -95,13 +95,31 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode) return -ENOMEM; if (mode == DBGFS_GET_SHORT) { - fsd->real_fops = NULL; - fsd->short_fops = (void *)((unsigned long)d_fsd & - ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); + const struct debugfs_short_fops *ops; + ops = (void *)((unsigned long)d_fsd & + ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); + fsd->short_fops = ops; + if (ops->llseek) + fsd->methods |= HAS_LSEEK; + if (ops->read) + fsd->methods |= HAS_READ; + if (ops->write) + fsd->methods |= HAS_WRITE; } else { - fsd->real_fops = (void *)((unsigned long)d_fsd & - ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); - fsd->short_fops = NULL; + const struct file_operations *ops; + ops = (void *)((unsigned long)d_fsd & + ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); + fsd->real_fops = ops; + if (ops->llseek) + fsd->methods |= HAS_LSEEK; + if (ops->read) + fsd->methods |= HAS_READ; + if (ops->write) + fsd->methods |= HAS_WRITE; + if (ops->unlocked_ioctl) + fsd->methods |= HAS_IOCTL; + if (ops->poll) + fsd->methods |= HAS_POLL; } refcount_set(&fsd->active_users, 1); init_completion(&fsd->active_users_drained); @@ -322,13 +340,16 @@ const struct file_operations debugfs_open_proxy_file_operations = { #define PROTO(args...) args #define ARGS(args...) args -#define FULL_PROXY_FUNC(name, ret_type, filp, proto, args) \ +#define FULL_PROXY_FUNC(name, ret_type, filp, proto, args, bit, ret) \ static ret_type full_proxy_ ## name(proto) \ { \ - struct dentry *dentry = F_DENTRY(filp); \ + struct dentry *dentry = F_DENTRY(filp); \ + struct debugfs_fsdata *fsd = dentry->d_fsdata; \ const struct file_operations *real_fops; \ ret_type r; \ \ + if (!(fsd->methods & bit)) \ + return ret; \ r = debugfs_file_get(dentry); \ if (unlikely(r)) \ return r; \ @@ -338,17 +359,18 @@ static ret_type full_proxy_ ## name(proto) \ return r; \ } -#define FULL_PROXY_FUNC_BOTH(name, ret_type, filp, proto, args) \ +#define FULL_PROXY_FUNC_BOTH(name, ret_type, filp, proto, args, bit, ret) \ static ret_type full_proxy_ ## name(proto) \ { \ struct dentry *dentry = F_DENTRY(filp); \ - struct debugfs_fsdata *fsd; \ + struct debugfs_fsdata *fsd = dentry->d_fsdata; \ ret_type r; \ \ + if (!(fsd->methods & bit)) \ + return ret; \ r = debugfs_file_get(dentry); \ if (unlikely(r)) \ return r; \ - fsd = dentry->d_fsdata; \ if (fsd->real_fops) \ r = fsd->real_fops->name(args); \ else \ @@ -359,29 +381,32 @@ static ret_type full_proxy_ ## name(proto) \ FULL_PROXY_FUNC_BOTH(llseek, loff_t, filp, PROTO(struct file *filp, loff_t offset, int whence), - ARGS(filp, offset, whence)); + ARGS(filp, offset, whence), HAS_LSEEK, -ESPIPE); FULL_PROXY_FUNC_BOTH(read, ssize_t, filp, PROTO(struct file *filp, char __user *buf, size_t size, loff_t *ppos), - ARGS(filp, buf, size, ppos)); + ARGS(filp, buf, size, ppos), HAS_READ, -EINVAL); FULL_PROXY_FUNC_BOTH(write, ssize_t, filp, PROTO(struct file *filp, const char __user *buf, size_t size, loff_t *ppos), - ARGS(filp, buf, size, ppos)); + ARGS(filp, buf, size, ppos), HAS_WRITE, -EINVAL); FULL_PROXY_FUNC(unlocked_ioctl, long, filp, PROTO(struct file *filp, unsigned int cmd, unsigned long arg), - ARGS(filp, cmd, arg)); + ARGS(filp, cmd, arg), HAS_IOCTL, -ENOTTY); static __poll_t full_proxy_poll(struct file *filp, struct poll_table_struct *wait) { struct dentry *dentry = F_DENTRY(filp); + struct debugfs_fsdata *fsd = dentry->d_fsdata; __poll_t r = 0; const struct file_operations *real_fops; + if (!(fsd->methods & HAS_POLL)) + return DEFAULT_POLLMASK; if (debugfs_file_get(dentry)) return EPOLLHUP; @@ -393,9 +418,7 @@ static __poll_t full_proxy_poll(struct file *filp, static int full_proxy_release(struct inode *inode, struct file *filp) { - const struct dentry *dentry = F_DENTRY(filp); const struct file_operations *real_fops = debugfs_real_fops(filp); - const struct file_operations *proxy_fops = filp->f_op; int r = 0; /* @@ -407,42 +430,15 @@ static int full_proxy_release(struct inode *inode, struct file *filp) if (real_fops && real_fops->release) r = real_fops->release(inode, filp); - replace_fops(filp, d_inode(dentry)->i_fop); - kfree(proxy_fops); fops_put(real_fops); return r; } -static void __full_proxy_fops_init(struct file_operations *proxy_fops, - struct debugfs_fsdata *fsd) -{ - proxy_fops->release = full_proxy_release; - - if ((fsd->real_fops && fsd->real_fops->llseek) || - (fsd->short_fops && fsd->short_fops->llseek)) - proxy_fops->llseek = full_proxy_llseek; - - if ((fsd->real_fops && fsd->real_fops->read) || - (fsd->short_fops && fsd->short_fops->read)) - proxy_fops->read = full_proxy_read; - - if ((fsd->real_fops && fsd->real_fops->write) || - (fsd->short_fops && fsd->short_fops->write)) - proxy_fops->write = full_proxy_write; - - if (fsd->real_fops && fsd->real_fops->poll) - proxy_fops->poll = full_proxy_poll; - - if (fsd->real_fops && fsd->real_fops->unlocked_ioctl) - proxy_fops->unlocked_ioctl = full_proxy_unlocked_ioctl; -} - static int full_proxy_open(struct inode *inode, struct file *filp, enum dbgfs_get_mode mode) { struct dentry *dentry = F_DENTRY(filp); const struct file_operations *real_fops; - struct file_operations *proxy_fops = NULL; struct debugfs_fsdata *fsd; int r; @@ -472,34 +468,20 @@ static int full_proxy_open(struct inode *inode, struct file *filp, goto out; } - proxy_fops = kzalloc(sizeof(*proxy_fops), GFP_KERNEL); - if (!proxy_fops) { - r = -ENOMEM; - goto free_proxy; - } - __full_proxy_fops_init(proxy_fops, fsd); - replace_fops(filp, proxy_fops); - if (!real_fops || real_fops->open) { if (real_fops) r = real_fops->open(inode, filp); else r = simple_open(inode, filp); if (r) { - replace_fops(filp, d_inode(dentry)->i_fop); - goto free_proxy; - } else if (filp->f_op != proxy_fops) { + fops_put(real_fops); + } else if (filp->f_op != &debugfs_full_proxy_file_operations) { /* No protection against file removal anymore. */ WARN(1, "debugfs file owner replaced proxy fops: %pd", dentry); - goto free_proxy; + fops_put(real_fops); } } - - goto out; -free_proxy: - kfree(proxy_fops); - fops_put(real_fops); out: debugfs_file_put(dentry); return r; @@ -512,6 +494,12 @@ static int full_proxy_open_regular(struct inode *inode, struct file *filp) const struct file_operations debugfs_full_proxy_file_operations = { .open = full_proxy_open_regular, + .release = full_proxy_release, + .llseek = full_proxy_llseek, + .read = full_proxy_read, + .write = full_proxy_write, + .poll = full_proxy_poll, + .unlocked_ioctl = full_proxy_unlocked_ioctl }; static int full_proxy_open_short(struct inode *inode, struct file *filp) @@ -521,6 +509,9 @@ static int full_proxy_open_short(struct inode *inode, struct file *filp) const struct file_operations debugfs_full_short_proxy_file_operations = { .open = full_proxy_open_short, + .llseek = full_proxy_llseek, + .read = full_proxy_read, + .write = full_proxy_write, }; ssize_t debugfs_attr_read(struct file *file, char __user *buf, diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index a644e44a0ee4..011ef8b1a99a 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -39,9 +39,18 @@ struct debugfs_fsdata { /* protect cancellations */ struct mutex cancellations_mtx; struct list_head cancellations; + unsigned int methods; }; }; +enum { + HAS_READ = 1, + HAS_WRITE = 2, + HAS_LSEEK = 4, + HAS_POLL = 8, + HAS_IOCTL = 16 +}; + /* * A dentry's ->d_fsdata either points to the real fops or to a * dynamically allocated debugfs_fsdata instance. -- cgit v1.2.3 From 95688800eefe28240204c2a0dd2bca5bf5f7f1d9 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 12 Jan 2025 08:06:48 +0000 Subject: debugfs: don't mess with bits in ->d_fsdata The reason we need that crap is the dual use ->d_fsdata has there - it's both holding a debugfs_fsdata reference after the first debugfs_file_get() (actually, after the call of proxy ->open()) *and* it serves as a place to stash a reference to real file_operations from object creation to the first open. Oh, and it's triple use, actually - that stashed reference might be to debugfs_short_fops. Bugger that for a game of solidiers - just put the operations reference into debugfs-private augmentation of inode. And split debugfs_full_file_operations into full and short cases, so that debugfs_get_file() could tell one from another. Voila - ->d_fsdata holds NULL until the first (successful) debugfs_get_file() and a reference to struct debugfs_fsdata afterwards. Signed-off-by: Al Viro Reviewed-by: Christian Brauner Link: https://lore.kernel.org/r/20250112080705.141166-4-viro@zeniv.linux.org.uk Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 58 +++++++++++++++++++++++++-------------------------- fs/debugfs/inode.c | 29 +++++++------------------- fs/debugfs/internal.h | 12 +++-------- 3 files changed, 38 insertions(+), 61 deletions(-) (limited to 'fs/debugfs/file.c') diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index eb59b01f5f25..ae014bd36a6f 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -51,7 +51,7 @@ 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) { + if (!fsd) { /* * Urgh, we've been called w/o a protecting * debugfs_file_get(). @@ -84,9 +84,11 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode) return -EINVAL; d_fsd = READ_ONCE(dentry->d_fsdata); - if (!((unsigned long)d_fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) { + if (d_fsd) { fsd = d_fsd; } else { + struct inode *inode = dentry->d_inode; + if (WARN_ON(mode == DBGFS_GET_ALREADY)) return -EINVAL; @@ -96,9 +98,7 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode) if (mode == DBGFS_GET_SHORT) { const struct debugfs_short_fops *ops; - ops = (void *)((unsigned long)d_fsd & - ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); - fsd->short_fops = ops; + ops = fsd->short_fops = DEBUGFS_I(inode)->short_fops; if (ops->llseek) fsd->methods |= HAS_LSEEK; if (ops->read) @@ -107,9 +107,7 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode) fsd->methods |= HAS_WRITE; } else { const struct file_operations *ops; - ops = (void *)((unsigned long)d_fsd & - ~DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); - fsd->real_fops = ops; + ops = fsd->real_fops = DEBUGFS_I(inode)->real_fops; if (ops->llseek) fsd->methods |= HAS_LSEEK; if (ops->read) @@ -126,10 +124,11 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode) INIT_LIST_HEAD(&fsd->cancellations); mutex_init(&fsd->cancellations_mtx); - if (cmpxchg(&dentry->d_fsdata, d_fsd, fsd) != d_fsd) { + d_fsd = cmpxchg(&dentry->d_fsdata, NULL, fsd); + if (d_fsd) { mutex_destroy(&fsd->cancellations_mtx); kfree(fsd); - fsd = READ_ONCE(dentry->d_fsdata); + fsd = d_fsd; } } @@ -226,8 +225,7 @@ void debugfs_enter_cancellation(struct file *file, return; fsd = READ_ONCE(dentry->d_fsdata); - if (WARN_ON(!fsd || - ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))) + if (WARN_ON(!fsd)) return; mutex_lock(&fsd->cancellations_mtx); @@ -258,8 +256,7 @@ void debugfs_leave_cancellation(struct file *file, return; fsd = READ_ONCE(dentry->d_fsdata); - if (WARN_ON(!fsd || - ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT))) + if (WARN_ON(!fsd)) return; mutex_lock(&fsd->cancellations_mtx); @@ -427,22 +424,21 @@ static int full_proxy_release(struct inode *inode, struct file *filp) * not to leak any resources. Releasers must not assume that * ->i_private is still being meaningful here. */ - if (real_fops && real_fops->release) + if (real_fops->release) r = real_fops->release(inode, filp); fops_put(real_fops); return r; } -static int full_proxy_open(struct inode *inode, struct file *filp, - enum dbgfs_get_mode mode) +static int full_proxy_open_regular(struct inode *inode, struct file *filp) { struct dentry *dentry = F_DENTRY(filp); const struct file_operations *real_fops; struct debugfs_fsdata *fsd; int r; - r = __debugfs_file_get(dentry, mode); + r = __debugfs_file_get(dentry, DBGFS_GET_REGULAR); if (r) return r == -EIO ? -ENOENT : r; @@ -452,7 +448,7 @@ static int full_proxy_open(struct inode *inode, struct file *filp, if (r) goto out; - if (real_fops && !fops_get(real_fops)) { + if (!fops_get(real_fops)) { #ifdef CONFIG_MODULES if (real_fops->owner && real_fops->owner->state == MODULE_STATE_GOING) { @@ -468,11 +464,8 @@ static int full_proxy_open(struct inode *inode, struct file *filp, goto out; } - if (!real_fops || real_fops->open) { - if (real_fops) - r = real_fops->open(inode, filp); - else - r = simple_open(inode, filp); + if (real_fops->open) { + r = real_fops->open(inode, filp); if (r) { fops_put(real_fops); } else if (filp->f_op != &debugfs_full_proxy_file_operations) { @@ -487,11 +480,6 @@ out: return r; } -static int full_proxy_open_regular(struct inode *inode, struct file *filp) -{ - return full_proxy_open(inode, filp, DBGFS_GET_REGULAR); -} - const struct file_operations debugfs_full_proxy_file_operations = { .open = full_proxy_open_regular, .release = full_proxy_release, @@ -504,7 +492,17 @@ const struct file_operations debugfs_full_proxy_file_operations = { static int full_proxy_open_short(struct inode *inode, struct file *filp) { - return full_proxy_open(inode, filp, DBGFS_GET_SHORT); + struct dentry *dentry = F_DENTRY(filp); + int r; + + r = __debugfs_file_get(dentry, DBGFS_GET_SHORT); + if (r) + return r == -EIO ? -ENOENT : r; + r = debugfs_locked_down(inode, filp, NULL); + if (!r) + r = simple_open(inode, filp); + debugfs_file_put(dentry); + return r; } const struct file_operations debugfs_full_short_proxy_file_operations = { diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 2f5afd7b1b94..c4e8b7f758e0 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -243,15 +243,10 @@ static void debugfs_release_dentry(struct dentry *dentry) { struct debugfs_fsdata *fsd = dentry->d_fsdata; - if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) - return; - - /* check it wasn't a dir or automount (no fsdata) */ if (fsd) { WARN_ON(!list_empty(&fsd->cancellations)); mutex_destroy(&fsd->cancellations_mtx); } - kfree(fsd); } @@ -459,9 +454,10 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode, inode->i_private = data; inode->i_op = &debugfs_file_inode_operations; + if (!real_fops) + proxy_fops = &debugfs_noop_file_operations; inode->i_fop = proxy_fops; - dentry->d_fsdata = (void *)((unsigned long)real_fops | - DEBUGFS_FSDATA_IS_REAL_FOPS_BIT); + DEBUGFS_I(inode)->raw = real_fops; d_instantiate(dentry, inode); fsnotify_create(d_inode(dentry->d_parent), dentry); @@ -472,13 +468,8 @@ struct dentry *debugfs_create_file_full(const char *name, umode_t mode, struct dentry *parent, void *data, const struct file_operations *fops) { - if (WARN_ON((unsigned long)fops & - DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) - return ERR_PTR(-EINVAL); - return __debugfs_create_file(name, mode, parent, data, - fops ? &debugfs_full_proxy_file_operations : - &debugfs_noop_file_operations, + &debugfs_full_proxy_file_operations, fops); } EXPORT_SYMBOL_GPL(debugfs_create_file_full); @@ -487,13 +478,8 @@ struct dentry *debugfs_create_file_short(const char *name, umode_t mode, struct dentry *parent, void *data, const struct debugfs_short_fops *fops) { - if (WARN_ON((unsigned long)fops & - DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)) - return ERR_PTR(-EINVAL); - return __debugfs_create_file(name, mode, parent, data, - fops ? &debugfs_full_short_proxy_file_operations : - &debugfs_noop_file_operations, + &debugfs_full_short_proxy_file_operations, fops); } EXPORT_SYMBOL_GPL(debugfs_create_file_short); @@ -531,8 +517,7 @@ struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, { return __debugfs_create_file(name, mode, parent, data, - fops ? &debugfs_open_proxy_file_operations : - &debugfs_noop_file_operations, + &debugfs_open_proxy_file_operations, fops); } EXPORT_SYMBOL_GPL(debugfs_create_file_unsafe); @@ -737,7 +722,7 @@ static void __debugfs_file_removed(struct dentry *dentry) */ smp_mb(); fsd = READ_ONCE(dentry->d_fsdata); - if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT) + if (!fsd) return; /* if this was the last reference, we're done */ diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index 011ef8b1a99a..8d2de647b42c 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -14,6 +14,9 @@ struct file_operations; struct debugfs_inode_info { struct inode vfs_inode; union { + const void *raw; + const struct file_operations *real_fops; + const struct debugfs_short_fops *short_fops; debugfs_automount_t automount; }; }; @@ -51,15 +54,6 @@ enum { HAS_IOCTL = 16 }; -/* - * 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) - -/* Access BITS */ #define DEBUGFS_ALLOW_API BIT(0) #define DEBUGFS_ALLOW_MOUNT BIT(1) -- cgit v1.2.3 From 12c92098932b4bbf38396e9aed0a343d35437a21 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 12 Jan 2025 08:06:49 +0000 Subject: debugfs: allow to store an additional opaque pointer at file creation Set by debugfs_create_file_aux(name, mode, parent, data, aux, fops). Plain debugfs_create_file() has it set to NULL. Accessed by debugfs_get_aux(file). Convenience macros for numeric opaque data - debugfs_create_file_aux_num and debugfs_get_aux_num, resp. Signed-off-by: Al Viro Reviewed-by: Christian Brauner Link: https://lore.kernel.org/r/20250112080705.141166-5-viro@zeniv.linux.org.uk Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 6 ++++++ fs/debugfs/inode.c | 14 +++++++++----- fs/debugfs/internal.h | 1 + include/linux/debugfs.h | 27 ++++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 6 deletions(-) (limited to 'fs/debugfs/file.c') diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index ae014bd36a6f..e33cc77699cd 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -47,6 +47,12 @@ const struct file_operations debugfs_noop_file_operations = { #define F_DENTRY(filp) ((filp)->f_path.dentry) +const void *debugfs_get_aux(const struct file *file) +{ + return DEBUGFS_I(file_inode(file))->aux; +} +EXPORT_SYMBOL_GPL(debugfs_get_aux); + const struct file_operations *debugfs_real_fops(const struct file *filp) { struct debugfs_fsdata *fsd = F_DENTRY(filp)->d_fsdata; diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index c4e8b7f758e0..51d4c3e9d422 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -424,6 +424,7 @@ static struct dentry *end_creating(struct dentry *dentry) static struct dentry *__debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, + const void *aux, const struct file_operations *proxy_fops, const void *real_fops) { @@ -458,6 +459,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode, proxy_fops = &debugfs_noop_file_operations; inode->i_fop = proxy_fops; DEBUGFS_I(inode)->raw = real_fops; + DEBUGFS_I(inode)->aux = aux; d_instantiate(dentry, inode); fsnotify_create(d_inode(dentry->d_parent), dentry); @@ -466,19 +468,21 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode, struct dentry *debugfs_create_file_full(const char *name, umode_t mode, struct dentry *parent, void *data, + const void *aux, const struct file_operations *fops) { - return __debugfs_create_file(name, mode, parent, data, + return __debugfs_create_file(name, mode, parent, data, aux, &debugfs_full_proxy_file_operations, fops); } EXPORT_SYMBOL_GPL(debugfs_create_file_full); struct dentry *debugfs_create_file_short(const char *name, umode_t mode, - struct dentry *parent, void *data, - const struct debugfs_short_fops *fops) + struct dentry *parent, void *data, + const void *aux, + const struct debugfs_short_fops *fops) { - return __debugfs_create_file(name, mode, parent, data, + return __debugfs_create_file(name, mode, parent, data, aux, &debugfs_full_short_proxy_file_operations, fops); } @@ -516,7 +520,7 @@ struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, const struct file_operations *fops) { - return __debugfs_create_file(name, mode, parent, data, + return __debugfs_create_file(name, mode, parent, data, NULL, &debugfs_open_proxy_file_operations, fops); } diff --git a/fs/debugfs/internal.h b/fs/debugfs/internal.h index 8d2de647b42c..93483fe84425 100644 --- a/fs/debugfs/internal.h +++ b/fs/debugfs/internal.h @@ -19,6 +19,7 @@ struct debugfs_inode_info { const struct debugfs_short_fops *short_fops; debugfs_automount_t automount; }; + const void *aux; }; static inline struct debugfs_inode_info *DEBUGFS_I(struct inode *inode) diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 59444b495d49..7c97417d73b5 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -79,9 +79,11 @@ struct debugfs_short_fops { struct dentry *debugfs_create_file_full(const char *name, umode_t mode, struct dentry *parent, void *data, + const void *aux, const struct file_operations *fops); struct dentry *debugfs_create_file_short(const char *name, umode_t mode, struct dentry *parent, void *data, + const void *aux, const struct debugfs_short_fops *fops); /** @@ -126,7 +128,15 @@ struct dentry *debugfs_create_file_short(const char *name, umode_t mode, const struct debugfs_short_fops *: debugfs_create_file_short, \ struct file_operations *: debugfs_create_file_full, \ struct debugfs_short_fops *: debugfs_create_file_short) \ - (name, mode, parent, data, fops) + (name, mode, parent, data, NULL, fops) + +#define debugfs_create_file_aux(name, mode, parent, data, aux, fops) \ + _Generic(fops, \ + const struct file_operations *: debugfs_create_file_full, \ + const struct debugfs_short_fops *: debugfs_create_file_short, \ + struct file_operations *: debugfs_create_file_full, \ + struct debugfs_short_fops *: debugfs_create_file_short) \ + (name, mode, parent, data, aux, fops) struct dentry *debugfs_create_file_unsafe(const char *name, umode_t mode, struct dentry *parent, void *data, @@ -153,6 +163,7 @@ void debugfs_remove(struct dentry *dentry); void debugfs_lookup_and_remove(const char *name, struct dentry *parent); const struct file_operations *debugfs_real_fops(const struct file *filp); +const void *debugfs_get_aux(const struct file *file); int debugfs_file_get(struct dentry *dentry); void debugfs_file_put(struct dentry *dentry); @@ -259,6 +270,14 @@ static inline struct dentry *debugfs_lookup(const char *name, return ERR_PTR(-ENODEV); } +static inline struct dentry *debugfs_create_file_aux(const char *name, + umode_t mode, struct dentry *parent, + void *data, void *aux, + const void *fops) +{ + return ERR_PTR(-ENODEV); +} + static inline struct dentry *debugfs_create_file(const char *name, umode_t mode, struct dentry *parent, void *data, const void *fops) @@ -312,6 +331,7 @@ static inline void debugfs_lookup_and_remove(const char *name, { } const struct file_operations *debugfs_real_fops(const struct file *filp); +void *debugfs_get_aux(const struct file *file); static inline int debugfs_file_get(struct dentry *dentry) { @@ -452,6 +472,11 @@ static inline ssize_t debugfs_read_file_str(struct file *file, #endif +#define debugfs_create_file_aux_num(name, mode, parent, data, n, fops) \ + debugfs_create_file_aux(name, mode, parent, data, \ + (void *)(unsigned long)n, fops) +#define debugfs_get_aux_num(f) (unsigned long)debugfs_get_aux(f) + /** * debugfs_create_xul - create a debugfs file that is used to read and write an * unsigned long value, formatted in hexadecimal -- cgit v1.2.3 From 57b314752ec0ad42685bc78b376326f1f4c04669 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 29 Jan 2025 19:19:37 +0000 Subject: debugfs: Fix the missing initializations in __debugfs_file_get() both method table pointers in debugfs_fsdata need to be initialized, obviously, and calculating the bitmap of present methods would also go better if we start with initialized state. Fixes: 41a0ecc0997c ("debugfs: get rid of dynamically allocation proxy_ops") Signed-off-by: Al Viro Link: https://lore.kernel.org/r/20250129191937.GR1977892@ZenIV Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'fs/debugfs/file.c') diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index e33cc77699cd..69e9ddcb113d 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -94,6 +94,7 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode) fsd = d_fsd; } else { struct inode *inode = dentry->d_inode; + unsigned int methods = 0; if (WARN_ON(mode == DBGFS_GET_ALREADY)) return -EINVAL; @@ -106,25 +107,28 @@ static int __debugfs_file_get(struct dentry *dentry, enum dbgfs_get_mode mode) const struct debugfs_short_fops *ops; ops = fsd->short_fops = DEBUGFS_I(inode)->short_fops; if (ops->llseek) - fsd->methods |= HAS_LSEEK; + methods |= HAS_LSEEK; if (ops->read) - fsd->methods |= HAS_READ; + methods |= HAS_READ; if (ops->write) - fsd->methods |= HAS_WRITE; + methods |= HAS_WRITE; + fsd->real_fops = NULL; } else { const struct file_operations *ops; ops = fsd->real_fops = DEBUGFS_I(inode)->real_fops; if (ops->llseek) - fsd->methods |= HAS_LSEEK; + methods |= HAS_LSEEK; if (ops->read) - fsd->methods |= HAS_READ; + methods |= HAS_READ; if (ops->write) - fsd->methods |= HAS_WRITE; + methods |= HAS_WRITE; if (ops->unlocked_ioctl) - fsd->methods |= HAS_IOCTL; + methods |= HAS_IOCTL; if (ops->poll) - fsd->methods |= HAS_POLL; + methods |= HAS_POLL; + fsd->short_fops = NULL; } + fsd->methods = methods; refcount_set(&fsd->active_users, 1); init_completion(&fsd->active_users_drained); INIT_LIST_HEAD(&fsd->cancellations); -- cgit v1.2.3