linux-kernel - [PATCH v0] kernfs: Skip kernfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1554463267-30841-1-git-send-email-gkohli@codeaurora.org>
Date:   Fri,  5 Apr 2019 16:51:07 +0530
From:   Gaurav Kohli <gkohli@...eaurora.org>
To:     gregkh@...uxfoundation.org, tj@...nel.org,
        linux-kernel@...r.kernel.org
Cc:     linux-arm-msm@...r.kernel.org,
        Gaurav Kohli <gkohli@...eaurora.org>,
        Mukesh Ojha <mojha@...eaurora.org>
Subject: [PATCH v0] kernfs: Skip kernfs_put of parent from child node

While adding kernfs node for child to the parent kernfs
node and when child node founds that parent kn count is
zero, then below comes like:

WARNING: fs/kernfs/dir.c:494 kernfs_get+0x64/0x88

This indicates that parent is in kernfs_put path/ or already
freed, and if the child node keeps continue to
make new kernfs node, then there is chance of
below race for parent node:

CPU0				         CPU1
//Parent node			         //child node
kernfs_put
atomic_dec_and_test(&kn->count)
//count is 0, so continue
					  kernfs_new_node(child)
					  kernfs_get(parent);
					  //increment parent count to 1
					  //warning come as parent count is 0
					  /* link in */
					  kernfs_add_one(kn);
					  // this should fail as parent is
					  //in free path.
					  kernfs_put(child)
kmem_cache_free(parent)
					  kmem_cache_free(child)
					  kn = parent
					  atomic_dec_and_test(&kn->count))
					  //this is 0 now, so release will
					  continue for parent.
					  kmem_cache_free(parent)

To prevent this race, child simply has to decrement count of parent
kernfs node and keep continue the free path for itself.

Signed-off-by: Gaurav Kohli <gkohli@...eaurora.org>
Signed-off-by: Mukesh Ojha <mojha@...eaurora.org>

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index b84d635..d5a36e8 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -515,7 +515,6 @@ void kernfs_put(struct kernfs_node *kn)
 	if (!kn || !atomic_dec_and_test(&kn->count))
 		return;
 	root = kernfs_root(kn);
- repeat:
 	/*
 	 * Moving/renaming is always done while holding reference.
 	 * kn->parent won't change beneath us.
@@ -545,8 +544,8 @@ void kernfs_put(struct kernfs_node *kn)
 
 	kn = parent;
 	if (kn) {
-		if (atomic_dec_and_test(&kn->count))
-			goto repeat;
+	/* Parent may be on free path, so simply decrement the count */
+		atomic_dec_and_test(&kn->count);
 	} else {
 		/* just released the root kn, free @root too */
 		idr_destroy(&root->ino_idr);
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project