Skip to content

Commit 4a31687

Browse files
PlaidCatPlaidCat
committed
smb: client: fix UAF in smb2_reconnect_server()
jira LE-4669 cve CVE-2024-35870 Rebuild_History Non-Buildable kernel-4.18.0-553.82.1.el8_10 commit-author Paulo Alcantara <[email protected]> commit 24a9799 Empty-Commit: Cherry-Pick Conflicts during history rebuild. Will be included in final tarball splat. Ref for failed cherry-pick at: ciq/ciq_backports/kernel-4.18.0-553.82.1.el8_10/24a9799a.failed The UAF bug is due to smb2_reconnect_server() accessing a session that is already being teared down by another thread that is executing __cifs_put_smb_ses(). This can happen when (a) the client has connection to the server but no session or (b) another thread ends up setting @ses->ses_status again to something different than SES_EXITING. To fix this, we need to make sure to unconditionally set @ses->ses_status to SES_EXITING and prevent any other threads from setting a new status while we're still tearing it down. The following can be reproduced by adding some delay to right after the ipc is freed in __cifs_put_smb_ses() - which will give smb2_reconnect_server() worker a chance to run and then accessing @ses->ipc: kinit ... mount.cifs //srv/share /mnt/1 -o sec=krb5,nohandlecache,echo_interval=10 [disconnect srv] ls /mnt/1 &>/dev/null sleep 30 kdestroy [reconnect srv] sleep 10 umount /mnt/1 ... CIFS: VFS: Verify user has a krb5 ticket and keyutils is installed CIFS: VFS: \\srv Send error in SessSetup = -126 CIFS: VFS: Verify user has a krb5 ticket and keyutils is installed CIFS: VFS: \\srv Send error in SessSetup = -126 general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: 0000 [#1] PREEMPT SMP NOPTI CPU: 3 PID: 50 Comm: kworker/3:1 Not tainted 6.9.0-rc2 #1 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-1.fc39 04/01/2014 Workqueue: cifsiod smb2_reconnect_server [cifs] RIP: 0010:__list_del_entry_valid_or_report+0x33/0xf0 Code: 4f 08 48 85 d2 74 42 48 85 c9 74 59 48 b8 00 01 00 00 00 00 ad de 48 39 c2 74 61 48 b8 22 01 00 00 00 00 74 69 <48> 8b 01 48 39 f8 75 7b 48 8b 72 08 48 39 c6 0f 85 88 00 00 00 b8 RSP: 0018:ffffc900001bfd70 EFLAGS: 00010a83 RAX: dead000000000122 RBX: ffff88810da53838 RCX: 6b6b6b6b6b6b6b6b RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc02f6878 RDI: ffff88810da53800 RBP: ffff88810da53800 R08: 0000000000000001 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000001 R12: ffff88810c064000 R13: 0000000000000001 R14: ffff88810c064000 R15: ffff8881039cc000 FS: 0000000000000000(0000) GS:ffff888157c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fe3728b1000 CR3: 000000010caa4000 CR4: 0000000000750ef0 PKRU: 55555554 Call Trace: <TASK> ? die_addr+0x36/0x90 ? exc_general_protection+0x1c1/0x3f0 ? asm_exc_general_protection+0x26/0x30 ? __list_del_entry_valid_or_report+0x33/0xf0 __cifs_put_smb_ses+0x1ae/0x500 [cifs] smb2_reconnect_server+0x4ed/0x710 [cifs] process_one_work+0x205/0x6b0 worker_thread+0x191/0x360 ? __pfx_worker_thread+0x10/0x10 kthread+0xe2/0x110 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x34/0x50 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1a/0x30 </TASK> Cc: [email protected] Signed-off-by: Paulo Alcantara (Red Hat) <[email protected]> Signed-off-by: Steve French <[email protected]> (cherry picked from commit 24a9799) Signed-off-by: Jonathan Maple <[email protected]> # Conflicts: # fs/cifs/connect.c
1 parent 6c057e6 commit 4a31687

