Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Stephen Brennan <stephen.s.brennan@oracle.com>
To:     Alexander Viro <viro@zeniv.linux.org.uk>,
        Jens Axboe <axboe@kernel.dk>,
        Dmitry Kadashev <dkadashev@gmail.com>
Cc:     Stephen Brennan <stephen.s.brennan@oracle.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 1/3] namei: Fix use after free in kern_path_locked
Date:   Tue,  7 Sep 2021 13:50:41 -0700
Message-Id: <20210907205043.16768-2-stephen.s.brennan@oracle.com>
In-Reply-To: <20210907205043.16768-1-stephen.s.brennan@oracle.com>
References: <20210907205043.16768-1-stephen.s.brennan@oracle.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?b0QyD8AeF9uuWsiajM0rxW3BBDxabT83LU1IHJatkUONSQJmDAoXY9xmnmAT?=
 =?us-ascii?Q?xQAJW56gOUxO+47KWAKdYqTVRy/JqTqLPdk8w8xGAja5B8CGgcHRHfl3nP1g?=
 =?us-ascii?Q?ecQ7aI65mEsHWqyFtWZKA3HN/j4hLcv9HAddTK03mZPMWuTtp7zTRFZqzHi3?=
 =?us-ascii?Q?gM0G+zZqUWQcIbIdOWeKTKGLbG/GT1CIN/bziAnkMTW/mrNE7NF3Th7HpfyF?=
 =?us-ascii?Q?0lgOX0lCGXfgYDzLlyKqaovA6fg83LqPDxy0o0pN4gzjgJzHTvRSEn5cQSCg?=
 =?us-ascii?Q?MVlnhrY/RAsa6eiEO63biyVIrTqMQ/te8mEkXQV4bAW48TPFjSo02fjSU2va?=
 =?us-ascii?Q?ExkcZDjOS8XhWkGT2RqehL06TC5xNsDdwKSTJ2bzQQPnP4L5kIszLWnsblmo?=
 =?us-ascii?Q?/EMllRdUDrFxVY1KhM/TgG2Zqn4nNjDSgwRmPuHkvkMSkcHfXdtMtW78zDw6?=
 =?us-ascii?Q?rTxAtEUnfKiyUd2gtmeQUdA+Rw+UODe9BfpiXmRL21x9lrj3jpMeYT6w6zmn?=
 =?us-ascii?Q?Tph/dHbCDAkm5JGSpwHEUBBMuxDccaynAxYfB/i3hDK3lfCfHJoGzxqN/W9s?=
 =?us-ascii?Q?ogi0S5JDqnb5kppYSHvZDOA69zCX+66rOvCPoINAnhK5zWnJO3car+pb43KK?=
 =?us-ascii?Q?ZZ9FOVAJiqDDVCHsEfuOwGQ5mdyXItzovWzZLrju/josTwT0UNAnpuNO2OSW?=
 =?us-ascii?Q?L44f87GN+06b7wFlTvFN8Rt5jI1Kq7w97/woQKlCNa8yGfRTrWqEIjKz7UOW?=
 =?us-ascii?Q?hPR6Qzk3Tdj85mc0RMm55ip2RIzfS8Te5rImKUJo45kAYyTOwm2l3NEcVAaj?=
 =?us-ascii?Q?s9nJhTaL5LR487lGrTimzitr0yRmgByGasGJQ4Mg9CEFR+QjznB4XMuYKmVS?=
 =?us-ascii?Q?ikf2mvd0aqRUvuv4LpOm9UKVrzAa5+f8J1ALFY6B9uZe6w0qQHdFpImAbXz9?=
 =?us-ascii?Q?FzavMqsEqVsFiM+LgHXGcy+Bibvx9uAZeHeO642M6xGNP5APOCe0ES62Ks4F?=
 =?us-ascii?Q?+ExtbF+vI3vmN2uUvFys2B0dy5yvMK0PY7qtRLXO6rZ1NvXULsqAOiuNsNDf?=
 =?us-ascii?Q?GSlIqbcVcF9P/S60gjseKuYri1Ax5Mx21K4x29waBkmbNQiZzBMzC5VSenQf?=
 =?us-ascii?Q?y6BQioGoHZkYUhuc1AizDJpufYINoUHaBgvTCdOsOXAJLuCKdNT5bGqkiSAY?=
 =?us-ascii?Q?EFrT7Atdxg6pJyUSNhuexSV8zP8mc80a3+/0q6EBAWZezoAs3YJMw8ORzxDC?=
 =?us-ascii?Q?7QC3ePF8h60FOnH9ASdH805sX2q9MT58xJTgBaGS8ccVZwWy5/NXVFk6WhHz?=
 =?us-ascii?Q?K02J2BcJu0lvkwa2ojzEK/vr?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fcb37d4a-e897-4a5e-1c8c-08d972412b10
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4166.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Sep 2021 20:50:47.1866
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: wWRTWYQplu4ZmBTOJQ6kNVJJwlur/HZrejFTEr0NMEM0iDqh/PQcA1yaUiToBAn4oZMRwzwMPYEflJUV2GdwiVMfSQEDsBdFaeMmuo8DvyI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB4149
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10100 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 malwarescore=0
 mlxscore=0 adultscore=0 phishscore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109030001
 definitions=main-2109070130
