From patchwork Mon Nov 18 21:50:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Aurich X-Patchwork-Id: 13879116 Received: from o-chul.darkrain42.org (o-chul.darkrain42.org [74.207.241.14]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C4C3A1C07C4 for ; Mon, 18 Nov 2024 21:50:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.207.241.14 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731966641; cv=none; b=lqwcVhx8WZufQiAC/u0C/AT7IK9fluTmZPSfNOA59wD8ge9B7WQiHcGiGKpHN6CgYAM88nl7VO+7jfD7kPP93fcRSYl4BNKzW0AhpE8BGLb0QI4nPyXvnmptacC0LxzfStqywQyfuSLwJPbO02cxenXS+ACic08tymPWChCgCxo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731966641; c=relaxed/simple; bh=/yD78hB77bBOfeAbWiSXOxm0RR53Z14u59VoVYRcqfw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=qIq/hSu0Pgj7L/huv1m1Ji5um04IAif5CK46z9XjBR1U+u/NA6VfN7QCgxEzZhdwmD+ylf8OprVJdJbUMfMhZWe8tXT4kWup+3y8YiY5YquvOQoScAhT4StzgV1ePqfiUxA56BLg+bOh0ziH30wZYBoGpj28r51U+gGzcwAGxno= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=darkrain42.org; spf=pass smtp.mailfrom=darkrain42.org; dkim=permerror (0-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b=oEIAxeWa; dkim=pass (2048-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b=YxDZJWJP; arc=none smtp.client-ip=74.207.241.14 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=darkrain42.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=darkrain42.org Authentication-Results: smtp.subspace.kernel.org; dkim=permerror (0-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b="oEIAxeWa"; dkim=pass (2048-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b="YxDZJWJP" DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=darkrain42.org; i=@darkrain42.org; q=dns/txt; s=ed25519-2022-03; t=1731966633; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : from; bh=joibekbaBl8hAo29wZ3AXThwiTfyTHXyMEsSRMa6ttE=; b=oEIAxeWaea/eOKQ9ffxkBICguxIt1QEktYxTBW454fUNSwdzLEqSMTEog25GDmId5jlGn NNXsIWlXT5oRK2EBQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=darkrain42.org; i=@darkrain42.org; q=dns/txt; s=rsa-2022-03; t=1731966633; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : from; bh=joibekbaBl8hAo29wZ3AXThwiTfyTHXyMEsSRMa6ttE=; b=YxDZJWJPQeD31VRJtHiqrEkRGa0iPI4F+8+2rySpjJIBgpROMqOTSXnJRQKs7px4RDZcV qMy0sy4t5VlYaFtRjAqw91Z6iyJJP/s696mlEJNV8ht40vjM9/YmjAkOxTVz9lWgUIvPjCR 1agPfDw9oLB+4rzpIFaewoNmkkc9x2IrbD7N+AKLghNAqcfQcW87qxImwfjwLYpzsj9FGqh F39gZu534KIJZ+9Ca9CrTMdqlk0VNThhArW6T6q0XHbQevybJhiheiAYtqtLJHoAqno8SxF DJelxGDp9Qlyyq2hWTq6oAGTSjQzc0IVlqz9CFfFQj4WgbW9NUGsjmiURIPg== Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (Client did not present a certificate) by o-chul.darkrain42.org (Postfix) with ESMTPSA id 3C7C583B6; Mon, 18 Nov 2024 13:50:33 -0800 (PST) From: Paul Aurich To: linux-cifs@vger.kernel.org, Steve French Cc: paul@darkrain42.org, Paulo Alcantara , Ronnie Sahlberg , Shyam Prasad N , Tom Talpey , Bharath SM Subject: [PATCH v2 1/4] smb: cached directories can be more than root file handle Date: Mon, 18 Nov 2024 13:50:25 -0800 Message-ID: <20241118215028.1066662-2-paul@darkrain42.org> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241118215028.1066662-1-paul@darkrain42.org> References: <20241118215028.1066662-1-paul@darkrain42.org> Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Update this log message since cached fids may represent things other than the root of a mount. Fixes: e4029e072673 ("cifs: find and use the dentry for cached non-root directories also") Signed-off-by: Paul Aurich --- fs/smb/client/cached_dir.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c index 0ff2491c311d..585e1dc72432 100644 --- a/fs/smb/client/cached_dir.c +++ b/fs/smb/client/cached_dir.c @@ -399,11 +399,12 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon, return -ENOENT; spin_lock(&cfids->cfid_list_lock); list_for_each_entry(cfid, &cfids->entries, entry) { if (dentry && cfid->dentry == dentry) { - cifs_dbg(FYI, "found a cached root file handle by dentry\n"); + cifs_dbg(FYI, "found a cached file handle by dentry for %pd\n", + dentry); kref_get(&cfid->refcount); *ret_cfid = cfid; spin_unlock(&cfids->cfid_list_lock); return 0; } From patchwork Mon Nov 18 21:50:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Aurich X-Patchwork-Id: 13879119 Received: from o-chul.darkrain42.org (o-chul.darkrain42.org [74.207.241.14]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 17E96150981 for ; Mon, 18 Nov 2024 21:50:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.207.241.14 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731966642; cv=none; b=sNA/zwMJpxTDetsTQwXOVfEBtoAR7qnELpTXFRPGnRvXpz9mKMeEPG/p7SoWbcynhb/meYOtitrVTj4aKKYQS2mcbTt6+qJLWql9cjfg0vv+2kufxubgA0RK3VRaesByMDnkNwSPqaGt+iI5b2DGmj5lqrx8rhKgM2JEt5u7Gb8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731966642; c=relaxed/simple; bh=7pC8WWnGKllVk08rsivXIGIXn5W5VeUS81Y+LAEFC2A=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=WWTMx5pSHEE+pvAaUM8p+/aXdGHaz6fnOIRRP/VDMNDGBSlFgqxProvqoMxtbdOkCuKt5lR89nvYmNoSLIN0dYdAN9zvq1F0gc6pHME3xVDzKFfZsCww9AIjjY9T5PKzdxGX//N2akw2I6VZmeS1aFBi2/cg55eQNY0xkd5d/O4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=darkrain42.org; spf=pass smtp.mailfrom=darkrain42.org; dkim=permerror (0-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b=HPa9+boQ; dkim=pass (2048-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b=ek9Ij43w; arc=none smtp.client-ip=74.207.241.14 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=darkrain42.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=darkrain42.org Authentication-Results: smtp.subspace.kernel.org; dkim=permerror (0-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b="HPa9+boQ"; dkim=pass (2048-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b="ek9Ij43w" DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=darkrain42.org; i=@darkrain42.org; q=dns/txt; s=ed25519-2022-03; t=1731966633; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : from; bh=4X/4KUZHzHMJg5arCl99IYVtwmYw+uOMDXcdr9WHh/0=; b=HPa9+boQPZRSnEC5WCEx8eBYp6MLIv0iWXKIheJiA85IoyEWVNMjeRSzbcsWcAz3xTYkz qT+VdzgRtBa6oSsBw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=darkrain42.org; i=@darkrain42.org; q=dns/txt; s=rsa-2022-03; t=1731966633; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : from; bh=4X/4KUZHzHMJg5arCl99IYVtwmYw+uOMDXcdr9WHh/0=; b=ek9Ij43wRHvp+SMi/XJ8MJ4OnT8riKUCECD3ytl+PeUBZpDkA59w17uYjQJvHcZqTlbwR 2DJ+v90ad6TAQb+KUFqVZNMTYbM3LtFfd45PHzmjlgf0tQypn75H9LG1LFd0RjI6FJRpDy0 9cuo1tRs/FTIUAbS/L4fQwOl2bunmZCm8B1c+fEoBplgIi9RgHck4n6ucy8OUZKMAdLBwWm BhCJaMQ3UR/ZMzjGZoR516kbo/I4eItFojf99V6ENa84VA0aQEJGXRvABJaDVuT8gwii9Sj 3KgaBio2FCz3wXD2YHIbC7anevtzLBZHk0c6SjXzwWcpHtg1j6orsymP4eEw== Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (Client did not present a certificate) by o-chul.darkrain42.org (Postfix) with ESMTPSA id 9759583E7; Mon, 18 Nov 2024 13:50:33 -0800 (PST) From: Paul Aurich To: linux-cifs@vger.kernel.org, Steve French Cc: paul@darkrain42.org, Paulo Alcantara , Ronnie Sahlberg , Shyam Prasad N , Tom Talpey , Bharath SM Subject: [PATCH v2 2/4] smb: Don't leak cfid when reconnect races with open_cached_dir Date: Mon, 18 Nov 2024 13:50:26 -0800 Message-ID: <20241118215028.1066662-3-paul@darkrain42.org> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241118215028.1066662-1-paul@darkrain42.org> References: <20241118215028.1066662-1-paul@darkrain42.org> Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 open_cached_dir() may either race with the tcon reconnection even before compound_send_recv() or directly trigger a reconnection via SMB2_open_init() or SMB_query_info_init(). The reconnection process invokes invalidate_all_cached_dirs() via cifs_mark_open_files_invalid(), which removes all cfids from the cfids->entries list but doesn't drop a ref if has_lease isn't true. This results in the currently-being-constructed cfid not being on the list, but still having a refcount of 2. It leaks if returned from open_cached_dir(). Fix this by setting cfid->has_lease when the ref is actually taken; the cfid will not be used by other threads until it has a valid time. Addresses these kmemleaks: unreferenced object 0xffff8881090c4000 (size 1024): comm "bash", pid 1860, jiffies 4295126592 hex dump (first 32 bytes): 00 01 00 00 00 00 ad de 22 01 00 00 00 00 ad de ........"....... 00 ca 45 22 81 88 ff ff f8 dc 4f 04 81 88 ff ff ..E"......O..... backtrace (crc 6f58c20f): [] __kmalloc_cache_noprof+0x2be/0x350 [] open_cached_dir+0x993/0x1fb0 [] cifs_readdir+0x15a0/0x1d50 [] iterate_dir+0x28f/0x4b0 [] __x64_sys_getdents64+0xfd/0x200 [] do_syscall_64+0x95/0x1a0 [] entry_SYSCALL_64_after_hwframe+0x76/0x7e unreferenced object 0xffff8881044fdcf8 (size 8): comm "bash", pid 1860, jiffies 4295126592 hex dump (first 8 bytes): 00 cc cc cc cc cc cc cc ........ backtrace (crc 10c106a9): [] __kmalloc_node_track_caller_noprof+0x363/0x480 [] kstrdup+0x36/0x60 [] open_cached_dir+0x9b0/0x1fb0 [] cifs_readdir+0x15a0/0x1d50 [] iterate_dir+0x28f/0x4b0 [] __x64_sys_getdents64+0xfd/0x200 [] do_syscall_64+0x95/0x1a0 [] entry_SYSCALL_64_after_hwframe+0x76/0x7e And addresses these BUG splats when unmounting the SMB filesystem: BUG: Dentry ffff888140590ba0{i=1000000000080,n=/} still in use (2) [unmount of cifs cifs] WARNING: CPU: 3 PID: 3433 at fs/dcache.c:1536 umount_check+0xd0/0x100 Modules linked in: CPU: 3 UID: 0 PID: 3433 Comm: bash Not tainted 6.12.0-rc4-g850925a8133c-dirty #49 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020 RIP: 0010:umount_check+0xd0/0x100 Code: 8d 7c 24 40 e8 31 5a f4 ff 49 8b 54 24 40 41 56 49 89 e9 45 89 e8 48 89 d9 41 57 48 89 de 48 c7 c7 80 e7 db ac e8 f0 72 9a ff <0f> 0b 58 31 c0 5a 5b 5d 41 5c 41 5d 41 5e 41 5f e9 2b e5 5d 01 41 RSP: 0018:ffff88811cc27978 EFLAGS: 00010286 RAX: 0000000000000000 RBX: ffff888140590ba0 RCX: ffffffffaaf20bae RDX: dffffc0000000000 RSI: 0000000000000008 RDI: ffff8881f6fb6f40 RBP: ffff8881462ec000 R08: 0000000000000001 R09: ffffed1023984ee3 R10: ffff88811cc2771f R11: 00000000016cfcc0 R12: ffff888134383e08 R13: 0000000000000002 R14: ffff8881462ec668 R15: ffffffffaceab4c0 FS: 00007f23bfa98740(0000) GS:ffff8881f6f80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000556de4a6f808 CR3: 0000000123c80000 CR4: 0000000000350ef0 Call Trace: d_walk+0x6a/0x530 shrink_dcache_for_umount+0x6a/0x200 generic_shutdown_super+0x52/0x2a0 kill_anon_super+0x22/0x40 cifs_kill_sb+0x159/0x1e0 deactivate_locked_super+0x66/0xe0 cleanup_mnt+0x140/0x210 task_work_run+0xfb/0x170 syscall_exit_to_user_mode+0x29f/0x2b0 do_syscall_64+0xa1/0x1a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x7f23bfb93ae7 Code: ff ff ff ff c3 66 0f 1f 44 00 00 48 8b 0d 11 93 0d 00 f7 d8 64 89 01 b8 ff ff ff ff eb bf 0f 1f 44 00 00 b8 50 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e9 92 0d 00 f7 d8 64 89 01 48 RSP: 002b:00007ffee9138598 EFLAGS: 00000246 ORIG_RAX: 0000000000000050 RAX: 0000000000000000 RBX: 0000558f1803e9a0 RCX: 00007f23bfb93ae7 RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000558f1803e9a0 RBP: 0000558f1803e600 R08: 0000000000000007 R09: 0000558f17fab610 R10: d91d5ec34ab757b0 R11: 0000000000000246 R12: 0000000000000001 R13: 0000000000000000 R14: 0000000000000015 R15: 0000000000000000 irq event stamp: 1163486 hardirqs last enabled at (1163485): [] _raw_spin_unlock_irqrestore+0x34/0x60 hardirqs last disabled at (1163486): [] __schedule+0xc7c/0x19a0 softirqs last enabled at (1163482): [] __smb_send_rqst+0x3de/0x990 softirqs last disabled at (1163480): [] release_sock+0x21/0xf0 ---[ end trace 0000000000000000 ]--- VFS: Busy inodes after unmount of cifs (cifs) ------------[ cut here ]------------ kernel BUG at fs/super.c:661! Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI CPU: 1 UID: 0 PID: 3433 Comm: bash Tainted: G W 6.12.0-rc4-g850925a8133c-dirty #49 Tainted: [W]=WARN Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020 RIP: 0010:generic_shutdown_super+0x290/0x2a0 Code: e8 15 7c f7 ff 48 8b 5d 28 48 89 df e8 09 7c f7 ff 48 8b 0b 48 89 ee 48 8d 95 68 06 00 00 48 c7 c7 80 7f db ac e8 00 69 af ff <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 90 90 90 90 90 90 RSP: 0018:ffff88811cc27a50 EFLAGS: 00010246 RAX: 000000000000003e RBX: ffffffffae994420 RCX: 0000000000000027 RDX: 0000000000000000 RSI: ffffffffab06180e RDI: ffff8881f6eb18c8 RBP: ffff8881462ec000 R08: 0000000000000001 R09: ffffed103edd6319 R10: ffff8881f6eb18cb R11: 00000000016d3158 R12: ffff8881462ec9c0 R13: ffff8881462ec050 R14: 0000000000000001 R15: 0000000000000000 FS: 00007f23bfa98740(0000) GS:ffff8881f6e80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f8364005d68 CR3: 0000000123c80000 CR4: 0000000000350ef0 Call Trace: kill_anon_super+0x22/0x40 cifs_kill_sb+0x159/0x1e0 deactivate_locked_super+0x66/0xe0 cleanup_mnt+0x140/0x210 task_work_run+0xfb/0x170 syscall_exit_to_user_mode+0x29f/0x2b0 do_syscall_64+0xa1/0x1a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x7f23bfb93ae7 Modules linked in: ---[ end trace 0000000000000000 ]--- RIP: 0010:generic_shutdown_super+0x290/0x2a0 Code: e8 15 7c f7 ff 48 8b 5d 28 48 89 df e8 09 7c f7 ff 48 8b 0b 48 89 ee 48 8d 95 68 06 00 00 48 c7 c7 80 7f db ac e8 00 69 af ff <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 90 90 90 90 90 90 RSP: 0018:ffff88811cc27a50 EFLAGS: 00010246 RAX: 000000000000003e RBX: ffffffffae994420 RCX: 0000000000000027 RDX: 0000000000000000 RSI: ffffffffab06180e RDI: ffff8881f6eb18c8 RBP: ffff8881462ec000 R08: 0000000000000001 R09: ffffed103edd6319 R10: ffff8881f6eb18cb R11: 00000000016d3158 R12: ffff8881462ec9c0 R13: ffff8881462ec050 R14: 0000000000000001 R15: 0000000000000000 FS: 00007f23bfa98740(0000) GS:ffff8881f6e80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f8364005d68 CR3: 0000000123c80000 CR4: 0000000000350ef0 This reproduces eventually with an SMB mount and two shells running these loops concurrently - while true; do cd ~; sleep 1; for i in {1..3}; do cd /mnt/test/subdir; echo $PWD; sleep 1; cd ..; echo $PWD; sleep 1; done; echo ...; done - while true; do iptables -F OUTPUT; mount -t cifs -a; for _ in {0..2}; do ls /mnt/test/subdir/ | wc -l; done; iptables -I OUTPUT -p tcp --dport 445 -j DROP; sleep 10 echo "unmounting"; umount -l -t cifs -a; echo "done unmounting"; sleep 20 echo "recovering"; iptables -F OUTPUT; sleep 10; done Fixes: ebe98f1447bb ("cifs: enable caching of directories for which a lease is held") Fixes: 5c86919455c1 ("smb: client: fix use-after-free in smb2_query_info_compound()") Cc: stable@vger.kernel.org Signed-off-by: Paul Aurich --- fs/smb/client/cached_dir.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c index 585e1dc72432..59f07adf28d3 100644 --- a/fs/smb/client/cached_dir.c +++ b/fs/smb/client/cached_dir.c @@ -57,10 +57,20 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids, cfid->cfids = cfids; cfids->num_entries++; list_add(&cfid->entry, &cfids->entries); cfid->on_list = true; kref_get(&cfid->refcount); + /* + * Set @cfid->has_lease to true during construction so that the 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, or if a reconnection is triggered during construction. + * Concurrent processes won't be to use it yet due to @cfid->time being + * zero. + */ + cfid->has_lease = true; + spin_unlock(&cfids->cfid_list_lock); return cfid; } static struct dentry * @@ -174,16 +184,16 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, if (cfid == NULL) { kfree(utf16_path); return -ENOENT; } /* - * Return cached fid if it has a lease. Otherwise, it is either a new - * entry or laundromat worker removed it from @cfids->entries. Caller - * will put last reference if the latter. + * Return cached fid if it is valid (has a lease and has a time). + * Otherwise, it is either a new entry or laundromat worker removed it + * from @cfids->entries. Caller will put last reference if the latter. */ spin_lock(&cfids->cfid_list_lock); - if (cfid->has_lease) { + if (cfid->has_lease && cfid->time) { spin_unlock(&cfids->cfid_list_lock); *ret_cfid = cfid; kfree(utf16_path); return 0; } @@ -265,19 +275,10 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, if (rc) goto oshr_free; 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; - if (retries) { smb2_set_replay(server, &rqst[0]); smb2_set_replay(server, &rqst[1]); } From patchwork Mon Nov 18 21:50:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Aurich X-Patchwork-Id: 13879118 Received: from o-chul.darkrain42.org (o-chul.darkrain42.org [74.207.241.14]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 17F021D0DDE for ; Mon, 18 Nov 2024 21:50:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.207.241.14 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731966642; cv=none; b=sLkRzAgF7l3YdDXHMdjWw5A1Da5AElp6EImuQRZ7X8vSxGhvH6Gi4/+cxmFVhaOf+/YVWLhWXenbZYCQ+pIPSSfzMUyDgyiQ4zeVNqFOL/WqUUxYVoyBY8Jsu4+fN1Jdeg8kRe2p9g9R+n0H96Uk85ZZfb6pxDtxZs8xd2v0TS4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731966642; c=relaxed/simple; bh=PYUls6ZOAgq1NVTNzW5rDo3EMMbxeWuExBK0J/5TxUo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=J6mfcRtSe7UJE8rc2RUd03tNiG/ftHd/EwvUGlcngtsyZvh6WP6Hp1z2An1aO+kqIx7l7tfTcw4bMBNSWL34ubWKZfDjglP4PbK+2nymQuXeu4DYAbn4DDEJV/7eSbKMBUP4w0xl4rs7F7R4fTFao1u2ohG7GJh15JdLZ7YIPZ0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=darkrain42.org; spf=pass smtp.mailfrom=darkrain42.org; dkim=permerror (0-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b=8RIhMW5e; dkim=pass (2048-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b=b10fGLES; arc=none smtp.client-ip=74.207.241.14 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=darkrain42.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=darkrain42.org Authentication-Results: smtp.subspace.kernel.org; dkim=permerror (0-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b="8RIhMW5e"; dkim=pass (2048-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b="b10fGLES" DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=darkrain42.org; i=@darkrain42.org; q=dns/txt; s=ed25519-2022-03; t=1731966634; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : from; bh=ntZCXwECanqArYbkt3PVEoUdjLsokkBLqN3RUs8tW10=; b=8RIhMW5efqUyqI2av5jzFZeB6BGYC3ObbsumTUZXrrQC0Fhhebn+mjxBUywnT+mdlB0lw OlLeo/4sAJ1RWcICA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=darkrain42.org; i=@darkrain42.org; q=dns/txt; s=rsa-2022-03; t=1731966634; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : from; bh=ntZCXwECanqArYbkt3PVEoUdjLsokkBLqN3RUs8tW10=; b=b10fGLESlQPt73DeQf0w2eJonO5u/+0JYG0wdsgKdvNvI5/HeMRmuTphbRpR9S05He5iv s8umUh8qwwmBgzq2dF9Ym8JnerF9FDVEJGBBWMN7p7BuMSqDFeEpz9MJtGtSb/JXQSahoST 6klIYeuv+meJCB0YpiqiZoM2WYTRcfmn7AEEW0s1lfPVNOJDL2JSWoVA75OvNhiPow8Mzz+ O/spBQP2cU/xrDXTlIXadkV0mwzQQOx0NPXGQSD+BlTZld+Q7BXIJLNjZrEwcyCJ6fhldxp U9xcDnONwaeHSS1qh6WixpJOpcIwJQ5AUrbaGmL8Vluc4rD+o4FP3ghvzCVQ== Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (Client did not present a certificate) by o-chul.darkrain42.org (Postfix) with ESMTPSA id 05A5683E8; Mon, 18 Nov 2024 13:50:33 -0800 (PST) From: Paul Aurich To: linux-cifs@vger.kernel.org, Steve French Cc: paul@darkrain42.org, Paulo Alcantara , Ronnie Sahlberg , Shyam Prasad N , Tom Talpey , Bharath SM Subject: [PATCH v2 3/4] smb: prevent use-after-free due to open_cached_dir error paths Date: Mon, 18 Nov 2024 13:50:27 -0800 Message-ID: <20241118215028.1066662-4-paul@darkrain42.org> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241118215028.1066662-1-paul@darkrain42.org> References: <20241118215028.1066662-1-paul@darkrain42.org> Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 If open_cached_dir() encounters an error parsing the lease from the server, the error handling may race with receiving a lease break, resulting in open_cached_dir() freeing the cfid while the queued work is pending. Update open_cached_dir() to drop refs rather than directly freeing the cfid. Have cached_dir_lease_break(), cfids_laundromat_worker(), and invalidate_all_cached_dirs() clear has_lease immediately while still holding cfids->cfid_list_lock, and then use this to also simplify the reference counting in cfids_laundromat_worker() and invalidate_all_cached_dirs(). Fixes this KASAN splat (which manually injects an error and lease break in open_cached_dir()): ================================================================== BUG: KASAN: slab-use-after-free in smb2_cached_lease_break+0x27/0xb0 Read of size 8 at addr ffff88811cc24c10 by task kworker/3:1/65 CPU: 3 UID: 0 PID: 65 Comm: kworker/3:1 Not tainted 6.12.0-rc6-g255cf264e6e5-dirty #87 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020 Workqueue: cifsiod smb2_cached_lease_break Call Trace: dump_stack_lvl+0x77/0xb0 print_report+0xce/0x660 kasan_report+0xd3/0x110 smb2_cached_lease_break+0x27/0xb0 process_one_work+0x50a/0xc50 worker_thread+0x2ba/0x530 kthread+0x17c/0x1c0 ret_from_fork+0x34/0x60 ret_from_fork_asm+0x1a/0x30 Allocated by task 2464: kasan_save_stack+0x33/0x60 kasan_save_track+0x14/0x30 __kasan_kmalloc+0xaa/0xb0 open_cached_dir+0xa7d/0x1fb0 smb2_query_path_info+0x43c/0x6e0 cifs_get_fattr+0x346/0xf10 cifs_get_inode_info+0x157/0x210 cifs_revalidate_dentry_attr+0x2d1/0x460 cifs_getattr+0x173/0x470 vfs_statx_path+0x10f/0x160 vfs_statx+0xe9/0x150 vfs_fstatat+0x5e/0xc0 __do_sys_newfstatat+0x91/0xf0 do_syscall_64+0x95/0x1a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e Freed by task 2464: kasan_save_stack+0x33/0x60 kasan_save_track+0x14/0x30 kasan_save_free_info+0x3b/0x60 __kasan_slab_free+0x51/0x70 kfree+0x174/0x520 open_cached_dir+0x97f/0x1fb0 smb2_query_path_info+0x43c/0x6e0 cifs_get_fattr+0x346/0xf10 cifs_get_inode_info+0x157/0x210 cifs_revalidate_dentry_attr+0x2d1/0x460 cifs_getattr+0x173/0x470 vfs_statx_path+0x10f/0x160 vfs_statx+0xe9/0x150 vfs_fstatat+0x5e/0xc0 __do_sys_newfstatat+0x91/0xf0 do_syscall_64+0x95/0x1a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e Last potentially related work creation: kasan_save_stack+0x33/0x60 __kasan_record_aux_stack+0xad/0xc0 insert_work+0x32/0x100 __queue_work+0x5c9/0x870 queue_work_on+0x82/0x90 open_cached_dir+0x1369/0x1fb0 smb2_query_path_info+0x43c/0x6e0 cifs_get_fattr+0x346/0xf10 cifs_get_inode_info+0x157/0x210 cifs_revalidate_dentry_attr+0x2d1/0x460 cifs_getattr+0x173/0x470 vfs_statx_path+0x10f/0x160 vfs_statx+0xe9/0x150 vfs_fstatat+0x5e/0xc0 __do_sys_newfstatat+0x91/0xf0 do_syscall_64+0x95/0x1a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e The buggy address belongs to the object at ffff88811cc24c00 which belongs to the cache kmalloc-1k of size 1024 The buggy address is located 16 bytes inside of freed 1024-byte region [ffff88811cc24c00, ffff88811cc25000) Cc: stable@vger.kernel.org Signed-off-by: Paul Aurich --- fs/smb/client/cached_dir.c | 70 ++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 41 deletions(-) diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c index 59f07adf28d3..64c67cbb2aa5 100644 --- a/fs/smb/client/cached_dir.c +++ b/fs/smb/client/cached_dir.c @@ -346,10 +346,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, oshr_free: 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); +out: if (rc) { spin_lock(&cfids->cfid_list_lock); if (cfid->on_list) { list_del(&cfid->entry); cfid->on_list = false; @@ -357,27 +358,18 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, } if (cfid->has_lease) { /* * We are guaranteed to have two references at this * point. One for the caller and one for a potential - * lease. Release the Lease-ref so that the directory - * will be closed when the caller closes the cached - * handle. + * lease. Release one here, and the second below. */ 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); - } -out: - if (rc) { - if (cfid->is_open) - SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid, - cfid->fid.volatile_fid); - free_cached_dir(cfid); + + kref_put(&cfid->refcount, smb2_close_cached_fid); } else { *ret_cfid = cfid; atomic_inc(&tcon->num_remote_opens); } kfree(utf16_path); @@ -512,42 +504,38 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon) list_for_each_entry_safe(cfid, q, &cfids->entries, entry) { list_move(&cfid->entry, &entry); cfids->num_entries--; cfid->is_open = false; cfid->on_list = false; - /* To prevent race with smb2_cached_lease_break() */ - kref_get(&cfid->refcount); + if (cfid->has_lease) { + /* + * The lease was never cancelled from the server, + * so steal that reference. + */ + cfid->has_lease = false; + } else + kref_get(&cfid->refcount); } spin_unlock(&cfids->cfid_list_lock); list_for_each_entry_safe(cfid, q, &entry, entry) { list_del(&cfid->entry); cancel_work_sync(&cfid->lease_break); - if (cfid->has_lease) { - /* - * We lease was never cancelled from the server so we - * need to drop the reference. - */ - spin_lock(&cfids->cfid_list_lock); - cfid->has_lease = false; - spin_unlock(&cfids->cfid_list_lock); - kref_put(&cfid->refcount, smb2_close_cached_fid); - } - /* Drop the extra reference opened above*/ + /* + * Drop the ref-count from above, either the lease-ref (if there + * was one) or the extra one acquired. + */ kref_put(&cfid->refcount, smb2_close_cached_fid); } } static void smb2_cached_lease_break(struct work_struct *work) { struct cached_fid *cfid = container_of(work, struct cached_fid, lease_break); - spin_lock(&cfid->cfids->cfid_list_lock); - cfid->has_lease = false; - spin_unlock(&cfid->cfids->cfid_list_lock); kref_put(&cfid->refcount, smb2_close_cached_fid); } int cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]) { @@ -561,10 +549,11 @@ int cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]) list_for_each_entry(cfid, &cfids->entries, entry) { if (cfid->has_lease && !memcmp(lease_key, cfid->fid.lease_key, SMB2_LEASE_KEY_SIZE)) { + cfid->has_lease = false; cfid->time = 0; /* * We found a lease remove it from the list * so no threads can access it. */ @@ -638,12 +627,18 @@ static void cfids_laundromat_worker(struct work_struct *work) if (cfid->time && time_after(jiffies, cfid->time + HZ * dir_cache_timeout)) { cfid->on_list = false; list_move(&cfid->entry, &entry); cfids->num_entries--; - /* To prevent race with smb2_cached_lease_break() */ - kref_get(&cfid->refcount); + if (cfid->has_lease) { + /* + * Our lease has not yet been cancelled from the + * server. Steal that reference. + */ + cfid->has_lease = false; + } else + kref_get(&cfid->refcount); } } spin_unlock(&cfids->cfid_list_lock); list_for_each_entry_safe(cfid, q, &entry, entry) { @@ -651,21 +646,14 @@ static void cfids_laundromat_worker(struct work_struct *work) /* * Cancel and wait for the work to finish in case we are racing * with it. */ cancel_work_sync(&cfid->lease_break); - if (cfid->has_lease) { - /* - * Our lease has not yet been cancelled from the server - * so we need to drop the reference. - */ - spin_lock(&cfids->cfid_list_lock); - cfid->has_lease = false; - spin_unlock(&cfids->cfid_list_lock); - kref_put(&cfid->refcount, smb2_close_cached_fid); - } - /* Drop the extra reference opened above */ + /* + * Drop the ref-count from above, either the lease-ref (if there + * was one) or the extra one acquired. + */ kref_put(&cfid->refcount, smb2_close_cached_fid); } queue_delayed_work(cifsiod_wq, &cfids->laundromat_work, dir_cache_timeout * HZ); } From patchwork Mon Nov 18 21:50:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Aurich X-Patchwork-Id: 13879120 Received: from o-chul.darkrain42.org (o-chul.darkrain42.org [74.207.241.14]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4489C1E5728 for ; Mon, 18 Nov 2024 21:50:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.207.241.14 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731966643; cv=none; b=tTUzfmEv+kWSRkelEX9NRqLglra6MeIkdAWqam+CutBCgyeMt6YbraIuATl+3F2Lwg4rDjDAP5nuQ05f+JT31+4st73aTs7Kd/e/9nWqjbR/xWHBg5num7QsUvbTJpOzbUkrNEYKK3Kjyf9jd1UxIWXOdyJxi22ccKQRX8uLU60= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1731966643; c=relaxed/simple; bh=8yO9Ev99EH50oUlNu3HfOCCCoKvTIkPSOqcuUIsZM4c=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=BKxo9ZiAMB/0DoZRwzM1BSEoI/DbI5e0O5SxWvbEiNaJIkSlNzIalrSULz/3A/n4pjYU3Knq78gOEzknl+3W7Wg5Y/W0raPoKpsdF7t/YENl+6FTlSHOZQtbCcm9cmIU2ld7+WfSRaArgXtAbikkDd6htVABEJvbPOr8J6f6Cy4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=darkrain42.org; spf=pass smtp.mailfrom=darkrain42.org; dkim=permerror (0-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b=FLn2825t; dkim=pass (2048-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b=kafMl57A; arc=none smtp.client-ip=74.207.241.14 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=darkrain42.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=darkrain42.org Authentication-Results: smtp.subspace.kernel.org; dkim=permerror (0-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b="FLn2825t"; dkim=pass (2048-bit key) header.d=darkrain42.org header.i=@darkrain42.org header.b="kafMl57A" DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=darkrain42.org; i=@darkrain42.org; q=dns/txt; s=ed25519-2022-03; t=1731966634; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : from; bh=NMWm5/p0DfcEsSv9jNuO6Hj3Zh7kCcKqqAkkM9DzaKo=; b=FLn2825tq/KCg0QOZTk+p/zpkJXXrX8cNARRvHwfJgVtX8aC2CM0W17oT0Yt/ykWXxOTc MVfrmabOx4LqqZrCg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=darkrain42.org; i=@darkrain42.org; q=dns/txt; s=rsa-2022-03; t=1731966634; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : from; bh=NMWm5/p0DfcEsSv9jNuO6Hj3Zh7kCcKqqAkkM9DzaKo=; b=kafMl57A1K0ek3gLhO9y+UKERUNl6aUFLmcytRg+VIJKGtO9QrHkuIPrxWDlzv73+GM1Q 6UExVHCAGJZ6whvQMXLnsrNZ3d/mbT/oqBBysyfsHn1WzWQrYMAxo6HM1ja2whlk5BxfZfD CYXdiJjqaS0O+HViJdqxsn8ZyYPvP2MHgxsyz9HH3kwzXA2J6MOBBtH8qV/qo3JB9rP36Xi tR6QNLjIVC19cfWqoR7GNXzOB7f0g4EPzUTRUMAIaTTwJ+WhjIayVBz+KJPhzfaVdF2AvUY RoE5hWRJUcDEQt9KsqUvLtj/6BDRiYk9TYAgUEPJilQy0wNbSd7p3ORZIyvA== Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (Client did not present a certificate) by o-chul.darkrain42.org (Postfix) with ESMTPSA id 60BE583E9; Mon, 18 Nov 2024 13:50:34 -0800 (PST) From: Paul Aurich To: linux-cifs@vger.kernel.org, Steve French Cc: paul@darkrain42.org, Paulo Alcantara , Ronnie Sahlberg , Shyam Prasad N , Tom Talpey , Bharath SM Subject: [PATCH v2 4/4] smb: During unmount, ensure all cached dir instances drop their dentry Date: Mon, 18 Nov 2024 13:50:28 -0800 Message-ID: <20241118215028.1066662-5-paul@darkrain42.org> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241118215028.1066662-1-paul@darkrain42.org> References: <20241118215028.1066662-1-paul@darkrain42.org> Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The unmount process (cifs_kill_sb() calling close_all_cached_dirs()) can race with various cached directory operations, which ultimately results in dentries not being dropped and these kernel BUGs: BUG: Dentry ffff88814f37e358{i=1000000000080,n=/} still in use (2) [unmount of cifs cifs] VFS: Busy inodes after unmount of cifs (cifs) ------------[ cut here ]------------ kernel BUG at fs/super.c:661! This happens when a cfid is in the process of being cleaned up when, and has been removed from the cfids->entries list, including: - Receiving a lease break from the server - Server reconnection triggers invalidate_all_cached_dirs(), which removes all the cfids from the list - The laundromat thread decides to expire an old cfid. To solve these problems, dropping the dentry is done in queued work done in a newly-added cfid_put_wq workqueue, and close_all_cached_dirs() flushes that workqueue after it drops all the dentries of which it's aware. This is a global workqueue (rather than scoped to a mount), but the queued work is minimal. The final cleanup work for cleaning up a cfid is performed via work queued in the serverclose_wq workqueue; this is done separate from dropping the dentries so that close_all_cached_dirs() doesn't block on any server operations. Both of these queued works expect to invoked with a cfid reference and a tcon reference to avoid those objects from being freed while the work is ongoing. While we're here, add proper locking to close_all_cached_dirs(), and locking around the freeing of cfid->dentry. Fixes: ebe98f1447bb ("cifs: enable caching of directories for which a lease is held") Cc: stable@vger.kernel.org Signed-off-by: Paul Aurich --- fs/smb/client/cached_dir.c | 156 ++++++++++++++++++++++++++++++------- fs/smb/client/cached_dir.h | 6 +- fs/smb/client/cifsfs.c | 14 +++- fs/smb/client/cifsglob.h | 3 +- fs/smb/client/inode.c | 3 - fs/smb/client/trace.h | 3 + 6 files changed, 148 insertions(+), 37 deletions(-) diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c index 64c67cbb2aa5..8fb95f4347df 100644 --- a/fs/smb/client/cached_dir.c +++ b/fs/smb/client/cached_dir.c @@ -15,10 +15,15 @@ static struct cached_fid *init_cached_dir(const char *path); static void free_cached_dir(struct cached_fid *cfid); static void smb2_close_cached_fid(struct kref *ref); static void cfids_laundromat_worker(struct work_struct *work); +struct cached_dir_dentry { + struct list_head entry; + struct dentry *dentry; +}; + static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids, const char *path, bool lookup_only, __u32 max_cached_dirs) { @@ -469,42 +474,68 @@ void close_all_cached_dirs(struct cifs_sb_info *cifs_sb) struct rb_node *node; struct cached_fid *cfid; struct cifs_tcon *tcon; struct tcon_link *tlink; struct cached_fids *cfids; + struct cached_dir_dentry *tmp_list, *q; + LIST_HEAD(entry); + spin_lock(&cifs_sb->tlink_tree_lock); for (node = rb_first(root); node; node = rb_next(node)) { tlink = rb_entry(node, struct tcon_link, tl_rbnode); tcon = tlink_tcon(tlink); if (IS_ERR(tcon)) continue; cfids = tcon->cfids; if (cfids == NULL) continue; + spin_lock(&cfids->cfid_list_lock); list_for_each_entry(cfid, &cfids->entries, entry) { - dput(cfid->dentry); + tmp_list = kmalloc(sizeof(*tmp_list), GFP_ATOMIC); + if (tmp_list == NULL) + break; + spin_lock(&cfid->fid_lock); + tmp_list->dentry = cfid->dentry; cfid->dentry = NULL; + spin_unlock(&cfid->fid_lock); + + list_add_tail(&tmp_list->entry, &entry); } + spin_unlock(&cfids->cfid_list_lock); } + spin_unlock(&cifs_sb->tlink_tree_lock); + + list_for_each_entry_safe(tmp_list, q, &entry, entry) { + list_del(&tmp_list->entry); + dput(tmp_list->dentry); + kfree(tmp_list); + } + + /* Flush any pending work that will drop dentries */ + flush_workqueue(cfid_put_wq); } /* * Invalidate all cached dirs when a TCON has been reset * due to a session loss. */ void invalidate_all_cached_dirs(struct cifs_tcon *tcon) { struct cached_fids *cfids = tcon->cfids; struct cached_fid *cfid, *q; - LIST_HEAD(entry); if (cfids == NULL) return; + /* + * Mark all the cfids as closed, and move them to the cfids->dying list. + * They'll be cleaned up later by cfids_invalidation_worker. Take + * a reference to each cfid during this process. + */ spin_lock(&cfids->cfid_list_lock); list_for_each_entry_safe(cfid, q, &cfids->entries, entry) { - list_move(&cfid->entry, &entry); + list_move(&cfid->entry, &cfids->dying); cfids->num_entries--; cfid->is_open = false; cfid->on_list = false; if (cfid->has_lease) { /* @@ -513,30 +544,51 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon) */ cfid->has_lease = false; } else kref_get(&cfid->refcount); } + /* + * Queue dropping of the dentries once locks have been dropped + */ + if (!list_empty(&cfids->dying)) + queue_work(cfid_put_wq, &cfids->invalidation_work); spin_unlock(&cfids->cfid_list_lock); - - list_for_each_entry_safe(cfid, q, &entry, entry) { - list_del(&cfid->entry); - cancel_work_sync(&cfid->lease_break); - /* - * Drop the ref-count from above, either the lease-ref (if there - * was one) or the extra one acquired. - */ - kref_put(&cfid->refcount, smb2_close_cached_fid); - } } static void -smb2_cached_lease_break(struct work_struct *work) +cached_dir_offload_close(struct work_struct *work) { struct cached_fid *cfid = container_of(work, - struct cached_fid, lease_break); + struct cached_fid, close_work); + struct cifs_tcon *tcon = cfid->tcon; + + WARN_ON(cfid->on_list); kref_put(&cfid->refcount, smb2_close_cached_fid); + cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_cached_close); +} + +/* + * Release the cached directory's dentry, and then queue work to drop cached + * directory itself (closing on server if needed). + * + * Must be called with a reference to the cached_fid and a reference to the + * tcon. + */ +static void cached_dir_put_work(struct work_struct *work) +{ + struct cached_fid *cfid = container_of(work, struct cached_fid, + put_work); + struct dentry *dentry; + + spin_lock(&cfid->fid_lock); + dentry = cfid->dentry; + cfid->dentry = NULL; + spin_unlock(&cfid->fid_lock); + + dput(dentry); + queue_work(serverclose_wq, &cfid->close_work); } int cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]) { struct cached_fids *cfids = tcon->cfids; @@ -559,12 +611,14 @@ int cached_dir_lease_break(struct cifs_tcon *tcon, __u8 lease_key[16]) */ list_del(&cfid->entry); cfid->on_list = false; cfids->num_entries--; - queue_work(cifsiod_wq, - &cfid->lease_break); + ++tcon->tc_count; + trace_smb3_tcon_ref(tcon->debug_id, tcon->tc_count, + netfs_trace_tcon_ref_get_cached_lease_break); + queue_work(cfid_put_wq, &cfid->put_work); spin_unlock(&cfids->cfid_list_lock); return true; } } spin_unlock(&cfids->cfid_list_lock); @@ -582,11 +636,12 @@ static struct cached_fid *init_cached_dir(const char *path) if (!cfid->path) { kfree(cfid); return NULL; } - INIT_WORK(&cfid->lease_break, smb2_cached_lease_break); + INIT_WORK(&cfid->close_work, cached_dir_offload_close); + INIT_WORK(&cfid->put_work, cached_dir_put_work); INIT_LIST_HEAD(&cfid->entry); INIT_LIST_HEAD(&cfid->dirents.entries); mutex_init(&cfid->dirents.de_mutex); spin_lock_init(&cfid->fid_lock); kref_init(&cfid->refcount); @@ -595,10 +650,13 @@ static struct cached_fid *init_cached_dir(const char *path) static void free_cached_dir(struct cached_fid *cfid) { struct cached_dirent *dirent, *q; + WARN_ON(work_pending(&cfid->close_work)); + WARN_ON(work_pending(&cfid->put_work)); + dput(cfid->dentry); cfid->dentry = NULL; /* * Delete all cached dirent names @@ -612,14 +670,34 @@ static void free_cached_dir(struct cached_fid *cfid) kfree(cfid->path); cfid->path = NULL; kfree(cfid); } +static void cfids_invalidation_worker(struct work_struct *work) +{ + struct cached_fids *cfids = container_of(work, struct cached_fids, + invalidation_work); + struct cached_fid *cfid, *q; + LIST_HEAD(entry); + + spin_lock(&cfids->cfid_list_lock); + /* move cfids->dying to the local list */ + list_cut_before(&entry, &cfids->dying, &cfids->dying); + spin_unlock(&cfids->cfid_list_lock); + + list_for_each_entry_safe(cfid, q, &entry, entry) { + list_del(&cfid->entry); + /* Drop the ref-count acquired in invalidate_all_cached_dirs */ + kref_put(&cfid->refcount, smb2_close_cached_fid); + } +} + static void cfids_laundromat_worker(struct work_struct *work) { struct cached_fids *cfids; struct cached_fid *cfid, *q; + struct dentry *dentry; LIST_HEAD(entry); cfids = container_of(work, struct cached_fids, laundromat_work.work); spin_lock(&cfids->cfid_list_lock); @@ -641,22 +719,32 @@ static void cfids_laundromat_worker(struct work_struct *work) } spin_unlock(&cfids->cfid_list_lock); list_for_each_entry_safe(cfid, q, &entry, entry) { list_del(&cfid->entry); - /* - * Cancel and wait for the work to finish in case we are racing - * with it. - */ - cancel_work_sync(&cfid->lease_break); - /* - * Drop the ref-count from above, either the lease-ref (if there - * was one) or the extra one acquired. - */ - kref_put(&cfid->refcount, smb2_close_cached_fid); + + spin_lock(&cfid->fid_lock); + dentry = cfid->dentry; + cfid->dentry = NULL; + spin_unlock(&cfid->fid_lock); + + dput(dentry); + if (cfid->is_open) { + spin_lock(&cifs_tcp_ses_lock); + ++cfid->tcon->tc_count; + trace_smb3_tcon_ref(cfid->tcon->debug_id, cfid->tcon->tc_count, + netfs_trace_tcon_ref_get_cached_laundromat); + spin_unlock(&cifs_tcp_ses_lock); + queue_work(serverclose_wq, &cfid->close_work); + } else + /* + * Drop the ref-count from above, either the lease-ref (if there + * was one) or the extra one acquired. + */ + kref_put(&cfid->refcount, smb2_close_cached_fid); } - queue_delayed_work(cifsiod_wq, &cfids->laundromat_work, + queue_delayed_work(cfid_put_wq, &cfids->laundromat_work, dir_cache_timeout * HZ); } struct cached_fids *init_cached_dirs(void) { @@ -665,13 +753,15 @@ struct cached_fids *init_cached_dirs(void) cfids = kzalloc(sizeof(*cfids), GFP_KERNEL); if (!cfids) return NULL; spin_lock_init(&cfids->cfid_list_lock); INIT_LIST_HEAD(&cfids->entries); + INIT_LIST_HEAD(&cfids->dying); + INIT_WORK(&cfids->invalidation_work, cfids_invalidation_worker); INIT_DELAYED_WORK(&cfids->laundromat_work, cfids_laundromat_worker); - queue_delayed_work(cifsiod_wq, &cfids->laundromat_work, + queue_delayed_work(cfid_put_wq, &cfids->laundromat_work, dir_cache_timeout * HZ); return cfids; } @@ -686,17 +776,23 @@ void free_cached_dirs(struct cached_fids *cfids) if (cfids == NULL) return; cancel_delayed_work_sync(&cfids->laundromat_work); + cancel_work_sync(&cfids->invalidation_work); spin_lock(&cfids->cfid_list_lock); list_for_each_entry_safe(cfid, q, &cfids->entries, entry) { cfid->on_list = false; cfid->is_open = false; list_move(&cfid->entry, &entry); } + list_for_each_entry_safe(cfid, q, &cfids->dying, entry) { + cfid->on_list = false; + cfid->is_open = false; + list_move(&cfid->entry, &entry); + } spin_unlock(&cfids->cfid_list_lock); list_for_each_entry_safe(cfid, q, &entry, entry) { list_del(&cfid->entry); free_cached_dir(cfid); diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h index 81ba0fd5cc16..1dfe79d947a6 100644 --- a/fs/smb/client/cached_dir.h +++ b/fs/smb/client/cached_dir.h @@ -42,23 +42,27 @@ struct cached_fid { struct kref refcount; struct cifs_fid fid; spinlock_t fid_lock; struct cifs_tcon *tcon; struct dentry *dentry; - struct work_struct lease_break; + struct work_struct put_work; + struct work_struct close_work; struct smb2_file_all_info file_all_info; struct cached_dirents dirents; }; /* default MAX_CACHED_FIDS is 16 */ struct cached_fids { /* Must be held when: * - accessing the cfids->entries list + * - accessing the cfids->dying list */ spinlock_t cfid_list_lock; int num_entries; struct list_head entries; + struct list_head dying; + struct work_struct invalidation_work; struct delayed_work laundromat_work; }; extern struct cached_fids *init_cached_dirs(void); extern void free_cached_dirs(struct cached_fids *cfids); diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c index 20cafdff5081..bf909c2f6b96 100644 --- a/fs/smb/client/cifsfs.c +++ b/fs/smb/client/cifsfs.c @@ -155,10 +155,11 @@ struct workqueue_struct *cifsiod_wq; struct workqueue_struct *decrypt_wq; struct workqueue_struct *fileinfo_put_wq; struct workqueue_struct *cifsoplockd_wq; struct workqueue_struct *deferredclose_wq; struct workqueue_struct *serverclose_wq; +struct workqueue_struct *cfid_put_wq; __u32 cifs_lock_secret; /* * Bumps refcount for cifs super block. * Note that it should be only called if a reference to VFS super block is @@ -1893,13 +1894,20 @@ init_cifs(void) if (!serverclose_wq) { rc = -ENOMEM; goto out_destroy_deferredclose_wq; } - rc = cifs_init_inodecache(); - if (rc) + cfid_put_wq = alloc_workqueue("cfid_put_wq", + WQ_FREEZABLE|WQ_MEM_RECLAIM, 0); + if (!cfid_put_wq) { + rc = -ENOMEM; goto out_destroy_serverclose_wq; + } + + rc = cifs_init_inodecache(); + if (rc) + goto out_destroy_cfid_put_wq; rc = cifs_init_netfs(); if (rc) goto out_destroy_inodecache; @@ -1963,10 +1971,12 @@ init_cifs(void) destroy_mids(); out_destroy_netfs: cifs_destroy_netfs(); out_destroy_inodecache: cifs_destroy_inodecache(); +out_destroy_cfid_put_wq: + destroy_workqueue(cfid_put_wq); out_destroy_serverclose_wq: destroy_workqueue(serverclose_wq); out_destroy_deferredclose_wq: destroy_workqueue(deferredclose_wq); out_destroy_cifsoplockd_wq: diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 5041b1ffc244..31ea19e7b998 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -1981,11 +1981,11 @@ require use of the stronger protocol */ * cifsInodeInfo->open_file_lock cifsInodeInfo->openFileList cifs_alloc_inode * cifsInodeInfo->writers_lock cifsInodeInfo->writers cifsInodeInfo_alloc * cifsInodeInfo->lock_sem cifsInodeInfo->llist cifs_init_once * ->can_cache_brlcks * cifsInodeInfo->deferred_lock cifsInodeInfo->deferred_closes cifsInodeInfo_alloc - * cached_fid->fid_mutex cifs_tcon->crfid tcon_info_alloc + * cached_fids->cfid_list_lock cifs_tcon->cfids->entries init_cached_dirs * cifsFileInfo->fh_mutex cifsFileInfo cifs_new_fileinfo * cifsFileInfo->file_info_lock cifsFileInfo->count cifs_new_fileinfo * ->invalidHandle initiate_cifs_search * ->oplock_break_cancelled ****************************************************************************/ @@ -2069,10 +2069,11 @@ extern struct workqueue_struct *cifsiod_wq; extern struct workqueue_struct *decrypt_wq; extern struct workqueue_struct *fileinfo_put_wq; extern struct workqueue_struct *cifsoplockd_wq; extern struct workqueue_struct *deferredclose_wq; extern struct workqueue_struct *serverclose_wq; +extern struct workqueue_struct *cfid_put_wq; extern __u32 cifs_lock_secret; extern mempool_t *cifs_sm_req_poolp; extern mempool_t *cifs_req_poolp; extern mempool_t *cifs_mid_poolp; diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index eff3f57235ee..20484853fb6b 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -2471,17 +2471,14 @@ cifs_dentry_needs_reval(struct dentry *dentry) if (!lookupCacheEnabled) return true; if (!open_cached_dir_by_dentry(tcon, dentry->d_parent, &cfid)) { - spin_lock(&cfid->fid_lock); if (cfid->time && cifs_i->time > cfid->time) { - spin_unlock(&cfid->fid_lock); close_cached_dir(cfid); return false; } - spin_unlock(&cfid->fid_lock); close_cached_dir(cfid); } /* * depending on inode type, check if attribute caching disabled for * files or directories diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h index 0b52d22a91a0..12cbd3428a6d 100644 --- a/fs/smb/client/trace.h +++ b/fs/smb/client/trace.h @@ -42,18 +42,21 @@ EM(netfs_trace_tcon_ref_free, "FRE ") \ EM(netfs_trace_tcon_ref_free_fail, "FRE Fail ") \ EM(netfs_trace_tcon_ref_free_ipc, "FRE Ipc ") \ EM(netfs_trace_tcon_ref_free_ipc_fail, "FRE Ipc-F ") \ EM(netfs_trace_tcon_ref_free_reconnect_server, "FRE Reconn") \ + EM(netfs_trace_tcon_ref_get_cached_laundromat, "GET Ch-Lau") \ + EM(netfs_trace_tcon_ref_get_cached_lease_break, "GET Ch-Lea") \ EM(netfs_trace_tcon_ref_get_cancelled_close, "GET Cn-Cls") \ EM(netfs_trace_tcon_ref_get_dfs_refer, "GET DfsRef") \ EM(netfs_trace_tcon_ref_get_find, "GET Find ") \ EM(netfs_trace_tcon_ref_get_find_sess_tcon, "GET FndSes") \ EM(netfs_trace_tcon_ref_get_reconnect_server, "GET Reconn") \ EM(netfs_trace_tcon_ref_new, "NEW ") \ EM(netfs_trace_tcon_ref_new_ipc, "NEW Ipc ") \ EM(netfs_trace_tcon_ref_new_reconnect_server, "NEW Reconn") \ + EM(netfs_trace_tcon_ref_put_cached_close, "PUT Ch-Cls") \ EM(netfs_trace_tcon_ref_put_cancelled_close, "PUT Cn-Cls") \ EM(netfs_trace_tcon_ref_put_cancelled_close_fid, "PUT Cn-Fid") \ EM(netfs_trace_tcon_ref_put_cancelled_mid, "PUT Cn-Mid") \ EM(netfs_trace_tcon_ref_put_mnt_ctx, "PUT MntCtx") \ EM(netfs_trace_tcon_ref_put_reconnect_server, "PUT Reconn") \