]>
Commit | Line | Data |
---|---|---|
04fd09d4 SL |
1 | From afdd1ea659684e70a9b24f1a04ba2983b3e31be5 Mon Sep 17 00:00:00 2001 |
2 | From: Aurelien Aptel <aaptel@suse.com> | |
3 | Date: Thu, 14 Mar 2019 18:44:16 +0100 | |
4 | Subject: CIFS: fix POSIX lock leak and invalid ptr deref | |
5 | ||
6 | [ Upstream commit bc31d0cdcfbadb6258b45db97e93b1c83822ba33 ] | |
7 | ||
8 | We have a customer reporting crashes in lock_get_status() with many | |
9 | "Leaked POSIX lock" messages preceeding the crash. | |
10 | ||
11 | Leaked POSIX lock on dev=0x0:0x56 ... | |
12 | Leaked POSIX lock on dev=0x0:0x56 ... | |
13 | Leaked POSIX lock on dev=0x0:0x56 ... | |
14 | Leaked POSIX lock on dev=0x0:0x53 ... | |
15 | Leaked POSIX lock on dev=0x0:0x53 ... | |
16 | Leaked POSIX lock on dev=0x0:0x53 ... | |
17 | Leaked POSIX lock on dev=0x0:0x53 ... | |
18 | POSIX: fl_owner=ffff8900e7b79380 fl_flags=0x1 fl_type=0x1 fl_pid=20709 | |
19 | Leaked POSIX lock on dev=0x0:0x4b ino... | |
20 | Leaked locks on dev=0x0:0x4b ino=0xf911400000029: | |
21 | POSIX: fl_owner=ffff89f41c870e00 fl_flags=0x1 fl_type=0x1 fl_pid=19592 | |
22 | stack segment: 0000 [#1] SMP | |
23 | Modules linked in: binfmt_misc msr tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag rpcsec_gss_krb5 arc4 ecb auth_rpcgss nfsv4 md4 nfs nls_utf8 lockd grace cifs sunrpc ccm dns_resolver fscache af_packet iscsi_ibft iscsi_boot_sysfs vmw_vsock_vmci_transport vsock xfs libcrc32c sb_edac edac_core crct10dif_pclmul crc32_pclmul ghash_clmulni_intel drbg ansi_cprng vmw_balloon aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd joydev pcspkr vmxnet3 i2c_piix4 vmw_vmci shpchp fjes processor button ac btrfs xor raid6_pq sr_mod cdrom ata_generic sd_mod ata_piix vmwgfx crc32c_intel drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm serio_raw ahci libahci drm libata vmw_pvscsi sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua scsi_mod autofs4 | |
24 | ||
25 | Supported: Yes | |
26 | CPU: 6 PID: 28250 Comm: lsof Not tainted 4.4.156-94.64-default #1 | |
27 | Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/05/2016 | |
28 | task: ffff88a345f28740 ti: ffff88c74005c000 task.ti: ffff88c74005c000 | |
29 | RIP: 0010:[<ffffffff8125dcab>] [<ffffffff8125dcab>] lock_get_status+0x9b/0x3b0 | |
30 | RSP: 0018:ffff88c74005fd90 EFLAGS: 00010202 | |
31 | RAX: ffff89bde83e20ae RBX: ffff89e870003d18 RCX: 0000000049534f50 | |
32 | RDX: ffffffff81a3541f RSI: ffffffff81a3544e RDI: ffff89bde83e20ae | |
33 | RBP: 0026252423222120 R08: 0000000020584953 R09: 000000000000ffff | |
34 | R10: 0000000000000000 R11: ffff88c74005fc70 R12: ffff89e5ca7b1340 | |
35 | R13: 00000000000050e5 R14: ffff89e870003d30 R15: ffff89e5ca7b1340 | |
36 | FS: 00007fafd64be800(0000) GS:ffff89f41fd00000(0000) knlGS:0000000000000000 | |
37 | CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 | |
38 | CR2: 0000000001c80018 CR3: 000000a522048000 CR4: 0000000000360670 | |
39 | DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 | |
40 | DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 | |
41 | Stack: | |
42 | 0000000000000208 ffffffff81a3d6b6 ffff89e870003d30 ffff89e870003d18 | |
43 | ffff89e5ca7b1340 ffff89f41738d7c0 ffff89e870003d30 ffff89e5ca7b1340 | |
44 | ffffffff8125e08f 0000000000000000 ffff89bc22b67d00 ffff88c74005ff28 | |
45 | Call Trace: | |
46 | [<ffffffff8125e08f>] locks_show+0x2f/0x70 | |
47 | [<ffffffff81230ad1>] seq_read+0x251/0x3a0 | |
48 | [<ffffffff81275bbc>] proc_reg_read+0x3c/0x70 | |
49 | [<ffffffff8120e456>] __vfs_read+0x26/0x140 | |
50 | [<ffffffff8120e9da>] vfs_read+0x7a/0x120 | |
51 | [<ffffffff8120faf2>] SyS_read+0x42/0xa0 | |
52 | [<ffffffff8161cbc3>] entry_SYSCALL_64_fastpath+0x1e/0xb7 | |
53 | ||
54 | When Linux closes a FD (close(), close-on-exec, dup2(), ...) it calls | |
55 | filp_close() which also removes all posix locks. | |
56 | ||
57 | The lock struct is initialized like so in filp_close() and passed | |
58 | down to cifs | |
59 | ||
60 | ... | |
61 | lock.fl_type = F_UNLCK; | |
62 | lock.fl_flags = FL_POSIX | FL_CLOSE; | |
63 | lock.fl_start = 0; | |
64 | lock.fl_end = OFFSET_MAX; | |
65 | ... | |
66 | ||
67 | Note the FL_CLOSE flag, which hints the VFS code that this unlocking | |
68 | is done for closing the fd. | |
69 | ||
70 | filp_close() | |
71 | locks_remove_posix(filp, id); | |
72 | vfs_lock_file(filp, F_SETLK, &lock, NULL); | |
73 | return filp->f_op->lock(filp, cmd, fl) => cifs_lock() | |
74 | rc = cifs_setlk(file, flock, type, wait_flag, posix_lck, lock, unlock, xid); | |
75 | rc = server->ops->mand_unlock_range(cfile, flock, xid); | |
76 | if (flock->fl_flags & FL_POSIX && !rc) | |
77 | rc = locks_lock_file_wait(file, flock) | |
78 | ||
79 | Notice how we don't call locks_lock_file_wait() which does the | |
80 | generic VFS lock/unlock/wait work on the inode if rc != 0. | |
81 | ||
82 | If we are closing the handle, the SMB server is supposed to remove any | |
83 | locks associated with it. Similarly, cifs.ko frees and wakes up any | |
84 | lock and lock waiter when closing the file: | |
85 | ||
86 | cifs_close() | |
87 | cifsFileInfo_put(file->private_data) | |
88 | /* | |
89 | * Delete any outstanding lock records. We'll lose them when the file | |
90 | * is closed anyway. | |
91 | */ | |
92 | down_write(&cifsi->lock_sem); | |
93 | list_for_each_entry_safe(li, tmp, &cifs_file->llist->locks, llist) { | |
94 | list_del(&li->llist); | |
95 | cifs_del_lock_waiters(li); | |
96 | kfree(li); | |
97 | } | |
98 | list_del(&cifs_file->llist->llist); | |
99 | kfree(cifs_file->llist); | |
100 | up_write(&cifsi->lock_sem); | |
101 | ||
102 | So we can safely ignore unlocking failures in cifs_lock() if they | |
103 | happen with the FL_CLOSE flag hint set as both the server and the | |
104 | client take care of it during the actual closing. | |
105 | ||
106 | This is not a proper fix for the unlocking failure but it's safe and | |
107 | it seems to prevent the lock leakages and crashes the customer | |
108 | experiences. | |
109 | ||
110 | Signed-off-by: Aurelien Aptel <aaptel@suse.com> | |
111 | Signed-off-by: NeilBrown <neil@brown.name> | |
112 | Signed-off-by: Steve French <stfrench@microsoft.com> | |
113 | Acked-by: Pavel Shilovsky <pshilov@microsoft.com> | |
114 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
115 | --- | |
116 | fs/cifs/file.c | 14 +++++++++++++- | |
117 | 1 file changed, 13 insertions(+), 1 deletion(-) | |
118 | ||
119 | diff --git a/fs/cifs/file.c b/fs/cifs/file.c | |
120 | index 72d6f4db9bdc..cd69c1e9750f 100644 | |
121 | --- a/fs/cifs/file.c | |
122 | +++ b/fs/cifs/file.c | |
123 | @@ -1631,8 +1631,20 @@ cifs_setlk(struct file *file, struct file_lock *flock, __u32 type, | |
124 | rc = server->ops->mand_unlock_range(cfile, flock, xid); | |
125 | ||
126 | out: | |
127 | - if (flock->fl_flags & FL_POSIX && !rc) | |
128 | + if (flock->fl_flags & FL_POSIX) { | |
129 | + /* | |
130 | + * If this is a request to remove all locks because we | |
131 | + * are closing the file, it doesn't matter if the | |
132 | + * unlocking failed as both cifs.ko and the SMB server | |
133 | + * remove the lock on file close | |
134 | + */ | |
135 | + if (rc) { | |
136 | + cifs_dbg(VFS, "%s failed rc=%d\n", __func__, rc); | |
137 | + if (!(flock->fl_flags & FL_CLOSE)) | |
138 | + return rc; | |
139 | + } | |
140 | rc = locks_lock_file_wait(file, flock); | |
141 | + } | |
142 | return rc; | |
143 | } | |
144 | ||
145 | -- | |
146 | 2.19.1 | |
147 |