linux-kernel - [PATCH v2 1/2] ovl: ovl_permission: Fix null pointer dereference at realinode in rcu-walk mode

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20230515133629.1974610-2-chengzhihao1@huawei.com>
Date:   Mon, 15 May 2023 21:36:28 +0800
From:   Zhihao Cheng <chengzhihao1@...wei.com>
To:     <miklos@...redi.hu>, <amir73il@...il.com>, <brauner@...nel.org>
CC:     <linux-unionfs@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <chengzhihao1@...wei.com>
Subject: [PATCH v2 1/2] ovl: ovl_permission: Fix null pointer dereference at realinode in rcu-walk mode

Following process:
          P1                     P2
 path_lookupat
  link_path_walk
   inode_permission
    ovl_permission
      ovl_i_path_real(inode, &realpath)
        path->dentry = ovl_i_dentry_upper(inode)
                          drop_cache
			   __dentry_kill(ovl_dentry)
		            iput(ovl_inode)
		             ovl_destroy_inode(ovl_inode)
		              dput(oi->__upperdentry)
		               dentry_kill(upperdentry)
		                dentry_unlink_inode
				 upperdentry->d_inode = NULL
      realinode = d_inode(realpath.dentry) // return NULL
      inode_permission(realinode)
       inode->i_sb  // NULL pointer dereference
, will trigger an null pointer dereference at realinode:
  [  335.664979] BUG: kernel NULL pointer dereference,
                 address: 0000000000000002
  [  335.668032] CPU: 0 PID: 2592 Comm: ls Not tainted 6.3.0
  [  335.669956] RIP: 0010:inode_permission+0x33/0x2c0
  [  335.678939] Call Trace:
  [  335.679165]  <TASK>
  [  335.679371]  ovl_permission+0xde/0x320
  [  335.679723]  inode_permission+0x15e/0x2c0
  [  335.680090]  link_path_walk+0x115/0x550
  [  335.680771]  path_lookupat.isra.0+0xb2/0x200
  [  335.681170]  filename_lookup+0xda/0x240
  [  335.681922]  vfs_statx+0xa6/0x1f0
  [  335.682233]  vfs_fstatat+0x7b/0xb0

Fetch a reproducer in [Link].

Add a new helper ovl_i_path_realinode() to get realpath and real inode
after non-nullptr checking, use the helper to replace the current realpath
getting logic.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217405
Fixes: 4b7791b2e958 ("ovl: handle idmappings in ovl_permission()")
Signed-off-by: Zhihao Cheng <chengzhihao1@...wei.com>
Suggested-by: Christian Brauner <brauner@...nel.org>
---
 fs/overlayfs/inode.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 541cf3717fc2..cc3ef5a6666a 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -278,6 +278,26 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 	return err;
 }
 
+static inline int ovl_i_path_realinode(struct inode *inode,
+				       struct path *realpath,
+				       struct inode **realinode, int rcu)
+{
+	/* Careful in RCU walk mode */
+	ovl_i_path_real(inode, realpath);
+	if (!realpath->dentry) {
+		WARN_ON(!rcu);
+		return -ECHILD;
+	}
+
+	*realinode = d_inode(realpath->dentry);
+	if (!*realinode) {
+		WARN_ON(!rcu);
+		return -ECHILD;
+	}
+
+	return 0;
+}
+
 int ovl_permission(struct mnt_idmap *idmap,
 		   struct inode *inode, int mask)
 {
@@ -287,12 +307,10 @@ int ovl_permission(struct mnt_idmap *idmap,
 	const struct cred *old_cred;
 	int err;
 
-	/* Careful in RCU walk mode */
-	ovl_i_path_real(inode, &realpath);
-	if (!realpath.dentry) {
-		WARN_ON(!(mask & MAY_NOT_BLOCK));
-		return -ECHILD;
-	}
+	err = ovl_i_path_realinode(inode, &realpath, &realinode,
+				   mask & MAY_NOT_BLOCK);
+	if (err)
+		return err;
 
 	/*
 	 * Check overlay inode with the creds of task and underlying inode
@@ -302,7 +320,6 @@ int ovl_permission(struct mnt_idmap *idmap,
 	if (err)
 		return err;
 
-	realinode = d_inode(realpath.dentry);
 	old_cred = ovl_override_creds(inode->i_sb);
 	if (!upperinode &&
 	    !special_file(realinode->i_mode) && mask & MAY_WRITE) {
-- 
2.39.2