On Wed, Jan 29, 2025 at 08:13:02AM +0100, Greg Kroah-Hartman wrote:
> > Both are needed, actually. Slightly longer term I would rather
> > split full_proxy_{read,write,lseek}() into short and full variant,
> > getting rid of the "check which pointer is non-NULL" and killed
> > the two remaining users of debugfs_real_fops() outside of
> > fs/debugfs/file.c; then we could union these ->..._fops pointers,
> > but until then they need to be initialized.
> >
> > And yes, ->methods obviously needs to be initialized.
> >
> > Al, bloody embarrassed ;-/
>
> No worries, want to send a patch to fix both of these up so we can fix
> up Linus's tree now?
[PATCH] 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"
Fucked-up-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
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);
