Skip to content

Commit 5c86919

Browse files
Paulo Alcantarasmfrench
Paulo Alcantara
authored and
smfrench
committed
smb: client: fix use-after-free in smb2_query_info_compound()
The following UAF was triggered when running fstests generic/072 with KASAN enabled against Windows Server 2022 and mount options 'multichannel,max_channels=2,vers=3.1.1,mfsymlinks,noperm' BUG: KASAN: slab-use-after-free in smb2_query_info_compound+0x423/0x6d0 [cifs] Read of size 8 at addr ffff888014941048 by task xfs_io/27534 CPU: 0 PID: 27534 Comm: xfs_io Not tainted 6.6.0-rc7 #1 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.2-3-gd478f380-rebuilt.opensuse.org 04/01/2014 Call Trace: dump_stack_lvl+0x4a/0x80 print_report+0xcf/0x650 ? srso_alias_return_thunk+0x5/0x7f ? srso_alias_return_thunk+0x5/0x7f ? __phys_addr+0x46/0x90 kasan_report+0xda/0x110 ? smb2_query_info_compound+0x423/0x6d0 [cifs] ? smb2_query_info_compound+0x423/0x6d0 [cifs] smb2_query_info_compound+0x423/0x6d0 [cifs] ? __pfx_smb2_query_info_compound+0x10/0x10 [cifs] ? srso_alias_return_thunk+0x5/0x7f ? __stack_depot_save+0x39/0x480 ? kasan_save_stack+0x33/0x60 ? kasan_set_track+0x25/0x30 ? ____kasan_slab_free+0x126/0x170 smb2_queryfs+0xc2/0x2c0 [cifs] ? __pfx_smb2_queryfs+0x10/0x10 [cifs] ? __pfx___lock_acquire+0x10/0x10 smb311_queryfs+0x210/0x220 [cifs] ? __pfx_smb311_queryfs+0x10/0x10 [cifs] ? srso_alias_return_thunk+0x5/0x7f ? __lock_acquire+0x480/0x26c0 ? lock_release+0x1ed/0x640 ? srso_alias_return_thunk+0x5/0x7f ? do_raw_spin_unlock+0x9b/0x100 cifs_statfs+0x18c/0x4b0 [cifs] statfs_by_dentry+0x9b/0xf0 fd_statfs+0x4e/0xb0 __do_sys_fstatfs+0x7f/0xe0 ? __pfx___do_sys_fstatfs+0x10/0x10 ? srso_alias_return_thunk+0x5/0x7f ? lockdep_hardirqs_on_prepare+0x136/0x200 ? srso_alias_return_thunk+0x5/0x7f do_syscall_64+0x3f/0x90 entry_SYSCALL_64_after_hwframe+0x6e/0xd8 Allocated by task 27534: kasan_save_stack+0x33/0x60 kasan_set_track+0x25/0x30 __kasan_kmalloc+0x8f/0xa0 open_cached_dir+0x71b/0x1240 [cifs] smb2_query_info_compound+0x5c3/0x6d0 [cifs] smb2_queryfs+0xc2/0x2c0 [cifs] smb311_queryfs+0x210/0x220 [cifs] cifs_statfs+0x18c/0x4b0 [cifs] statfs_by_dentry+0x9b/0xf0 fd_statfs+0x4e/0xb0 __do_sys_fstatfs+0x7f/0xe0 do_syscall_64+0x3f/0x90 entry_SYSCALL_64_after_hwframe+0x6e/0xd8 Freed by task 27534: kasan_save_stack+0x33/0x60 kasan_set_track+0x25/0x30 kasan_save_free_info+0x2b/0x50 ____kasan_slab_free+0x126/0x170 slab_free_freelist_hook+0xd0/0x1e0 __kmem_cache_free+0x9d/0x1b0 open_cached_dir+0xff5/0x1240 [cifs] smb2_query_info_compound+0x5c3/0x6d0 [cifs] smb2_queryfs+0xc2/0x2c0 [cifs] This is a race between open_cached_dir() and cached_dir_lease_break() where the cache entry for the open directory handle receives a lease break while creating it. And before returning from open_cached_dir(), we put the last reference of the new @cfid because of !@cfid->has_lease. Besides the UAF, while running xfstests a lot of missed lease breaks have been noticed in tests that run several concurrent statfs(2) calls on those cached fids CIFS: VFS: \\w22-root1.gandalf.test No task to wake, unknown frame... CIFS: VFS: \\w22-root1.gandalf.test Cmd: 18 Err: 0x0 Flags: 0x1... CIFS: VFS: \\w22-root1.gandalf.test smb buf 00000000715bfe83 len 108 CIFS: VFS: Dump pending requests: CIFS: VFS: \\w22-root1.gandalf.test No task to wake, unknown frame... CIFS: VFS: \\w22-root1.gandalf.test Cmd: 18 Err: 0x0 Flags: 0x1... CIFS: VFS: \\w22-root1.gandalf.test smb buf 000000005aa7316e len 108 ... To fix both, in open_cached_dir() ensure that @cfid->has_lease is set right before sending out compounded request so that any potential lease break will be get processed by demultiplex thread while we're still caching @cfid. And, if open failed for some reason, re-check @cfid->has_lease to decide whether or not put lease reference. Cc: stable@vger.kernel.org Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent c37ed2d commit 5c86919

File tree

1 file changed

+49
-35
lines changed

1 file changed

+49
-35
lines changed