X-Proofpoint-GUID: 8T5xfO6vbJTeRdxhLaCDuyW__VlgV5LE
X-Proofpoint-ORIG-GUID: 8T5xfO6vbJTeRdxhLaCDuyW__VlgV5LE
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

In 0ee50b47532a ("namei: change filename_parentat() calling
conventions"), filename_parentat() was made to always call putname() on
the  filename before returning, and kern_path_locked() was migrated to
this calling convention. However, kern_path_locked() uses the "last"
parameter to lookup and potentially create a new dentry. The last
parameter contains the last component of the path and points within the
filename, which was recently freed at the end of filename_parentat().
Thus, when kern_path_locked() calls __lookup_hash(), it is using the
filename after it has already been freed.

In this case, filename_parentat() is fundamentally broken due to this
use after free. So, remove it, and rename __filename_parentat to
filename_parentat, migrating all callers. Adjust kern_path_locked to put
the filename once all users are done with it.

Fixes: 0ee50b47532a ("namei: change filename_parentat() calling conventions")
Link: https://lore.kernel.org/linux-fsdevel/YS9D4AlEsaCxLFV0@infradead.org/
Link: https://lore.kernel.org/linux-fsdevel/YS+csMTV2tTXKg3s@zeniv-ca.linux.org.uk/
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Reported-by: syzbot+fb0d60a179096e8c2731@syzkaller.appspotmail.com
Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Co-authored-by: Dmitry Kadashev <dkadashev@gmail.com>
---
 fs/namei.c | 47 ++++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index d049d3972695..f2af301cc79f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2514,9 +2514,10 @@ static int path_parentat(struct nameidata *nd, unsigned flags,
 	return err;
 }
 
-static int __filename_parentat(int dfd, struct filename *name,
-				unsigned int flags, struct path *parent,
-				struct qstr *last, int *type)
+/* Note: this does not consume "name" */
+static int filename_parentat(int dfd, struct filename *name,
+			     unsigned int flags, struct path *parent,
+			     struct qstr *last, int *type)
 {
 	int retval;
 	struct nameidata nd;
@@ -2538,30 +2539,24 @@ static int __filename_parentat(int dfd, struct filename *name,
 	return retval;
 }
 
-static int filename_parentat(int dfd, struct filename *name,
-				unsigned int flags, struct path *parent,
-				struct qstr *last, int *type)
-{
-	int retval = __filename_parentat(dfd, name, flags, parent, last, type);
-
-	putname(name);
-	return retval;
-}
-
 /* does lookup, returns the object with parent locked */
 struct dentry *kern_path_locked(const char *name, struct path *path)
 {
+	struct filename *filename;
 	struct dentry *d;
 	struct qstr last;
 	int type, error;
 
-	error = filename_parentat(AT_FDCWD, getname_kernel(name), 0, path,
-				    &last, &type);
-	if (error)
-		return ERR_PTR(error);
+	filename = getname_kernel(name);
+	error = filename_parentat(AT_FDCWD, filename, 0, path, &last, &type);
+	if (error) {
+		d = ERR_PTR(error);
+		goto out;
+	}
 	if (unlikely(type != LAST_NORM)) {
 		path_put(path);
-		return ERR_PTR(-EINVAL);
+		d = ERR_PTR(-EINVAL);
+		goto out;
 	}
 	inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
 	d = __lookup_hash(&last, path->dentry, 0);
@@ -2569,6 +2564,8 @@ struct dentry *kern_path_locked(const char *name, struct path *path)
 		inode_unlock(path->dentry->d_inode);
 		path_put(path);
 	}
+out:
+	putname(filename);
 	return d;
 }
 
@@ -3634,7 +3631,7 @@ static struct dentry *__filename_create(int dfd, struct filename *name,
 	 */
 	lookup_flags &= LOOKUP_REVAL;
 
-	error = __filename_parentat(dfd, name, lookup_flags, path, &last, &type);
+	error = filename_parentat(dfd, name, lookup_flags, path, &last, &type);
 	if (error)
 		return ERR_PTR(error);
 
@@ -3996,7 +3993,7 @@ int do_rmdir(int dfd, struct filename *name)
 	int type;
 	unsigned int lookup_flags = 0;
 retry:
-	error = __filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
+	error = filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
 	if (error)
 		goto exit1;
 
@@ -4135,7 +4132,7 @@ int do_unlinkat(int dfd, struct filename *name)
 	struct inode *delegated_inode = NULL;
 	unsigned int lookup_flags = 0;
 retry:
-	error = __filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
+	error = filename_parentat(dfd, name, lookup_flags, &path, &last, &type);
 	if (error)
 		goto exit1;
 
@@ -4683,13 +4680,13 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
 		target_flags = 0;
 
 retry:
-	error = __filename_parentat(olddfd, from, lookup_flags, &old_path,
-					&old_last, &old_type);
+	error = filename_parentat(olddfd, from, lookup_flags, &old_path,
+				  &old_last, &old_type);
 	if (error)
 		goto put_names;
 
-	error = __filename_parentat(newdfd, to, lookup_flags, &new_path, &new_last,
-				&new_type);
+	error = filename_parentat(newdfd, to, lookup_flags, &new_path, &new_last,
+				  &new_type);
 	if (error)
 		goto exit1;
 
-- 
2.30.2