Skip to content

Commit

Permalink
fs/namespace.c: WARN if mnt_count has become negative
Browse files Browse the repository at this point in the history
Missing calls to mntget() (or equivalently, too many calls to mntput())
are hard to detect because mntput() delays freeing mounts using
task_work_add(), then again using call_rcu().  As a result, mnt_count
can often be decremented to -1 without getting a KASAN use-after-free
report.  Such cases are still bugs though, and they point to real
use-after-frees being possible.

For an example of this, see the bug fixed by commit 1b0b9cc
("vfs: fsmount: add missing mntget()"), discussed at
https://lkml.kernel.org/linux-fsdevel/20190605135401.GB30925@xxxxxxxxxxxxxxxxxxxxxxxxx/T/#u.
This bug *should* have been trivial to find.  But actually, it wasn't
found until syzkaller happened to use fchdir() to manipulate the
reference count just right for the bug to be noticeable.

Address this by making mntput_no_expire() issue a WARN if mnt_count has
become negative.

Suggested-by: Miklos Szeredi <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Al Viro <[email protected]>
  • Loading branch information
ebiggers authored and Al Viro committed Dec 10, 2020
1 parent b650545 commit edf7ddb
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
9 changes: 6 additions & 3 deletions fs/namespace.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@ static inline void mnt_add_count(struct mount *mnt, int n)
/*
* vfsmount lock must be held for write
*/
unsigned int mnt_get_count(struct mount *mnt)
int mnt_get_count(struct mount *mnt)
{
#ifdef CONFIG_SMP
unsigned int count = 0;
int count = 0;
int cpu;

for_each_possible_cpu(cpu) {
Expand Down Expand Up @@ -1139,6 +1139,7 @@ static DECLARE_DELAYED_WORK(delayed_mntput_work, delayed_mntput);
static void mntput_no_expire(struct mount *mnt)
{
LIST_HEAD(list);
int count;

rcu_read_lock();
if (likely(READ_ONCE(mnt->mnt_ns))) {
Expand All @@ -1162,7 +1163,9 @@ static void mntput_no_expire(struct mount *mnt)
*/
smp_mb();
mnt_add_count(mnt, -1);
if (mnt_get_count(mnt)) {
count = mnt_get_count(mnt);
if (count != 0) {
WARN_ON(count < 0);
rcu_read_unlock();
unlock_mount_hash();
return;
Expand Down
2 changes: 1 addition & 1 deletion fs/pnode.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ int propagate_mount_busy(struct mount *, int);
void propagate_mount_unlock(struct mount *);
void mnt_release_group_id(struct mount *);
int get_dominating_id(struct mount *mnt, const struct path *root);
unsigned int mnt_get_count(struct mount *mnt);
int mnt_get_count(struct mount *mnt);
void mnt_set_mountpoint(struct mount *, struct mountpoint *,
struct mount *);
void mnt_change_mountpoint(struct mount *parent, struct mountpoint *mp,
Expand Down

0 comments on commit edf7ddb

Please sign in to comment.