‎fs/smb/client/cached_dir.c‎

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
3232
* fully cached or it may be in the process of
3333
* being deleted due to a lease break.
3434
*/
35-
if (!cfid->has_lease) {
35+
if (!cfid->time || !cfid->has_lease) {
3636
spin_unlock(&cfids->cfid_list_lock);
3737
return NULL;
3838
}
@@ -193,10 +193,20 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
193193
npath = path_no_prefix(cifs_sb, path);
194194
if (IS_ERR(npath)) {
195195
rc = PTR_ERR(npath);
196-
kfree(utf16_path);
197-
return rc;
196+
goto out;
198197
}
199198

199+
if (!npath[0]) {
200+
dentry = dget(cifs_sb->root);
201+
} else {
202+
dentry = path_to_dentry(cifs_sb, npath);
203+
if (IS_ERR(dentry)) {
204+
rc = -ENOENT;
205+
goto out;
206+
}
207+
}
208+
cfid->dentry = dentry;
209+
200210
/*
201211
* We do not hold the lock for the open because in case
202212
* SMB2_open needs to reconnect.
@@ -249,6 +259,15 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
249259

250260
smb2_set_related(&rqst[1]);
251261

262+
/*
263+
* Set @cfid->has_lease to true before sending out compounded request so
264+
* its lease reference can be put in cached_dir_lease_break() due to a
265+
* potential lease break right after the request is sent or while @cfid
266+
* is still being cached. Concurrent processes won't be to use it yet
267+
* due to @cfid->time being zero.
268+
*/
269+
cfid->has_lease = true;
270+
252271
rc = compound_send_recv(xid, ses, server,
253272
flags, 2, rqst,
254273
resp_buftype, rsp_iov);
@@ -263,89 +282,84 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
263282
cfid->tcon = tcon;
264283
cfid->is_open = true;
265284

285+
spin_lock(&cfids->cfid_list_lock);
286+
266287
o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
267288
oparms.fid->persistent_fid = o_rsp->PersistentFileId;
268289
oparms.fid->volatile_fid = o_rsp->VolatileFileId;
269290
#ifdef CONFIG_CIFS_DEBUG2
270291
oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
271292
#endif /* CIFS_DEBUG2 */
272293

273-
if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE)
294+
rc = -EINVAL;
295+
if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
296+
spin_unlock(&cfids->cfid_list_lock);
274297
goto oshr_free;
298+
}
275299

276300
smb2_parse_contexts(server, o_rsp,
277301
&oparms.fid->epoch,
278302
oparms.fid->lease_key, &oplock,
279303
NULL, NULL);
280-
if (!(oplock & SMB2_LEASE_READ_CACHING_HE))
304+
if (!(oplock & SMB2_LEASE_READ_CACHING_HE)) {
305+
spin_unlock(&cfids->cfid_list_lock);
281306
goto oshr_free;
307+
}
282308
qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
283-
if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info))
309+
if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info)) {
310+
spin_unlock(&cfids->cfid_list_lock);
284311
goto oshr_free;
312+
}
285313
if (!smb2_validate_and_copy_iov(
286314
le16_to_cpu(qi_rsp->OutputBufferOffset),
287315
sizeof(struct smb2_file_all_info),
288316
&rsp_iov[1], sizeof(struct smb2_file_all_info),
289317
(char *)&cfid->file_all_info))
290318
cfid->file_all_info_is_valid = true;
291319

292-
if (!npath[0])
293-
dentry = dget(cifs_sb->root);
294-
else {
295-
dentry = path_to_dentry(cifs_sb, npath);
296-
if (IS_ERR(dentry)) {
297-
rc = -ENOENT;
298-
goto oshr_free;
299-
}
300-
}
301-
spin_lock(&cfids->cfid_list_lock);
302-
cfid->dentry = dentry;
303320
cfid->time = jiffies;
304-
cfid->has_lease = true;
305321
spin_unlock(&cfids->cfid_list_lock);
322+
/* At this point the directory handle is fully cached */
323+
rc = 0;
306324

307325
oshr_free:
308-
kfree(utf16_path);
309326
SMB2_open_free(&rqst[0]);
310327
SMB2_query_info_free(&rqst[1]);
311328
free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
312329
free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
313-
spin_lock(&cfids->cfid_list_lock);
314-
if (!cfid->has_lease) {
315-
if (rc) {
316-
if (cfid->on_list) {
317-
list_del(&cfid->entry);
318-
cfid->on_list = false;
319-
cfids->num_entries--;
320-
}
321-
rc = -ENOENT;
322-
} else {
330+
if (rc) {
331+
spin_lock(&cfids->cfid_list_lock);
332+
if (cfid->on_list) {
333+
list_del(&cfid->entry);
334+
cfid->on_list = false;
335+
cfids->num_entries--;
336+
}
337+
if (cfid->has_lease) {
323338
/*
324339
* We are guaranteed to have two references at this
325340
* point. One for the caller and one for a potential
326341
* lease. Release the Lease-ref so that the directory
327342
* will be closed when the caller closes the cached
328343
* handle.
329344
*/
345+
cfid->has_lease = false;
330346
spin_unlock(&cfids->cfid_list_lock);
331347
kref_put(&cfid->refcount, smb2_close_cached_fid);
332348
goto out;
333349
}
350+
spin_unlock(&cfids->cfid_list_lock);
334351
}
335-
spin_unlock(&cfids->cfid_list_lock);
352+
out:
336353
if (rc) {
337354
if (cfid->is_open)
338355
SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
339356
cfid->fid.volatile_fid);
340357
free_cached_dir(cfid);
341-
cfid = NULL;
342-
}
343-
out:
344-
if (rc == 0) {
358+
} else {
345359
*ret_cfid = cfid;
346360
atomic_inc(&tcon->num_remote_opens);
347361
}
348-
362+
kfree(utf16_path);
349363
return rc;
350364
}
351365

0 commit comments

Comments
 (0)