Skip to content

Commit 8354d88

Browse files
Paulo Alcantara (SUSE)Steve French
authored andcommitted
cifs: Fix use-after-free bug in cifs_reconnect()
Ensure we grab an active reference in cifs superblock while doing failover to prevent automounts (DFS links) of expiring and then destroying the superblock pointer. This patch fixes the following KASAN report: [ 464.301462] BUG: KASAN: use-after-free in cifs_reconnect+0x6ab/0x1350 [ 464.303052] Read of size 8 at addr ffff888155e580d0 by task cifsd/1107 [ 464.304682] CPU: 3 PID: 1107 Comm: cifsd Not tainted 5.4.0-rc4+ #13 [ 464.305552] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.1-0-ga5cab58-rebuilt.opensuse.org 04/01/2014 [ 464.307146] Call Trace: [ 464.307875] dump_stack+0x5b/0x90 [ 464.308631] print_address_description.constprop.0+0x16/0x200 [ 464.309478] ? cifs_reconnect+0x6ab/0x1350 [ 464.310253] ? cifs_reconnect+0x6ab/0x1350 [ 464.311040] __kasan_report.cold+0x1a/0x41 [ 464.311811] ? cifs_reconnect+0x6ab/0x1350 [ 464.312563] kasan_report+0xe/0x20 [ 464.313300] cifs_reconnect+0x6ab/0x1350 [ 464.314062] ? extract_hostname.part.0+0x90/0x90 [ 464.314829] ? printk+0xad/0xde [ 464.315525] ? _raw_spin_lock+0x7c/0xd0 [ 464.316252] ? _raw_read_lock_irq+0x40/0x40 [ 464.316961] ? ___ratelimit+0xed/0x182 [ 464.317655] cifs_readv_from_socket+0x289/0x3b0 [ 464.318386] cifs_read_from_socket+0x98/0xd0 [ 464.319078] ? cifs_readv_from_socket+0x3b0/0x3b0 [ 464.319782] ? try_to_wake_up+0x43c/0xa90 [ 464.320463] ? cifs_small_buf_get+0x4b/0x60 [ 464.321173] ? allocate_buffers+0x98/0x1a0 [ 464.321856] cifs_demultiplex_thread+0x218/0x14a0 [ 464.322558] ? cifs_handle_standard+0x270/0x270 [ 464.323237] ? __switch_to_asm+0x40/0x70 [ 464.323893] ? __switch_to_asm+0x34/0x70 [ 464.324554] ? __switch_to_asm+0x40/0x70 [ 464.325226] ? __switch_to_asm+0x40/0x70 [ 464.325863] ? __switch_to_asm+0x34/0x70 [ 464.326505] ? __switch_to_asm+0x40/0x70 [ 464.327161] ? __switch_to_asm+0x34/0x70 [ 464.327784] ? finish_task_switch+0xa1/0x330 [ 464.328414] ? __switch_to+0x363/0x640 [ 464.329044] ? __schedule+0x575/0xaf0 [ 464.329655] ? _raw_spin_lock_irqsave+0x82/0xe0 [ 464.330301] kthread+0x1a3/0x1f0 [ 464.330884] ? cifs_handle_standard+0x270/0x270 [ 464.331624] ? kthread_create_on_node+0xd0/0xd0 [ 464.332347] ret_from_fork+0x35/0x40 [ 464.333577] Allocated by task 1110: [ 464.334381] save_stack+0x1b/0x80 [ 464.335123] __kasan_kmalloc.constprop.0+0xc2/0xd0 [ 464.335848] cifs_smb3_do_mount+0xd4/0xb00 [ 464.336619] legacy_get_tree+0x6b/0xa0 [ 464.337235] vfs_get_tree+0x41/0x110 [ 464.337975] fc_mount+0xa/0x40 [ 464.338557] vfs_kern_mount.part.0+0x6c/0x80 [ 464.339227] cifs_dfs_d_automount+0x336/0xd29 [ 464.339846] follow_managed+0x1b1/0x450 [ 464.340449] lookup_fast+0x231/0x4a0 [ 464.341039] path_openat+0x240/0x1fd0 [ 464.341634] do_filp_open+0x126/0x1c0 [ 464.342277] do_sys_open+0x1eb/0x2c0 [ 464.342957] do_syscall_64+0x5e/0x190 [ 464.343555] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 464.344772] Freed by task 0: [ 464.345347] save_stack+0x1b/0x80 [ 464.345966] __kasan_slab_free+0x12c/0x170 [ 464.346576] kfree+0xa6/0x270 [ 464.347211] rcu_core+0x39c/0xc80 [ 464.347800] __do_softirq+0x10d/0x3da [ 464.348919] The buggy address belongs to the object at ffff888155e58000 which belongs to the cache kmalloc-256 of size 256 [ 464.350222] The buggy address is located 208 bytes inside of 256-byte region [ffff888155e58000, ffff888155e58100) [ 464.351575] The buggy address belongs to the page: [ 464.352333] page:ffffea0005579600 refcount:1 mapcount:0 mapping:ffff88815a803400 index:0x0 compound_mapcount: 0 [ 464.353583] flags: 0x200000000010200(slab|head) [ 464.354209] raw: 0200000000010200 ffffea0005576200 0000000400000004 ffff88815a803400 [ 464.355353] raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000 [ 464.356458] page dumped because: kasan: bad access detected [ 464.367005] Memory state around the buggy address: [ 464.367787] ffff888155e57f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 464.368877] ffff888155e58000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 464.369967] >ffff888155e58080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 464.371111] ^ [ 464.371775] ffff888155e58100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 464.372893] ffff888155e58180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 464.373983] ================================================================== Signed-off-by: Paulo Alcantara (SUSE) <[email protected]> Reviewed-by: Aurelien Aptel <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 8515092 commit 8354d88

