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 1/3] namei: Fix use after free in kern_path_locked
Date:   Wed,  1 Sep 2021 10:51:41 -0700
Message-Id: <20210901175144.121048-2-stephen.s.brennan@oracle.com>
In-Reply-To: <20210901175144.121048-1-stephen.s.brennan@oracle.com>
References: <20210901175144.121048-1-stephen.s.brennan@oracle.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from localhost (148.87.23.10) by SJ0PR05CA0140.namprd05.prod.outlook.com (2603:10b6:a03:33d::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4500.4 via Frontend Transport; Wed, 1 Sep 2021 17:51:47 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: a4f2bed7-7bdc-4e28-8b2e-08d96d712b4b
X-MS-TrafficTypeDiagnostic: CH2PR10MB3863:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <CH2PR10MB38637F034C295F2AC27EA29DDBCD9@CH2PR10MB3863.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:4303;
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?fnd1zrepxoV7FY96CCZSrshBfX1lUKXnMLqWvXC5MpzmT3bhZatoIniAJTMJ?=
 =?us-ascii?Q?DzBcBo1dNHTg5LqmIGsx0KgXpRfhOJ86romyI8cbPMQt0QPSf/b9nfVZnVW3?=
 =?us-ascii?Q?IsaPvenyxy+/YGHaWlEg5nBnSNFGLWyPwyOeLXeoKuxVai2Ri6d9JSe2H2TE?=
 =?us-ascii?Q?3p8pWN28mcoslBsz/i1qfwgmiiaK81oHgON6MK71qgDDBZMC32bJO5B8a/MF?=
 =?us-ascii?Q?eJ5VMDwor3HKvDrc9LRK3TLBftpUWz6/AA9wDKwTxAITqO5sFdiaWSCWn9ei?=
 =?us-ascii?Q?xb6jucccZ9Jbr25RqraO3T8wA4EXf1gBq7mjOMrUCo3DH1x/HoBz+gvblVeF?=
 =?us-ascii?Q?1JjDVCzOMZpp7rS4mXEeY/k+aTqWOCA7vIGhu7kMVQRvRamdxObESHF/F0ri?=
 =?us-ascii?Q?FHr9TfeFw4aeY/rVONCXXhP3i8BHk6JPZ3wFKwIQdBXCqFuISrESoJcyYFtO?=
 =?us-ascii?Q?Ay8jg8MgYY+bCJbqKNIi6OPB0dL+jTb0od66n986YHUjbwMaO+Eb6GlDAC6j?=
 =?us-ascii?Q?QypucdvPYE3yfx+afo+vVSNLUY3Q9V9hcBYuw8zBTZLF0+VAJAl2W91P82kA?=
 =?us-ascii?Q?SONoKo4cTNyGDKoAiCYA/APuLXRTFIE8Ufr+4yDU4wp9p9+EBy9VXssKXeeT?=
 =?us-ascii?Q?GZvreSVj3yVpB7aJOOOxmT8WN+xOEu4VrLKQxdsaSdx3KTCaeBQvNWHaBh7o?=
 =?us-ascii?Q?5JBQrkfLVjt/zSw0vH6PXKeS8BGR74o48utYk1vwiJN+ZP1WLzpaZK9CvNoC?=
 =?us-ascii?Q?w363kT9JslJ1qOBhC9bsGZ+TYpJPl7CejTPxvr9ZBNbYDqd0VND+tAODEkTT?=
 =?us-ascii?Q?GVkZlflWIcmyZxYZdZv7hK0/oSIrYEFTJpDpeWlxt6LuvGwcNGgC05tfkPI8?=
 =?us-ascii?Q?NUiwn4x8hffIJvV1AmkqGVcn9tMs5E0YQWjvAXzDFAIMDS0CfRyszSBw7h39?=
 =?us-ascii?Q?6vpDazdOhcuL6hji0E/FNcmQe7ZzfW4kufgHV9z+jHxZlydjAZS2r9aRHqB+?=
 =?us-ascii?Q?ucQUd+yjzPd7uaqw/Gf/Mcz2pieC8LwycuJbb53pstLNYM51/q9UsLm8bMnL?=
 =?us-ascii?Q?iXmojSS5gVCmN5PoxzD913GcyzHwZnKpdP1qY8bHNC4C3LWfm3fdSXQC5gQ0?=
 =?us-ascii?Q?WsCjc4GwH9XlKqguP3HgD7WR6kDbbUdzQA=3D=3D?=
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:5;SRV:;IPV:NLI;SFV:SPM;H:CH2PR10MB4166.namprd10.prod.outlook.com;PTR:;CAT:OSPM;SFS:(39860400002)(136003)(376002)(346002)(366004)(396003)(5660300002)(316002)(4326008)(38350700002)(38100700002)(6486002)(66946007)(66476007)(110136005)(66556008)(83380400001)(86362001)(8936002)(478600001)(36756003)(103116003)(8676002)(6666004)(6496006)(2906002)(52116002)(2616005)(956004)(966005)(26005)(1076003)(186003)(23200700001);DIR:OUT;SFP:1501;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?zxGJC4ODN5zSV/pV7alN514x0x7cWTpXi9ioq/KwRSEa/RpV5Gw2fQYQxKrC?=
 =?us-ascii?Q?M3jY8v8+Ze36zG0Ll15Gv6QQc6rgmNPdPlcZJlHjk8J3s4CdjnfdCqFSREeF?=
 =?us-ascii?Q?wy1Uam76uXFeN+N3prdVcYIxLyHUHKkbzYFIGjAcc/+ZMTFDHHVtZjUZGgYZ?=
 =?us-ascii?Q?kTu7tfJ20AE2Y7nFVIPyQ9lyQKhwXzVd3FI76wZ1CrVo05fpUFmcW0/lzPUm?=
 =?us-ascii?Q?e/KpuiWwh6fCqc4YnEGTZJJ9bh4a4OHhsr/JIp22pVNZm9lFtCyFbAflJ4Ws?=
 =?us-ascii?Q?FB0TNNdPLZzJR5d1POZInwuOFV3StQut51ghuR27wQUXQMNFFq1G5SA2B0SD?=
 =?us-ascii?Q?O0y0Ox+3g9fRvviDinC7jvSZL9Dxm5lSQe++VHhahURrgco6k0zvW/6H+IND?=
 =?us-ascii?Q?iT+ws55+hQtvxi/MB6K4YzMZPY4Xg1iL+darOFvGsIQDoJjf5HnsaRhhbrVB?=
 =?us-ascii?Q?iJnzJEwqGZVLic/ntxFLS2GNdmcTRuAm4u6DAFmkvOgEZVLQLTO0/ag8qECo?=
 =?us-ascii?Q?GYY/ginJu3PDvFZZZBYIFZU6IrxeBDmzBIevJlQXQ7RD2UODTVRLwGbq0BOx?=
 =?us-ascii?Q?YkiiUgTMDLJTpJYGkKJsZkl/PTqBnGo5e8h3YK/8FNcLZAgV5dRakI3RB6j/?=
 =?us-ascii?Q?Kci6RHvZ9ci1AnCUcuF05XkOlELPUASh62Lmg4BGQTKryset1KEKxddGV4Cb?=
 =?us-ascii?Q?GdXdCGqcpaPv9R9n/FmbfoO6Kd9UZM6jxZ2d4H7uib5Fqfa6/Gd7yCBc3NTA?=
 =?us-ascii?Q?68nIWaKdHMVHOjSXJYlXfVrgaTXMy+A7NlBrSnYxaXtPFILDLHmNCO4sIq1E?=
 =?us-ascii?Q?c6XLrzsgVwjO8WJQtWd5nB7GF+c23BU/Pxa330oHqULKuxhH08bY8/XFLDLP?=
 =?us-ascii?Q?oJrUB9UcQEOb0gCefAQBQF/JiEC/NnM+Jy2pNniX2CpJvKimgwg7fNjmYI0f?=
 =?us-ascii?Q?G3FQ0ruQUB+RE3ogObfHJo28tfNNFmL0KVm9k0XMWhheGyqmK+dM1wpJ/MdE?=
 =?us-ascii?Q?Dp8GeH5BpSG83omJHJNES97f3nkt9mgepf0nGqtn5Q/YaPDz+OREsp2LAFAj?=
 =?us-ascii?Q?m+kMHF5GWWCR+Jn1ANz1XI67RqXFMn9njefDlrNTjwwwbiACdW/AV5q/chGY?=
 =?us-ascii?Q?zHcOn3R4Pfmo5RRcU0xsXVPDDTMD7kooGZpJ2pYVQaxFUAExGZdV80a1VQRh?=
 =?us-ascii?Q?DZBxbLeaPsqyZHrd8GktpYDeQxuylu67wR3bexRG/YjzLV4iuN6oV6Rjz+7Z?=
 =?us-ascii?Q?HaLOcknvyM5BjJ9BkJbX2rAEE0mbxX6vTLGxvfVIHG07aL8lD0iaecBS2MB/?=
 =?us-ascii?Q?NnXeaO8xe9ud4ueB5AGrOAmL?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a4f2bed7-7bdc-4e28-8b2e-08d96d712b4b
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4166.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Sep 2021 17:51:47.6889
 (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: qFkR5H0pc68KENW57OmAuxBvVK/Ju0f+hXTS5eRjMA8CygCpb7Z9znHdiTv4GVGYMvB7wWEzQpeKZ++FhMHngrJYbZX0IMf92fb5ODJiO8Q=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB3863
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10094 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 malwarescore=0 bulkscore=0
 suspectscore=0 phishscore=0 adultscore=0 mlxscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2108310000
 definitions=main-2109010102
X-Proofpoint-GUID: p7dpRHDQA3L0qtWHM6lwwlMLZ1AuFIHq
X-Proofpoint-ORIG-GUID: p7dpRHDQA3L0qtWHM6lwwlMLZ1AuFIHq
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