File tree

1 file changed

+395
-0
lines changed

1 file changed

+395
-0
lines changed

‎ciq/ciq_backports/kernel-4.18.0-553.82.1.el8_10/24a9799a.failed‎

Lines changed: 395 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,395 @@
1+
smb: client: fix UAF in smb2_reconnect_server()
2+
3+
jira LE-4669
4+
cve CVE-2024-35870
5+
Rebuild_History Non-Buildable kernel-4.18.0-553.82.1.el8_10
6+
commit-author Paulo Alcantara <[email protected]>
7+
commit 24a9799aa8efecd0eb55a75e35f9d8e6400063aa
8+
Empty-Commit: Cherry-Pick Conflicts during history rebuild.
9+
Will be included in final tarball splat. Ref for failed cherry-pick at:
10+
ciq/ciq_backports/kernel-4.18.0-553.82.1.el8_10/24a9799a.failed
11+
12+
The UAF bug is due to smb2_reconnect_server() accessing a session that
13+
is already being teared down by another thread that is executing
14+
__cifs_put_smb_ses(). This can happen when (a) the client has
15+
connection to the server but no session or (b) another thread ends up
16+
setting @ses->ses_status again to something different than
17+
SES_EXITING.
18+
19+
To fix this, we need to make sure to unconditionally set
20+
@ses->ses_status to SES_EXITING and prevent any other threads from
21+
setting a new status while we're still tearing it down.
22+
23+
The following can be reproduced by adding some delay to right after
24+
the ipc is freed in __cifs_put_smb_ses() - which will give
25+
smb2_reconnect_server() worker a chance to run and then accessing
26+
@ses->ipc:
27+
28+
kinit ...
29+
mount.cifs //srv/share /mnt/1 -o sec=krb5,nohandlecache,echo_interval=10
30+
[disconnect srv]
31+
ls /mnt/1 &>/dev/null
32+
sleep 30
33+
kdestroy
34+
[reconnect srv]
35+
sleep 10
36+
umount /mnt/1
37+
...
38+
CIFS: VFS: Verify user has a krb5 ticket and keyutils is installed
39+
CIFS: VFS: \\srv Send error in SessSetup = -126
40+
CIFS: VFS: Verify user has a krb5 ticket and keyutils is installed
41+
CIFS: VFS: \\srv Send error in SessSetup = -126
42+
general protection fault, probably for non-canonical address
43+
0x6b6b6b6b6b6b6b6b: 0000 [#1] PREEMPT SMP NOPTI
44+
CPU: 3 PID: 50 Comm: kworker/3:1 Not tainted 6.9.0-rc2 #1
45+
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-1.fc39
46+
04/01/2014
47+
Workqueue: cifsiod smb2_reconnect_server [cifs]
48+
RIP: 0010:__list_del_entry_valid_or_report+0x33/0xf0
49+
Code: 4f 08 48 85 d2 74 42 48 85 c9 74 59 48 b8 00 01 00 00 00 00 ad
50+
de 48 39 c2 74 61 48 b8 22 01 00 00 00 00 74 69 <48> 8b 01 48 39 f8 75
51+
7b 48 8b 72 08 48 39 c6 0f 85 88 00 00 00 b8
52+
RSP: 0018:ffffc900001bfd70 EFLAGS: 00010a83
53+
RAX: dead000000000122 RBX: ffff88810da53838 RCX: 6b6b6b6b6b6b6b6b
54+
RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc02f6878 RDI: ffff88810da53800
55+
RBP: ffff88810da53800 R08: 0000000000000001 R09: 0000000000000000
56+
R10: 0000000000000000 R11: 0000000000000001 R12: ffff88810c064000
57+
R13: 0000000000000001 R14: ffff88810c064000 R15: ffff8881039cc000
58+
FS: 0000000000000000(0000) GS:ffff888157c00000(0000)
59+
knlGS:0000000000000000
60+
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
61+
CR2: 00007fe3728b1000 CR3: 000000010caa4000 CR4: 0000000000750ef0
62+
PKRU: 55555554
63+
Call Trace:
64+
<TASK>
65+
? die_addr+0x36/0x90
66+
? exc_general_protection+0x1c1/0x3f0
67+
? asm_exc_general_protection+0x26/0x30
68+
? __list_del_entry_valid_or_report+0x33/0xf0
69+
__cifs_put_smb_ses+0x1ae/0x500 [cifs]
70+
smb2_reconnect_server+0x4ed/0x710 [cifs]
71+
process_one_work+0x205/0x6b0
72+
worker_thread+0x191/0x360
73+
? __pfx_worker_thread+0x10/0x10
74+
kthread+0xe2/0x110
75+
? __pfx_kthread+0x10/0x10
76+
ret_from_fork+0x34/0x50
77+
? __pfx_kthread+0x10/0x10
78+
ret_from_fork_asm+0x1a/0x30
79+
</TASK>
80+
81+
Cc: [email protected]
82+
Signed-off-by: Paulo Alcantara (Red Hat) <[email protected]>
83+
Signed-off-by: Steve French <[email protected]>
84+
(cherry picked from commit 24a9799aa8efecd0eb55a75e35f9d8e6400063aa)
85+
Signed-off-by: Jonathan Maple <[email protected]>
86+
87+
# Conflicts:
88+
# fs/cifs/connect.c
89+
diff --cc fs/cifs/connect.c
90+
index ca2926c7b59e,ee29bc57300c..000000000000
91+
--- a/fs/cifs/connect.c
92+
+++ b/fs/cifs/connect.c
93+
@@@ -164,39 -144,161 +164,132 @@@ static void smb2_query_server_interface
94+
(SMB_INTERFACE_POLL_INTERVAL * HZ));
95+
}
96+
97+
-/*
98+
- * Update the tcpStatus for the server.
99+
- * This is used to signal the cifsd thread to call cifs_reconnect
100+
- * ONLY cifsd thread should call cifs_reconnect. For any other
101+
- * thread, use this function
102+
- *
103+
- * @server: the tcp ses for which reconnect is needed
104+
- * @all_channels: if this needs to be done for all channels
105+
- */
106+
-void
107+
-cifs_signal_cifsd_for_reconnect(struct TCP_Server_Info *server,
108+
- bool all_channels)
109+
+static void cifs_resolve_server(struct work_struct *work)
110+
{
111+
- struct TCP_Server_Info *pserver;
112+
- struct cifs_ses *ses;
113+
- int i;
114+
+ int rc;
115+
+ struct TCP_Server_Info *server = container_of(work,
116+
+ struct TCP_Server_Info, resolve.work);
117+
118+
- /* If server is a channel, select the primary channel */
119+
- pserver = SERVER_IS_CHAN(server) ? server->primary_server : server;
120+
+ mutex_lock(&server->srv_mutex);
121+
122+
- /* if we need to signal just this channel */
123+
- if (!all_channels) {
124+
- spin_lock(&server->srv_lock);
125+
- if (server->tcpStatus != CifsExiting)
126+
- server->tcpStatus = CifsNeedReconnect;
127+
- spin_unlock(&server->srv_lock);
128+
- return;
129+
+ /*
130+
+ * Resolve the hostname again to make sure that IP address is up-to-date.
131+
+ */
132+
+ rc = reconn_set_ipaddr_from_hostname(server);
133+
+ if (rc) {
134+
+ cifs_dbg(FYI, "%s: failed to resolve hostname: %d\n",
135+
+ __func__, rc);
136+
}
137+
138+
- spin_lock(&cifs_tcp_ses_lock);
139+
- list_for_each_entry(ses, &pserver->smb_ses_list, smb_ses_list) {
140+
- spin_lock(&ses->chan_lock);
141+
- for (i = 0; i < ses->chan_count; i++) {
142+
- if (!ses->chans[i].server)
143+
- continue;
144+
-
145+
- spin_lock(&ses->chans[i].server->srv_lock);
146+
- if (ses->chans[i].server->tcpStatus != CifsExiting)
147+
- ses->chans[i].server->tcpStatus = CifsNeedReconnect;
148+
- spin_unlock(&ses->chans[i].server->srv_lock);
149+
- }
150+
- spin_unlock(&ses->chan_lock);
151+
- }
152+
- spin_unlock(&cifs_tcp_ses_lock);
153+
+ mutex_unlock(&server->srv_mutex);
154+
}
155+
156+
-/*
157+
+/**
158+
* Mark all sessions and tcons for reconnect.
159+
- * IMPORTANT: make sure that this gets called only from
160+
- * cifsd thread. For any other thread, use
161+
- * cifs_signal_cifsd_for_reconnect
162+
*
163+
- * @server: the tcp ses for which reconnect is needed
164+
* @server needs to be previously set to CifsNeedReconnect.
165+
- * @mark_smb_session: whether even sessions need to be marked
166+
*/
167+
-void
168+
-cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
169+
- bool mark_smb_session)
170+
+static void cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server)
171+
{
172+
- struct TCP_Server_Info *pserver;
173+
- struct cifs_ses *ses, *nses;
174+
+ unsigned int num_sessions = 0;
175+
+ struct cifs_ses *ses;
176+
struct cifs_tcon *tcon;
177+
++<<<<<<< HEAD:fs/cifs/connect.c
178+
++=======
179+
+
180+
+ /*
181+
+ * before reconnecting the tcp session, mark the smb session (uid) and the tid bad so they
182+
+ * are not used until reconnected.
183+
+ */
184+
+ cifs_dbg(FYI, "%s: marking necessary sessions and tcons for reconnect\n", __func__);
185+
+
186+
+ /* If server is a channel, select the primary channel */
187+
+ pserver = SERVER_IS_CHAN(server) ? server->primary_server : server;
188+
+
189+
+ /*
190+
+ * if the server has been marked for termination, there is a
191+
+ * chance that the remaining channels all need reconnect. To be
192+
+ * on the safer side, mark the session and trees for reconnect
193+
+ * for this scenario. This might cause a few redundant session
194+
+ * setup and tree connect requests, but it is better than not doing
195+
+ * a tree connect when needed, and all following requests failing
196+
+ */
197+
+ if (server->terminate) {
198+
+ mark_smb_session = true;
199+
+ server = pserver;
200+
+ }
201+
+
202+
+ spin_lock(&cifs_tcp_ses_lock);
203+
+ list_for_each_entry_safe(ses, nses, &pserver->smb_ses_list, smb_ses_list) {
204+
+ spin_lock(&ses->ses_lock);
205+
+ if (ses->ses_status == SES_EXITING) {
206+
+ spin_unlock(&ses->ses_lock);
207+
+ continue;
208+
+ }
209+
+ spin_unlock(&ses->ses_lock);
210+
+
211+
+ spin_lock(&ses->chan_lock);
212+
+ if (cifs_ses_get_chan_index(ses, server) ==
213+
+ CIFS_INVAL_CHAN_INDEX) {
214+
+ spin_unlock(&ses->chan_lock);
215+
+ continue;
216+
+ }
217+
+
218+
+ if (!cifs_chan_is_iface_active(ses, server)) {
219+
+ spin_unlock(&ses->chan_lock);
220+
+ cifs_chan_update_iface(ses, server);
221+
+ spin_lock(&ses->chan_lock);
222+
+ }
223+
+
224+
+ if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server)) {
225+
+ spin_unlock(&ses->chan_lock);
226+
+ continue;
227+
+ }
228+
+
229+
+ if (mark_smb_session)
230+
+ CIFS_SET_ALL_CHANS_NEED_RECONNECT(ses);
231+
+ else
232+
+ cifs_chan_set_need_reconnect(ses, server);
233+
+
234+
+ cifs_dbg(FYI, "%s: channel connect bitmap: 0x%lx\n",
235+
+ __func__, ses->chans_need_reconnect);
236+
+
237+
+ /* If all channels need reconnect, then tcon needs reconnect */
238+
+ if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {
239+
+ spin_unlock(&ses->chan_lock);
240+
+ continue;
241+
+ }
242+
+ spin_unlock(&ses->chan_lock);
243+
+
244+
+ spin_lock(&ses->ses_lock);
245+
+ ses->ses_status = SES_NEED_RECON;
246+
+ spin_unlock(&ses->ses_lock);
247+
+
248+
+ list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
249+
+ tcon->need_reconnect = true;
250+
+ spin_lock(&tcon->tc_lock);
251+
+ tcon->status = TID_NEED_RECON;
252+
+ spin_unlock(&tcon->tc_lock);
253+
+
254+
+ cancel_delayed_work(&tcon->query_interfaces);
255+
+ }
256+
+ if (ses->tcon_ipc) {
257+
+ ses->tcon_ipc->need_reconnect = true;
258+
+ spin_lock(&ses->tcon_ipc->tc_lock);
259+
+ ses->tcon_ipc->status = TID_NEED_RECON;
260+
+ spin_unlock(&ses->tcon_ipc->tc_lock);
261+
+ }
262+
+ }
263+
+ spin_unlock(&cifs_tcp_ses_lock);
264+
+ }
265+
+
266+
+ static void
267+
+ cifs_abort_connection(struct TCP_Server_Info *server)
268+
+ {
269+
++>>>>>>> 24a9799aa8ef (smb: client: fix UAF in smb2_reconnect_server()):fs/smb/client/connect.c
270+
struct mid_q_entry *mid, *nmid;
271+
struct list_head retry_list;
272+
+ struct TCP_Server_Info *pserver;
273+
274+
server->maxBuf = 0;
275+
server->max_read = 0;
276+
@@@ -1774,54 -1976,70 +1842,97 @@@ cifs_find_smb_ses(struct TCP_Server_Inf
277+
278+
spin_lock(&cifs_tcp_ses_lock);
279+
list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
280+
- spin_lock(&ses->ses_lock);
281+
- if (ses->ses_status == SES_EXITING) {
282+
- spin_unlock(&ses->ses_lock);
283+
+ if (ses->status == CifsExiting)
284+
continue;
285+
- }
286+
spin_lock(&ses->chan_lock);
287+
- if (match_session(ses, ctx)) {
288+
+ if (!match_session(ses, ctx)) {
289+
spin_unlock(&ses->chan_lock);
290+
- spin_unlock(&ses->ses_lock);
291+
- ret = ses;
292+
- break;
293+
+ continue;
294+
}
295+
spin_unlock(&ses->chan_lock);
296+
- spin_unlock(&ses->ses_lock);
297+
+ ++ses->ses_count;
298+
+ spin_unlock(&cifs_tcp_ses_lock);
299+
+ return ses;
300+
}
301+
- if (ret)
302+
- cifs_smb_ses_inc_refcount(ret);
303+
spin_unlock(&cifs_tcp_ses_lock);
304+
- return ret;
305+
+ return NULL;
306+
}
307+
308+
-void __cifs_put_smb_ses(struct cifs_ses *ses)
309+
+void cifs_put_smb_ses(struct cifs_ses *ses)
310+
{
311+
+ unsigned int rc, xid;
312+
+ unsigned int chan_count;
313+
struct TCP_Server_Info *server = ses->server;
314+
- struct cifs_tcon *tcon;
315+
- unsigned int xid;
316+
- size_t i;
317+
- bool do_logoff;
318+
- int rc;
319+
++<<<<<<< HEAD:fs/cifs/connect.c
320+
321+
spin_lock(&cifs_tcp_ses_lock);
322+
- spin_lock(&ses->ses_lock);
323+
- cifs_dbg(FYI, "%s: id=0x%llx ses_count=%d ses_status=%u ipc=%s\n",
324+
- __func__, ses->Suid, ses->ses_count, ses->ses_status,
325+
+ if (ses->status == CifsExiting) {
326+
+ spin_unlock(&cifs_tcp_ses_lock);
327+
+ return;
328+
+ }
329+
+
330+
+ cifs_dbg(FYI, "%s: ses_count=%d\n", __func__, ses->ses_count);
331+
+ cifs_dbg(FYI, "%s: ses ipc: %s\n", __func__, ses->tcon_ipc ? ses->tcon_ipc->treeName : "NONE");
332+
+
333+
+ if (--ses->ses_count > 0) {
334+
+ spin_unlock(&cifs_tcp_ses_lock);
335+
+ return;
336+
+ }
337+
+ spin_unlock(&cifs_tcp_ses_lock);
338+
+
339+
+ /* ses_count can never go negative */
340+
+ WARN_ON(ses->ses_count < 0);
341+
+
342+
+ spin_lock(&GlobalMid_Lock);
343+
+ if (ses->status == CifsGood)
344+
+ ses->status = CifsExiting;
345+
+ spin_unlock(&GlobalMid_Lock);
346+
+
347+
+ cifs_free_ipc(ses);
348+
+
349+
+ if (ses->status == CifsExiting && server->ops->logoff) {
350+
++=======
351+
++ struct cifs_tcon *tcon;
352+
++ unsigned int xid;
353+
++ size_t i;
354+
++ bool do_logoff;
355+
++ int rc;
356+
++
357+
++ spin_lock(&cifs_tcp_ses_lock);
358+
++ spin_lock(&ses->ses_lock);
359+
++ cifs_dbg(FYI, "%s: id=0x%llx ses_count=%d ses_status=%u ipc=%s\n",
360+
++ __func__, ses->Suid, ses->ses_count, ses->ses_status,
361+
+ ses->tcon_ipc ? ses->tcon_ipc->tree_name : "none");
362+
+ if (ses->ses_status == SES_EXITING || --ses->ses_count > 0) {
363+
+ spin_unlock(&ses->ses_lock);
364+
+ spin_unlock(&cifs_tcp_ses_lock);
365+
+ return;
366+
+ }
367+
+ /* ses_count can never go negative */
368+
+ WARN_ON(ses->ses_count < 0);
369+
+
370+
+ spin_lock(&ses->chan_lock);
371+
+ cifs_chan_clear_need_reconnect(ses, server);
372+
+ spin_unlock(&ses->chan_lock);
373+
+
374+
+ do_logoff = ses->ses_status == SES_GOOD && server->ops->logoff;
375+
+ ses->ses_status = SES_EXITING;
376+
+ tcon = ses->tcon_ipc;
377+
+ ses->tcon_ipc = NULL;
378+
+ spin_unlock(&ses->ses_lock);
379+
+ spin_unlock(&cifs_tcp_ses_lock);
380+
+
381+
+ /*
382+
+ * On session close, the IPC is closed and the server must release all
383+
+ * tcons of the session. No need to send a tree disconnect here.
384+
+ *
385+
+ * Besides, it will make the server to not close durable and resilient
386+
+ * files on session close, as specified in MS-SMB2 3.3.5.6 Receiving an
387+
+ * SMB2 LOGOFF Request.
388+
+ */
389+
+ tconInfoFree(tcon);
390+
+ if (do_logoff) {
391+
++>>>>>>> 24a9799aa8ef (smb: client: fix UAF in smb2_reconnect_server()):fs/smb/client/connect.c
392+
xid = get_xid();
393+
rc = server->ops->logoff(xid, ses);
394+
if (rc)
395+
* Unmerged path fs/cifs/connect.c

0 commit comments

Comments
 (0)