From: <gregkh@linuxfoundation.org>
To: pc@manguebit.com,stfrench@microsoft.com
Cc: <stable@vger.kernel.org>
Subject: FAILED: patch "[PATCH] smb: client: fix use-after-free in smb2_query_info_compound()" failed to apply to 6.1-stable tree
Date: Fri, 24 Nov 2023 11:54:51 +0000 [thread overview]
Message-ID: <2023112451-helpless-variable-cfb0@gregkh> (raw)
The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 5c86919455c1edec99ebd3338ad213b59271a71b
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2023112451-helpless-variable-cfb0@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
Possible dependencies:
5c86919455c1 ("smb: client: fix use-after-free in smb2_query_info_compound()")
81ba10959970 ("smb: client: prevent new fids from being removed by laundromat")
e95f3f744650 ("smb: client: make laundromat a delayed worker")
2da338ff752a ("smb3: do not start laundromat thread when dir leases disabled")
238b351d0935 ("smb3: allow controlling length of time directory entries are cached with dir leases")
d14de8067e3f ("cifs: Add a laundromat thread for cached directories")
38c8a9a52082 ("smb: move client and server files to common directory fs/smb")
be4fde79812f ("cifs: fix dentry lookups in directory handle cache")
df57109bd50b ("cifs: use tcon allocation functions even for dummy tcon")
8e843bf38f7b ("cifs: return a single-use cfid if we did not get a lease")
66d45ca1350a ("cifs: Check the lease context if we actually got a lease")
abdb1742a312 ("cifs: get rid of mount options string parsing")
9fd29a5bae6e ("cifs: use fs_context for automounts")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 5c86919455c1edec99ebd3338ad213b59271a71b Mon Sep 17 00:00:00 2001
From: Paulo Alcantara <pc@manguebit.com>
Date: Mon, 30 Oct 2023 17:19:56 -0300
Subject: [PATCH] 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>
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index fe1bf5b6e0cb..59f6b8e32cc9 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -32,7 +32,7 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
* fully cached or it may be in the process of
* being deleted due to a lease break.
*/
- if (!cfid->has_lease) {
+ if (!cfid->time || !cfid->has_lease) {
spin_unlock(&cfids->cfid_list_lock);
return NULL;
}
@@ -193,10 +193,20 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
npath = path_no_prefix(cifs_sb, path);
if (IS_ERR(npath)) {
rc = PTR_ERR(npath);
- kfree(utf16_path);
- return rc;
+ goto out;
}
+ if (!npath[0]) {
+ dentry = dget(cifs_sb->root);
+ } else {
+ dentry = path_to_dentry(cifs_sb, npath);
+ if (IS_ERR(dentry)) {
+ rc = -ENOENT;
+ goto out;
+ }
+ }
+ cfid->dentry = dentry;
+
/*
* We do not hold the lock for the open because in case
* SMB2_open needs to reconnect.
@@ -249,6 +259,15 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
smb2_set_related(&rqst[1]);
+ /*
+ * Set @cfid->has_lease to true before sending out compounded request so
+ * its lease reference can be put in cached_dir_lease_break() due to a
+ * potential lease break right after the request is sent or while @cfid
+ * is still being cached. Concurrent processes won't be to use it yet
+ * due to @cfid->time being zero.
+ */
+ cfid->has_lease = true;
+
rc = compound_send_recv(xid, ses, server,
flags, 2, rqst,
resp_buftype, rsp_iov);
@@ -263,6 +282,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
cfid->tcon = tcon;
cfid->is_open = true;
+ spin_lock(&cfids->cfid_list_lock);
+
o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
oparms.fid->persistent_fid = o_rsp->PersistentFileId;
oparms.fid->volatile_fid = o_rsp->VolatileFileId;
@@ -270,18 +291,25 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
#endif /* CIFS_DEBUG2 */
- if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE)
+ rc = -EINVAL;
+ if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
+ spin_unlock(&cfids->cfid_list_lock);
goto oshr_free;
+ }
smb2_parse_contexts(server, o_rsp,
&oparms.fid->epoch,
oparms.fid->lease_key, &oplock,
NULL, NULL);
- if (!(oplock & SMB2_LEASE_READ_CACHING_HE))
+ if (!(oplock & SMB2_LEASE_READ_CACHING_HE)) {
+ spin_unlock(&cfids->cfid_list_lock);
goto oshr_free;
+ }
qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
- if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info))
+ if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info)) {
+ spin_unlock(&cfids->cfid_list_lock);
goto oshr_free;
+ }
if (!smb2_validate_and_copy_iov(
le16_to_cpu(qi_rsp->OutputBufferOffset),
sizeof(struct smb2_file_all_info),
@@ -289,37 +317,24 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
(char *)&cfid->file_all_info))
cfid->file_all_info_is_valid = true;
- if (!npath[0])
- dentry = dget(cifs_sb->root);
- else {
- dentry = path_to_dentry(cifs_sb, npath);
- if (IS_ERR(dentry)) {
- rc = -ENOENT;
- goto oshr_free;
- }
- }
- spin_lock(&cfids->cfid_list_lock);
- cfid->dentry = dentry;
cfid->time = jiffies;
- cfid->has_lease = true;
spin_unlock(&cfids->cfid_list_lock);
+ /* At this point the directory handle is fully cached */
+ rc = 0;
oshr_free:
- kfree(utf16_path);
SMB2_open_free(&rqst[0]);
SMB2_query_info_free(&rqst[1]);
free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
- spin_lock(&cfids->cfid_list_lock);
- if (!cfid->has_lease) {
- if (rc) {
- if (cfid->on_list) {
- list_del(&cfid->entry);
- cfid->on_list = false;
- cfids->num_entries--;
- }
- rc = -ENOENT;
- } else {
+ if (rc) {
+ spin_lock(&cfids->cfid_list_lock);
+ if (cfid->on_list) {
+ list_del(&cfid->entry);
+ cfid->on_list = false;
+ cfids->num_entries--;
+ }
+ if (cfid->has_lease) {
/*
* We are guaranteed to have two references at this
* point. One for the caller and one for a potential
@@ -327,25 +342,24 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
* will be closed when the caller closes the cached
* handle.
*/
+ cfid->has_lease = false;
spin_unlock(&cfids->cfid_list_lock);
kref_put(&cfid->refcount, smb2_close_cached_fid);
goto out;
}
+ spin_unlock(&cfids->cfid_list_lock);
}
- spin_unlock(&cfids->cfid_list_lock);
+out:
if (rc) {
if (cfid->is_open)
SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
cfid->fid.volatile_fid);
free_cached_dir(cfid);
- cfid = NULL;
- }
-out:
- if (rc == 0) {
+ } else {
*ret_cfid = cfid;
atomic_inc(&tcon->num_remote_opens);
}
-
+ kfree(utf16_path);
return rc;
}
reply other threads:[~2023-11-24 11:54 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2023112451-helpless-variable-cfb0@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=pc@manguebit.com \
--cc=stable@vger.kernel.org \
--cc=stfrench@microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.