File tree

1 file changed

+35
-11
lines changed

1 file changed

+35
-11
lines changed

fs/cifs/connect.c

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ static inline int reconn_set_ipaddr(struct TCP_Server_Info *server)
391391
#ifdef CONFIG_CIFS_DFS_UPCALL
392392
struct super_cb_data {
393393
struct TCP_Server_Info *server;
394-
struct cifs_sb_info *cifs_sb;
394+
struct super_block *sb;
395395
};
396396

397397
/* These functions must be called with server->srv_mutex held */
@@ -402,25 +402,39 @@ static void super_cb(struct super_block *sb, void *arg)
402402
struct cifs_sb_info *cifs_sb;
403403
struct cifs_tcon *tcon;
404404

405-
if (d->cifs_sb)
405+
if (d->sb)
406406
return;
407407

408408
cifs_sb = CIFS_SB(sb);
409409
tcon = cifs_sb_master_tcon(cifs_sb);
410410
if (tcon->ses->server == d->server)
411-
d->cifs_sb = cifs_sb;
411+
d->sb = sb;
412412
}
413413

414-
static inline struct cifs_sb_info *
415-
find_super_by_tcp(struct TCP_Server_Info *server)
414+
static struct super_block *get_tcp_super(struct TCP_Server_Info *server)
416415
{
417416
struct super_cb_data d = {
418417
.server = server,
419-
.cifs_sb = NULL,
418+
.sb = NULL,
420419
};
421420

422421
iterate_supers_type(&cifs_fs_type, super_cb, &d);
423-
return d.cifs_sb ? d.cifs_sb : ERR_PTR(-ENOENT);
422+
423+
if (unlikely(!d.sb))
424+
return ERR_PTR(-ENOENT);
425+
/*
426+
* Grab an active reference in order to prevent automounts (DFS links)
427+
* of expiring and then freeing up our cifs superblock pointer while
428+
* we're doing failover.
429+
*/
430+
cifs_sb_active(d.sb);
431+
return d.sb;
432+
}
433+
434+
static inline void put_tcp_super(struct super_block *sb)
435+
{
436+
if (!IS_ERR_OR_NULL(sb))
437+
cifs_sb_deactive(sb);
424438
}
425439

426440
static void reconn_inval_dfs_target(struct TCP_Server_Info *server,
@@ -484,6 +498,7 @@ cifs_reconnect(struct TCP_Server_Info *server)
484498
struct mid_q_entry *mid_entry;
485499
struct list_head retry_list;
486500
#ifdef CONFIG_CIFS_DFS_UPCALL
501+
struct super_block *sb = NULL;
487502
struct cifs_sb_info *cifs_sb = NULL;
488503
struct dfs_cache_tgt_list tgt_list = {0};
489504
struct dfs_cache_tgt_iterator *tgt_it = NULL;
@@ -493,13 +508,15 @@ cifs_reconnect(struct TCP_Server_Info *server)
493508
server->nr_targets = 1;
494509
#ifdef CONFIG_CIFS_DFS_UPCALL
495510
spin_unlock(&GlobalMid_Lock);
496-
cifs_sb = find_super_by_tcp(server);
497-
if (IS_ERR(cifs_sb)) {
498-
rc = PTR_ERR(cifs_sb);
511+
sb = get_tcp_super(server);
512+
if (IS_ERR(sb)) {
513+
rc = PTR_ERR(sb);
499514
cifs_dbg(FYI, "%s: will not do DFS failover: rc = %d\n",
500515
__func__, rc);
501-
cifs_sb = NULL;
516+
sb = NULL;
502517
} else {
518+
cifs_sb = CIFS_SB(sb);
519+
503520
rc = reconn_setup_dfs_targets(cifs_sb, &tgt_list, &tgt_it);
504521
if (rc && (rc != -EOPNOTSUPP)) {
505522
cifs_server_dbg(VFS, "%s: no target servers for DFS failover\n",
@@ -516,6 +533,10 @@ cifs_reconnect(struct TCP_Server_Info *server)
516533
/* the demux thread will exit normally
517534
next time through the loop */
518535
spin_unlock(&GlobalMid_Lock);
536+
#ifdef CONFIG_CIFS_DFS_UPCALL
537+
dfs_cache_free_tgts(&tgt_list);
538+
put_tcp_super(sb);
539+
#endif
519540
return rc;
520541
} else
521542
server->tcpStatus = CifsNeedReconnect;
@@ -642,7 +663,10 @@ cifs_reconnect(struct TCP_Server_Info *server)
642663
__func__, rc);
643664
}
644665
dfs_cache_free_tgts(&tgt_list);
666+
645667
}
668+
669+
put_tcp_super(sb);
646670
#endif
647671
if (server->tcpStatus == CifsNeedNegotiate)
648672
mod_delayed_work(cifsiod_wq, &server->echo, 0);

0 commit comments

Comments
 (0)