Skip to content

Commit

Permalink
debugfs: annotate debugfs handlers vs. removal with lockdep
Browse files Browse the repository at this point in the history
When you take a lock in a debugfs handler but also try
to remove the debugfs file under that lock, things can
deadlock since the removal has to wait for all users
to finish.

Add lockdep annotations in debugfs_file_get()/_put()
to catch such issues.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
  • Loading branch information
Johannes Berg committed Nov 27, 2023
1 parent 0ed04a1 commit f4acfcd
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 0 deletions.
10 changes: 10 additions & 0 deletions fs/debugfs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ int debugfs_file_get(struct dentry *dentry)
kfree(fsd);
fsd = READ_ONCE(dentry->d_fsdata);
}
#ifdef CONFIG_LOCKDEP
fsd->lock_name = kasprintf(GFP_KERNEL, "debugfs:%pd", dentry);
lockdep_register_key(&fsd->key);
lockdep_init_map(&fsd->lockdep_map, fsd->lock_name ?: "debugfs",
&fsd->key, 0);
#endif
}

/*
Expand All @@ -124,6 +130,8 @@ int debugfs_file_get(struct dentry *dentry)
if (!refcount_inc_not_zero(&fsd->active_users))
return -EIO;

lock_map_acquire_read(&fsd->lockdep_map);

return 0;
}
EXPORT_SYMBOL_GPL(debugfs_file_get);
Expand All @@ -141,6 +149,8 @@ void debugfs_file_put(struct dentry *dentry)
{
struct debugfs_fsdata *fsd = READ_ONCE(dentry->d_fsdata);

lock_map_release(&fsd->lockdep_map);

if (refcount_dec_and_test(&fsd->active_users))
complete(&fsd->active_users_drained);
}
Expand Down
12 changes: 12 additions & 0 deletions fs/debugfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,14 @@ static void debugfs_release_dentry(struct dentry *dentry)
if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
return;

/* check it wasn't a dir (no fsdata) or automount (no real_fops) */
if (fsd && fsd->real_fops) {
#ifdef CONFIG_LOCKDEP
lockdep_unregister_key(&fsd->key);
kfree(fsd->lock_name);
#endif
}

kfree(fsd);
}

Expand Down Expand Up @@ -744,6 +752,10 @@ static void __debugfs_file_removed(struct dentry *dentry)
fsd = READ_ONCE(dentry->d_fsdata);
if ((unsigned long)fsd & DEBUGFS_FSDATA_IS_REAL_FOPS_BIT)
return;

lock_map_acquire(&fsd->lockdep_map);
lock_map_release(&fsd->lockdep_map);

if (!refcount_dec_and_test(&fsd->active_users))
wait_for_completion(&fsd->active_users_drained);
}
Expand Down
6 changes: 6 additions & 0 deletions fs/debugfs/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#ifndef _DEBUGFS_INTERNAL_H_
#define _DEBUGFS_INTERNAL_H_
#include <linux/lockdep.h>

struct file_operations;

Expand All @@ -23,6 +24,11 @@ struct debugfs_fsdata {
struct {
refcount_t active_users;
struct completion active_users_drained;
#ifdef CONFIG_LOCKDEP
struct lockdep_map lockdep_map;
struct lock_class_key key;
char *lock_name;
#endif
};
};
};
Expand Down

0 comments on commit f4acfcd

Please sign in to comment.