Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6fd03179 authored by Hao Luo's avatar Hao Luo Committed by Greg Kroah-Hartman
Browse files

kernfs: Separate kernfs_pr_cont_buf and rename_lock. 



[ Upstream commit 1a702dc88e150487c9c173a249b3d236498b9183 ]

Previously the protection of kernfs_pr_cont_buf was piggy backed by
rename_lock, which means that pr_cont() needs to be protected under
rename_lock. This can cause potential circular lock dependencies.

If there is an OOM, we have the following call hierarchy:

 -> cpuset_print_current_mems_allowed()
   -> pr_cont_cgroup_name()
     -> pr_cont_kernfs_name()

pr_cont_kernfs_name() will grab rename_lock and call printk. So we have
the following lock dependencies:

 kernfs_rename_lock -> console_sem

Sometimes, printk does a wakeup before releasing console_sem, which has
the dependence chain:

 console_sem -> p->pi_lock -> rq->lock

Now, imagine one wants to read cgroup_name under rq->lock, for example,
printing cgroup_name in a tracepoint in the scheduler code. They will
be holding rq->lock and take rename_lock:

 rq->lock -> kernfs_rename_lock

Now they will deadlock.

A prevention to this circular lock dependency is to separate the
protection of pr_cont_buf from rename_lock. In principle, rename_lock
is to protect the integrity of cgroup name when copying to buf. Once
pr_cont_buf has got its content, rename_lock can be dropped. So it's
safe to drop rename_lock after kernfs_name_locked (and
kernfs_path_from_node_locked) and rely on a dedicated pr_cont_lock
to protect pr_cont_buf.

Acked-by: default avatarTejun Heo <tj@kernel.org>
Signed-off-by: default avatarHao Luo <haoluo@google.com>
Link: https://lore.kernel.org/r/20220516190951.3144144-1-haoluo@google.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent 19f4b51b

fs/kernfs/dir.c

+19 −12
Original line number Diff line number Diff line
@@ -19,7 +19,15 @@ 


DEFINE_MUTEX(kernfs_mutex);

static DEFINE_SPINLOCK(kernfs_rename_lock);	/* kn->parent and ->name */

static char kernfs_pr_cont_buf[PATH_MAX];	/* protected by rename_lock */

/*

 * Don't use rename_lock to piggy back on pr_cont_buf. We don't want to

 * call pr_cont() while holding rename_lock. Because sometimes pr_cont()

 * will perform wakeups when releasing console_sem. Holding rename_lock

 * will introduce deadlock if the scheduler reads the kernfs_name in the

 * wakeup path.

 */

static DEFINE_SPINLOCK(kernfs_pr_cont_lock);

static char kernfs_pr_cont_buf[PATH_MAX];	/* protected by pr_cont_lock */

static DEFINE_SPINLOCK(kernfs_idr_lock);	/* root->ino_idr */



#define rb_to_kn(X) rb_entry((X), struct kernfs_node, rb)
@@ -230,12 +238,12 @@ void pr_cont_kernfs_name(struct kernfs_node *kn) 
{

	unsigned long flags;



	spin_lock_irqsave(&kernfs_rename_lock, flags);

	spin_lock_irqsave(&kernfs_pr_cont_lock, flags);



	kernfs_name_locked(kn, kernfs_pr_cont_buf, sizeof(kernfs_pr_cont_buf));

	kernfs_name(kn, kernfs_pr_cont_buf, sizeof(kernfs_pr_cont_buf));

	pr_cont("%s", kernfs_pr_cont_buf);



	spin_unlock_irqrestore(&kernfs_rename_lock, flags);

	spin_unlock_irqrestore(&kernfs_pr_cont_lock, flags);

}



/**
@@ -249,9 +257,9 @@ void pr_cont_kernfs_path(struct kernfs_node *kn) 
	unsigned long flags;

	int sz;



	spin_lock_irqsave(&kernfs_rename_lock, flags);

	spin_lock_irqsave(&kernfs_pr_cont_lock, flags);



	sz = kernfs_path_from_node_locked(kn, NULL, kernfs_pr_cont_buf,

	sz = kernfs_path_from_node(kn, NULL, kernfs_pr_cont_buf,

				   sizeof(kernfs_pr_cont_buf));

	if (sz < 0) {

		pr_cont("(error)");
@@ -266,7 +274,7 @@ void pr_cont_kernfs_path(struct kernfs_node *kn) 
	pr_cont("%s", kernfs_pr_cont_buf);



out:

	spin_unlock_irqrestore(&kernfs_rename_lock, flags);

	spin_unlock_irqrestore(&kernfs_pr_cont_lock, flags);

}



/**
@@ -870,13 +878,12 @@ static struct kernfs_node *kernfs_walk_ns(struct kernfs_node *parent, 


	lockdep_assert_held(&kernfs_mutex);



	/* grab kernfs_rename_lock to piggy back on kernfs_pr_cont_buf */

	spin_lock_irq(&kernfs_rename_lock);

	spin_lock_irq(&kernfs_pr_cont_lock);



	len = strlcpy(kernfs_pr_cont_buf, path, sizeof(kernfs_pr_cont_buf));



	if (len >= sizeof(kernfs_pr_cont_buf)) {

		spin_unlock_irq(&kernfs_rename_lock);

		spin_unlock_irq(&kernfs_pr_cont_lock);

		return NULL;

	}
@@ -888,7 +895,7 @@ static struct kernfs_node *kernfs_walk_ns(struct kernfs_node *parent, 
		parent = kernfs_find_ns(parent, name, ns);

	}



	spin_unlock_irq(&kernfs_rename_lock);

	spin_unlock_irq(&kernfs_pr_cont_lock);



	return parent;

}