* [PATCH 0/7] DFS fixes
@ 2019-11-22 15:30 Paulo Alcantara (SUSE)
2019-11-22 15:30 ` [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() Paulo Alcantara (SUSE)
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Paulo Alcantara (SUSE) @ 2019-11-22 15:30 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE)
Hi,
Follow a set of DFS fixes and cleanups, and one fix related to
multichannel support.
Paulo Alcantara (SUSE) (7):
cifs: Fix use-after-free bug in cifs_reconnect()
cifs: Fix lookup of root ses in DFS referral cache
cifs: Fix potential softlockups while refreshing DFS cache
cifs: Clean up DFS referral cache
cifs: Fix potential deadlock when updating vol in cifs_reconnect()
cifs: Fix retrieval of DFS referrals in cifs_mount()
cifs: Always update signing key of first channel
fs/cifs/connect.c | 78 ++-
fs/cifs/dfs_cache.c | 1019 ++++++++++++++++++++-------------------
fs/cifs/smb2pdu.c | 41 +-
fs/cifs/smb2transport.c | 4 +
4 files changed, 612 insertions(+), 530 deletions(-)
--
2.24.0
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() 2019-11-22 15:30 [PATCH 0/7] DFS fixes Paulo Alcantara (SUSE) @ 2019-11-22 15:30 ` Paulo Alcantara (SUSE) 2019-11-25 7:34 ` Steve French 2019-11-25 11:36 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 2/7] cifs: Fix lookup of root ses in DFS referral cache Paulo Alcantara (SUSE) ` (5 subsequent siblings) 6 siblings, 2 replies; 19+ messages in thread From: Paulo Alcantara (SUSE) @ 2019-11-22 15:30 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) 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) <pc@cjr.nz> --- fs/cifs/connect.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index fce71a6cc3f4..668d477cc9c7 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -391,7 +391,7 @@ static inline int reconn_set_ipaddr(struct TCP_Server_Info *server) #ifdef CONFIG_CIFS_DFS_UPCALL struct super_cb_data { struct TCP_Server_Info *server; - struct cifs_sb_info *cifs_sb; + struct super_block *sb; }; /* These functions must be called with server->srv_mutex held */ @@ -402,25 +402,39 @@ static void super_cb(struct super_block *sb, void *arg) struct cifs_sb_info *cifs_sb; struct cifs_tcon *tcon; - if (d->cifs_sb) + if (d->sb) return; cifs_sb = CIFS_SB(sb); tcon = cifs_sb_master_tcon(cifs_sb); if (tcon->ses->server == d->server) - d->cifs_sb = cifs_sb; + d->sb = sb; } -static inline struct cifs_sb_info * -find_super_by_tcp(struct TCP_Server_Info *server) +static struct super_block *get_tcp_super(struct TCP_Server_Info *server) { struct super_cb_data d = { .server = server, - .cifs_sb = NULL, + .sb = NULL, }; iterate_supers_type(&cifs_fs_type, super_cb, &d); - return d.cifs_sb ? d.cifs_sb : ERR_PTR(-ENOENT); + + if (unlikely(!d.sb)) + return ERR_PTR(-ENOENT); + /* + * Grab an active reference in order to prevent automounts (DFS links) + * of expiring and then freeing up our cifs superblock pointer while + * we're doing failover. + */ + cifs_sb_active(d.sb); + return d.sb; +} + +static inline void put_tcp_super(struct super_block *sb) +{ + if (!IS_ERR_OR_NULL(sb)) + cifs_sb_deactive(sb); } static void reconn_inval_dfs_target(struct TCP_Server_Info *server, @@ -484,6 +498,7 @@ cifs_reconnect(struct TCP_Server_Info *server) struct mid_q_entry *mid_entry; struct list_head retry_list; #ifdef CONFIG_CIFS_DFS_UPCALL + struct super_block *sb = NULL; struct cifs_sb_info *cifs_sb = NULL; struct dfs_cache_tgt_list tgt_list = {0}; struct dfs_cache_tgt_iterator *tgt_it = NULL; @@ -493,13 +508,15 @@ cifs_reconnect(struct TCP_Server_Info *server) server->nr_targets = 1; #ifdef CONFIG_CIFS_DFS_UPCALL spin_unlock(&GlobalMid_Lock); - cifs_sb = find_super_by_tcp(server); - if (IS_ERR(cifs_sb)) { - rc = PTR_ERR(cifs_sb); + sb = get_tcp_super(server); + if (IS_ERR(sb)) { + rc = PTR_ERR(sb); cifs_dbg(FYI, "%s: will not do DFS failover: rc = %d\n", __func__, rc); - cifs_sb = NULL; + sb = NULL; } else { + cifs_sb = CIFS_SB(sb); + rc = reconn_setup_dfs_targets(cifs_sb, &tgt_list, &tgt_it); if (rc && (rc != -EOPNOTSUPP)) { cifs_server_dbg(VFS, "%s: no target servers for DFS failover\n", @@ -516,6 +533,10 @@ cifs_reconnect(struct TCP_Server_Info *server) /* the demux thread will exit normally next time through the loop */ spin_unlock(&GlobalMid_Lock); +#ifdef CONFIG_CIFS_DFS_UPCALL + dfs_cache_free_tgts(&tgt_list); + put_tcp_super(sb); +#endif return rc; } else server->tcpStatus = CifsNeedReconnect; @@ -642,7 +663,10 @@ cifs_reconnect(struct TCP_Server_Info *server) __func__, rc); } dfs_cache_free_tgts(&tgt_list); + } + + put_tcp_super(sb); #endif if (server->tcpStatus == CifsNeedNegotiate) mod_delayed_work(cifsiod_wq, &server->echo, 0); -- 2.24.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() 2019-11-22 15:30 ` [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() Paulo Alcantara (SUSE) @ 2019-11-25 7:34 ` Steve French 2019-11-25 11:36 ` Aurélien Aptel 1 sibling, 0 replies; 19+ messages in thread From: Steve French @ 2019-11-25 7:34 UTC (permalink / raw) To: Paulo Alcantara (SUSE); +Cc: CIFS merged into cifs-2.6.git for-next pending testing and more reviews On Fri, Nov 22, 2019 at 9:31 AM Paulo Alcantara (SUSE) <pc@cjr.nz> wrote: > > 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) <pc@cjr.nz> > --- > fs/cifs/connect.c | 46 +++++++++++++++++++++++++++++++++++----------- > 1 file changed, 35 insertions(+), 11 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index fce71a6cc3f4..668d477cc9c7 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -391,7 +391,7 @@ static inline int reconn_set_ipaddr(struct TCP_Server_Info *server) > #ifdef CONFIG_CIFS_DFS_UPCALL > struct super_cb_data { > struct TCP_Server_Info *server; > - struct cifs_sb_info *cifs_sb; > + struct super_block *sb; > }; > > /* These functions must be called with server->srv_mutex held */ > @@ -402,25 +402,39 @@ static void super_cb(struct super_block *sb, void *arg) > struct cifs_sb_info *cifs_sb; > struct cifs_tcon *tcon; > > - if (d->cifs_sb) > + if (d->sb) > return; > > cifs_sb = CIFS_SB(sb); > tcon = cifs_sb_master_tcon(cifs_sb); > if (tcon->ses->server == d->server) > - d->cifs_sb = cifs_sb; > + d->sb = sb; > } > > -static inline struct cifs_sb_info * > -find_super_by_tcp(struct TCP_Server_Info *server) > +static struct super_block *get_tcp_super(struct TCP_Server_Info *server) > { > struct super_cb_data d = { > .server = server, > - .cifs_sb = NULL, > + .sb = NULL, > }; > > iterate_supers_type(&cifs_fs_type, super_cb, &d); > - return d.cifs_sb ? d.cifs_sb : ERR_PTR(-ENOENT); > + > + if (unlikely(!d.sb)) > + return ERR_PTR(-ENOENT); > + /* > + * Grab an active reference in order to prevent automounts (DFS links) > + * of expiring and then freeing up our cifs superblock pointer while > + * we're doing failover. > + */ > + cifs_sb_active(d.sb); > + return d.sb; > +} > + > +static inline void put_tcp_super(struct super_block *sb) > +{ > + if (!IS_ERR_OR_NULL(sb)) > + cifs_sb_deactive(sb); > } > > static void reconn_inval_dfs_target(struct TCP_Server_Info *server, > @@ -484,6 +498,7 @@ cifs_reconnect(struct TCP_Server_Info *server) > struct mid_q_entry *mid_entry; > struct list_head retry_list; > #ifdef CONFIG_CIFS_DFS_UPCALL > + struct super_block *sb = NULL; > struct cifs_sb_info *cifs_sb = NULL; > struct dfs_cache_tgt_list tgt_list = {0}; > struct dfs_cache_tgt_iterator *tgt_it = NULL; > @@ -493,13 +508,15 @@ cifs_reconnect(struct TCP_Server_Info *server) > server->nr_targets = 1; > #ifdef CONFIG_CIFS_DFS_UPCALL > spin_unlock(&GlobalMid_Lock); > - cifs_sb = find_super_by_tcp(server); > - if (IS_ERR(cifs_sb)) { > - rc = PTR_ERR(cifs_sb); > + sb = get_tcp_super(server); > + if (IS_ERR(sb)) { > + rc = PTR_ERR(sb); > cifs_dbg(FYI, "%s: will not do DFS failover: rc = %d\n", > __func__, rc); > - cifs_sb = NULL; > + sb = NULL; > } else { > + cifs_sb = CIFS_SB(sb); > + > rc = reconn_setup_dfs_targets(cifs_sb, &tgt_list, &tgt_it); > if (rc && (rc != -EOPNOTSUPP)) { > cifs_server_dbg(VFS, "%s: no target servers for DFS failover\n", > @@ -516,6 +533,10 @@ cifs_reconnect(struct TCP_Server_Info *server) > /* the demux thread will exit normally > next time through the loop */ > spin_unlock(&GlobalMid_Lock); > +#ifdef CONFIG_CIFS_DFS_UPCALL > + dfs_cache_free_tgts(&tgt_list); > + put_tcp_super(sb); > +#endif > return rc; > } else > server->tcpStatus = CifsNeedReconnect; > @@ -642,7 +663,10 @@ cifs_reconnect(struct TCP_Server_Info *server) > __func__, rc); > } > dfs_cache_free_tgts(&tgt_list); > + > } > + > + put_tcp_super(sb); > #endif > if (server->tcpStatus == CifsNeedNegotiate) > mod_delayed_work(cifsiod_wq, &server->echo, 0); > -- > 2.24.0 > -- Thanks, Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() 2019-11-22 15:30 ` [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() Paulo Alcantara (SUSE) 2019-11-25 7:34 ` Steve French @ 2019-11-25 11:36 ` Aurélien Aptel 1 sibling, 0 replies; 19+ messages in thread From: Aurélien Aptel @ 2019-11-25 11:36 UTC (permalink / raw) To: Paulo Alcantara (SUSE), smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) "Paulo Alcantara (SUSE)" <pc@cjr.nz> writes: > 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. Reviewed-by: Aurelien Aptel <aaptel@suse.com> -- Aurélien Aptel / SUSE Labs Samba Team GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/7] cifs: Fix lookup of root ses in DFS referral cache 2019-11-22 15:30 [PATCH 0/7] DFS fixes Paulo Alcantara (SUSE) 2019-11-22 15:30 ` [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() Paulo Alcantara (SUSE) @ 2019-11-22 15:30 ` Paulo Alcantara (SUSE) 2019-11-25 7:33 ` Steve French 2019-11-25 11:37 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 3/7] cifs: Fix potential softlockups while refreshing DFS cache Paulo Alcantara (SUSE) ` (4 subsequent siblings) 6 siblings, 2 replies; 19+ messages in thread From: Paulo Alcantara (SUSE) @ 2019-11-22 15:30 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) We don't care about module aliasing validation in cifs_compose_mount_options(..., is_smb3) when finding the root SMB session of an DFS namespace in order to refresh DFS referral cache. The following issue has been observed when mounting with '-t smb3' and then specifying 'vers=2.0': ... Nov 08 15:27:08 tw kernel: address conversion returned 0 for FS0.WIN.LOCAL Nov 08 15:27:08 tw kernel: [kworke] ==> dns_query((null),FS0.WIN.LOCAL,13,(null)) Nov 08 15:27:08 tw kernel: [kworke] call request_key(,FS0.WIN.LOCAL,) Nov 08 15:27:08 tw kernel: [kworke] ==> dns_resolver_cmp(FS0.WIN.LOCAL,FS0.WIN.LOCAL) Nov 08 15:27:08 tw kernel: [kworke] <== dns_resolver_cmp() = 1 Nov 08 15:27:08 tw kernel: [kworke] <== dns_query() = 13 Nov 08 15:27:08 tw kernel: fs/cifs/dns_resolve.c: dns_resolve_server_name_to_ip: resolved: FS0.WIN.LOCAL to 192.168.30.26 ===> Nov 08 15:27:08 tw kernel: CIFS VFS: vers=2.0 not permitted when mounting with smb3 Nov 08 15:27:08 tw kernel: fs/cifs/dfs_cache.c: CIFS VFS: leaving refresh_tcon (xid = 26) rc = -22 ... Fixes: 5072010ccf05 ("cifs: Fix DFS cache refresher for DFS links") Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/dfs_cache.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c index 1692c0c6c23a..2faa05860a48 100644 --- a/fs/cifs/dfs_cache.c +++ b/fs/cifs/dfs_cache.c @@ -1317,7 +1317,6 @@ static struct cifs_ses *find_root_ses(struct dfs_cache_vol_info *vi, int rc; struct dfs_info3_param ref = {0}; char *mdata = NULL, *devname = NULL; - bool is_smb3 = tcon->ses->server->vals->header_preamble_size == 0; struct TCP_Server_Info *server; struct cifs_ses *ses; struct smb_vol vol; @@ -1344,7 +1343,7 @@ static struct cifs_ses *find_root_ses(struct dfs_cache_vol_info *vi, goto out; } - rc = cifs_setup_volume_info(&vol, mdata, devname, is_smb3); + rc = cifs_setup_volume_info(&vol, mdata, devname, false); kfree(devname); if (rc) { -- 2.24.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/7] cifs: Fix lookup of root ses in DFS referral cache 2019-11-22 15:30 ` [PATCH 2/7] cifs: Fix lookup of root ses in DFS referral cache Paulo Alcantara (SUSE) @ 2019-11-25 7:33 ` Steve French 2019-11-25 11:37 ` Aurélien Aptel 1 sibling, 0 replies; 19+ messages in thread From: Steve French @ 2019-11-25 7:33 UTC (permalink / raw) To: Paulo Alcantara (SUSE); +Cc: CIFS merged into cifs-2.6.git for-next On Fri, Nov 22, 2019 at 9:31 AM Paulo Alcantara (SUSE) <pc@cjr.nz> wrote: > > We don't care about module aliasing validation in > cifs_compose_mount_options(..., is_smb3) when finding the root SMB > session of an DFS namespace in order to refresh DFS referral cache. > > The following issue has been observed when mounting with '-t smb3' and > then specifying 'vers=2.0': > > ... > Nov 08 15:27:08 tw kernel: address conversion returned 0 for FS0.WIN.LOCAL > Nov 08 15:27:08 tw kernel: [kworke] ==> dns_query((null),FS0.WIN.LOCAL,13,(null)) > Nov 08 15:27:08 tw kernel: [kworke] call request_key(,FS0.WIN.LOCAL,) > Nov 08 15:27:08 tw kernel: [kworke] ==> dns_resolver_cmp(FS0.WIN.LOCAL,FS0.WIN.LOCAL) > Nov 08 15:27:08 tw kernel: [kworke] <== dns_resolver_cmp() = 1 > Nov 08 15:27:08 tw kernel: [kworke] <== dns_query() = 13 > Nov 08 15:27:08 tw kernel: fs/cifs/dns_resolve.c: dns_resolve_server_name_to_ip: resolved: FS0.WIN.LOCAL to 192.168.30.26 > ===> Nov 08 15:27:08 tw kernel: CIFS VFS: vers=2.0 not permitted when mounting with smb3 > Nov 08 15:27:08 tw kernel: fs/cifs/dfs_cache.c: CIFS VFS: leaving refresh_tcon (xid = 26) rc = -22 > ... > > Fixes: 5072010ccf05 ("cifs: Fix DFS cache refresher for DFS links") > Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > --- > fs/cifs/dfs_cache.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c > index 1692c0c6c23a..2faa05860a48 100644 > --- a/fs/cifs/dfs_cache.c > +++ b/fs/cifs/dfs_cache.c > @@ -1317,7 +1317,6 @@ static struct cifs_ses *find_root_ses(struct dfs_cache_vol_info *vi, > int rc; > struct dfs_info3_param ref = {0}; > char *mdata = NULL, *devname = NULL; > - bool is_smb3 = tcon->ses->server->vals->header_preamble_size == 0; > struct TCP_Server_Info *server; > struct cifs_ses *ses; > struct smb_vol vol; > @@ -1344,7 +1343,7 @@ static struct cifs_ses *find_root_ses(struct dfs_cache_vol_info *vi, > goto out; > } > > - rc = cifs_setup_volume_info(&vol, mdata, devname, is_smb3); > + rc = cifs_setup_volume_info(&vol, mdata, devname, false); > kfree(devname); > > if (rc) { > -- > 2.24.0 > -- Thanks, Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/7] cifs: Fix lookup of root ses in DFS referral cache 2019-11-22 15:30 ` [PATCH 2/7] cifs: Fix lookup of root ses in DFS referral cache Paulo Alcantara (SUSE) 2019-11-25 7:33 ` Steve French @ 2019-11-25 11:37 ` Aurélien Aptel 1 sibling, 0 replies; 19+ messages in thread From: Aurélien Aptel @ 2019-11-25 11:37 UTC (permalink / raw) To: Paulo Alcantara (SUSE), smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) "Paulo Alcantara (SUSE)" <pc@cjr.nz> writes: > We don't care about module aliasing validation in > cifs_compose_mount_options(..., is_smb3) when finding the root SMB > session of an DFS namespace in order to refresh DFS referral cache. Reviewed-by: Aurelien Aptel <aaptel@suse.com> -- Aurélien Aptel / SUSE Labs Samba Team GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/7] cifs: Fix potential softlockups while refreshing DFS cache 2019-11-22 15:30 [PATCH 0/7] DFS fixes Paulo Alcantara (SUSE) 2019-11-22 15:30 ` [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() Paulo Alcantara (SUSE) 2019-11-22 15:30 ` [PATCH 2/7] cifs: Fix lookup of root ses in DFS referral cache Paulo Alcantara (SUSE) @ 2019-11-22 15:30 ` Paulo Alcantara (SUSE) 2019-11-25 11:41 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 4/7] cifs: Clean up DFS referral cache Paulo Alcantara (SUSE) ` (3 subsequent siblings) 6 siblings, 1 reply; 19+ messages in thread From: Paulo Alcantara (SUSE) @ 2019-11-22 15:30 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE), Aurelien Aptel We used to skip reconnects on all SMB2_IOCTL commands due to SMB3+ FSCTL_VALIDATE_NEGOTIATE_INFO - which made sense since we're still establishing a SMB session. However, when refresh_cache_worker() calls smb2_get_dfs_refer() and we're under reconnect, SMB2_ioctl() will not be able to get a proper status error (e.g. -EHOSTDOWN in case we failed to reconnect) but an -EAGAIN from cifs_send_recv() thus looping forever in refresh_cache_worker(). Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Suggested-by: Aurelien Aptel <aaptel@suse.com> --- fs/cifs/smb2pdu.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index c09ca6963394..3cd90088d8ac 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -252,7 +252,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon) if (tcon == NULL) return 0; - if (smb2_command == SMB2_TREE_CONNECT || smb2_command == SMB2_IOCTL) + if (smb2_command == SMB2_TREE_CONNECT) return 0; if (tcon->tidStatus == CifsExiting) { @@ -426,16 +426,9 @@ fill_small_buf(__le16 smb2_command, struct cifs_tcon *tcon, void *buf, * SMB information in the SMB header. If the return code is zero, this * function must have filled in request_buf pointer. */ -static int -smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon, - void **request_buf, unsigned int *total_len) +static int __smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon, + void **request_buf, unsigned int *total_len) { - int rc; - - rc = smb2_reconnect(smb2_command, tcon); - if (rc) - return rc; - /* BB eventually switch this to SMB2 specific small buf size */ if (smb2_command == SMB2_SET_INFO) *request_buf = cifs_buf_get(); @@ -456,7 +449,31 @@ smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon, cifs_stats_inc(&tcon->num_smbs_sent); } - return rc; + return 0; +} + +static int smb2_plain_req_init(__le16 smb2_command, struct cifs_tcon *tcon, + void **request_buf, unsigned int *total_len) +{ + int rc; + + rc = smb2_reconnect(smb2_command, tcon); + if (rc) + return rc; + + return __smb2_plain_req_init(smb2_command, tcon, request_buf, + total_len); +} + +static int smb2_ioctl_req_init(u32 opcode, struct cifs_tcon *tcon, + void **request_buf, unsigned int *total_len) +{ + /* Skip reconnect only for FSCTL_VALIDATE_NEGOTIATE_INFO IOCTLs */ + if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO) { + return __smb2_plain_req_init(SMB2_IOCTL, tcon, request_buf, + total_len); + } + return smb2_plain_req_init(SMB2_IOCTL, tcon, request_buf, total_len); } /* For explanation of negotiate contexts see MS-SMB2 section 2.2.3.1 */ @@ -2686,7 +2703,7 @@ SMB2_ioctl_init(struct cifs_tcon *tcon, struct smb_rqst *rqst, int rc; char *in_data_buf; - rc = smb2_plain_req_init(SMB2_IOCTL, tcon, (void **) &req, &total_len); + rc = smb2_ioctl_req_init(opcode, tcon, (void **) &req, &total_len); if (rc) return rc; -- 2.24.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] cifs: Fix potential softlockups while refreshing DFS cache 2019-11-22 15:30 ` [PATCH 3/7] cifs: Fix potential softlockups while refreshing DFS cache Paulo Alcantara (SUSE) @ 2019-11-25 11:41 ` Aurélien Aptel 2019-11-25 15:35 ` Steve French 0 siblings, 1 reply; 19+ messages in thread From: Aurélien Aptel @ 2019-11-25 11:41 UTC (permalink / raw) To: Paulo Alcantara (SUSE), smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) "Paulo Alcantara (SUSE)" <pc@cjr.nz> writes: > We used to skip reconnects on all SMB2_IOCTL commands due to SMB3+ > FSCTL_VALIDATE_NEGOTIATE_INFO - which made sense since we're still > establishing a SMB session. > > However, when refresh_cache_worker() calls smb2_get_dfs_refer() and > we're under reconnect, SMB2_ioctl() will not be able to get a proper > status error (e.g. -EHOSTDOWN in case we failed to reconnect) but an > -EAGAIN from cifs_send_recv() thus looping forever in > refresh_cache_worker(). I think we can add a Fixes tag: Fixes: e99c63e4d86d ("SMB3: Fix deadlock in validate negotiate hits reconnect") Otherwise looks good. Reviewed-by: Aurelien Aptel <aaptel@suse.com> -- Aurélien Aptel / SUSE Labs Samba Team GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] cifs: Fix potential softlockups while refreshing DFS cache 2019-11-25 11:41 ` Aurélien Aptel @ 2019-11-25 15:35 ` Steve French 2019-11-25 19:53 ` Pavel Shilovsky 0 siblings, 1 reply; 19+ messages in thread From: Steve French @ 2019-11-25 15:35 UTC (permalink / raw) To: Aurélien Aptel, Pavel Shilovsky; +Cc: Paulo Alcantara (SUSE), CIFS tags added - tentatively merged into cifs-2.6.git for-next pending testing (buildbot) On Mon, Nov 25, 2019 at 5:41 AM Aurélien Aptel <aaptel@suse.com> wrote: > > "Paulo Alcantara (SUSE)" <pc@cjr.nz> writes: > > We used to skip reconnects on all SMB2_IOCTL commands due to SMB3+ > > FSCTL_VALIDATE_NEGOTIATE_INFO - which made sense since we're still > > establishing a SMB session. > > > > However, when refresh_cache_worker() calls smb2_get_dfs_refer() and > > we're under reconnect, SMB2_ioctl() will not be able to get a proper > > status error (e.g. -EHOSTDOWN in case we failed to reconnect) but an > > -EAGAIN from cifs_send_recv() thus looping forever in > > refresh_cache_worker(). > > > I think we can add a Fixes tag: > > Fixes: e99c63e4d86d ("SMB3: Fix deadlock in validate negotiate hits reconnect") > > Otherwise looks good. > > Reviewed-by: Aurelien Aptel <aaptel@suse.com> > > -- > Aurélien Aptel / SUSE Labs Samba Team > GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) -- Thanks, Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/7] cifs: Fix potential softlockups while refreshing DFS cache 2019-11-25 15:35 ` Steve French @ 2019-11-25 19:53 ` Pavel Shilovsky 0 siblings, 0 replies; 19+ messages in thread From: Pavel Shilovsky @ 2019-11-25 19:53 UTC (permalink / raw) To: Steve French; +Cc: Aurélien Aptel, Paulo Alcantara (SUSE), CIFS Stable candidate? -- Best regards, Pavel Shilovsky пн, 25 нояб. 2019 г. в 07:35, Steve French <smfrench@gmail.com>: > > tags added - tentatively merged into cifs-2.6.git for-next pending > testing (buildbot) > > On Mon, Nov 25, 2019 at 5:41 AM Aurélien Aptel <aaptel@suse.com> wrote: > > > > "Paulo Alcantara (SUSE)" <pc@cjr.nz> writes: > > > We used to skip reconnects on all SMB2_IOCTL commands due to SMB3+ > > > FSCTL_VALIDATE_NEGOTIATE_INFO - which made sense since we're still > > > establishing a SMB session. > > > > > > However, when refresh_cache_worker() calls smb2_get_dfs_refer() and > > > we're under reconnect, SMB2_ioctl() will not be able to get a proper > > > status error (e.g. -EHOSTDOWN in case we failed to reconnect) but an > > > -EAGAIN from cifs_send_recv() thus looping forever in > > > refresh_cache_worker(). > > > > > > I think we can add a Fixes tag: > > > > Fixes: e99c63e4d86d ("SMB3: Fix deadlock in validate negotiate hits reconnect") > > > > Otherwise looks good. > > > > Reviewed-by: Aurelien Aptel <aaptel@suse.com> > > > > -- > > Aurélien Aptel / SUSE Labs Samba Team > > GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 > > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE > > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) > > > > -- > Thanks, > > Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/7] cifs: Clean up DFS referral cache 2019-11-22 15:30 [PATCH 0/7] DFS fixes Paulo Alcantara (SUSE) ` (2 preceding siblings ...) 2019-11-22 15:30 ` [PATCH 3/7] cifs: Fix potential softlockups while refreshing DFS cache Paulo Alcantara (SUSE) @ 2019-11-22 15:30 ` Paulo Alcantara (SUSE) 2019-11-25 11:54 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 5/7] cifs: Fix potential deadlock when updating vol in cifs_reconnect() Paulo Alcantara (SUSE) ` (2 subsequent siblings) 6 siblings, 1 reply; 19+ messages in thread From: Paulo Alcantara (SUSE) @ 2019-11-22 15:30 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) This patch does a couple of things: - Do some renaming in static code (cosmetic) - Use rwlock for cache list - Use spinlock for volume list - Avoid lock contention in some places Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/dfs_cache.c | 989 ++++++++++++++++++++++---------------------- 1 file changed, 497 insertions(+), 492 deletions(-) diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c index 2faa05860a48..b082603c793a 100644 --- a/fs/cifs/dfs_cache.c +++ b/fs/cifs/dfs_cache.c @@ -5,8 +5,6 @@ * Copyright (c) 2018-2019 Paulo Alcantara <palcantara@suse.de> */ -#include <linux/rcupdate.h> -#include <linux/rculist.h> #include <linux/jhash.h> #include <linux/ktime.h> #include <linux/slab.h> @@ -22,8 +20,8 @@ #include "dfs_cache.h" -#define DFS_CACHE_HTABLE_SIZE 32 -#define DFS_CACHE_MAX_ENTRIES 64 +#define CACHE_HTABLE_SIZE 32 +#define CACHE_MAX_ENTRIES 64 #define IS_INTERLINK_SET(v) ((v) & (DFSREF_REFERRAL_SERVER | \ DFSREF_STORAGE_SERVER)) @@ -33,55 +31,48 @@ struct dfs_cache_tgt { struct list_head t_list; }; -struct dfs_cache_entry { - struct hlist_node ce_hlist; - const char *ce_path; - int ce_ttl; - int ce_srvtype; - int ce_flags; - struct timespec64 ce_etime; - int ce_path_consumed; - int ce_numtgts; - struct list_head ce_tlist; - struct dfs_cache_tgt *ce_tgthint; - struct rcu_head ce_rcu; +struct cache_entry { + struct hlist_node hlist; + const char *path; + int ttl; + int srvtype; + int flags; + struct timespec64 etime; + int path_consumed; + int numtgts; + struct list_head tlist; + struct dfs_cache_tgt *tgthint; }; -static struct kmem_cache *dfs_cache_slab __read_mostly; - -struct dfs_cache_vol_info { - char *vi_fullpath; - struct smb_vol vi_vol; - char *vi_mntdata; - struct list_head vi_list; +struct vol_info { + char *fullpath; + struct smb_vol smb_vol; + char *mntdata; + struct list_head list; }; -struct dfs_cache { - struct mutex dc_lock; - struct nls_table *dc_nlsc; - struct list_head dc_vol_list; - int dc_ttl; - struct delayed_work dc_refresh; -}; +static struct kmem_cache *cache_slab __read_mostly; +static struct workqueue_struct *dfscache_wq __read_mostly; -static struct dfs_cache dfs_cache; +static atomic_t cache_ttl; +static struct nls_table *cache_nlsc; /* * Number of entries in the cache */ -static size_t dfs_cache_count; +static atomic_t cache_count; -static DEFINE_MUTEX(dfs_cache_list_lock); -static struct hlist_head dfs_cache_htable[DFS_CACHE_HTABLE_SIZE]; +static struct hlist_head cache_htable[CACHE_HTABLE_SIZE]; +static DECLARE_RWSEM(list_sem); + +static LIST_HEAD(vol_list); +static DEFINE_SPINLOCK(vol_lock); static void refresh_cache_worker(struct work_struct *work); -static inline bool is_path_valid(const char *path) -{ - return path && (strchr(path + 1, '\\') || strchr(path + 1, '/')); -} +static DECLARE_DELAYED_WORK(refresh_task, refresh_cache_worker); -static inline int get_normalized_path(const char *path, char **npath) +static int get_normalized_path(const char *path, char **npath) { if (*path == '\\') { *npath = (char *)path; @@ -100,57 +91,48 @@ static inline void free_normalized_path(const char *path, char *npath) kfree(npath); } -static inline bool cache_entry_expired(const struct dfs_cache_entry *ce) +static inline bool cache_entry_expired(const struct cache_entry *ce) { struct timespec64 ts; ktime_get_coarse_real_ts64(&ts); - return timespec64_compare(&ts, &ce->ce_etime) >= 0; + return timespec64_compare(&ts, &ce->etime) >= 0; } -static inline void free_tgts(struct dfs_cache_entry *ce) +static inline void free_tgts(struct cache_entry *ce) { struct dfs_cache_tgt *t, *n; - list_for_each_entry_safe(t, n, &ce->ce_tlist, t_list) { + list_for_each_entry_safe(t, n, &ce->tlist, t_list) { list_del(&t->t_list); kfree(t->t_name); kfree(t); } } -static void free_cache_entry(struct rcu_head *rcu) +static inline void flush_cache_ent(struct cache_entry *ce) { - struct dfs_cache_entry *ce = container_of(rcu, struct dfs_cache_entry, - ce_rcu); - kmem_cache_free(dfs_cache_slab, ce); -} - -static inline void flush_cache_ent(struct dfs_cache_entry *ce) -{ - if (hlist_unhashed(&ce->ce_hlist)) - return; - - hlist_del_init_rcu(&ce->ce_hlist); - kfree_const(ce->ce_path); + hlist_del_init(&ce->hlist); + kfree(ce->path); free_tgts(ce); - dfs_cache_count--; - call_rcu(&ce->ce_rcu, free_cache_entry); + atomic_dec(&cache_count); + kmem_cache_free(cache_slab, ce); } static void flush_cache_ents(void) { int i; - rcu_read_lock(); - for (i = 0; i < DFS_CACHE_HTABLE_SIZE; i++) { - struct hlist_head *l = &dfs_cache_htable[i]; - struct dfs_cache_entry *ce; + for (i = 0; i < CACHE_HTABLE_SIZE; i++) { + struct hlist_head *l = &cache_htable[i]; + struct hlist_node *n; + struct cache_entry *ce; - hlist_for_each_entry_rcu(ce, l, ce_hlist) - flush_cache_ent(ce); + hlist_for_each_entry_safe(ce, n, l, hlist) { + if (!hlist_unhashed(&ce->hlist)) + flush_cache_ent(ce); + } } - rcu_read_unlock(); } /* @@ -158,36 +140,39 @@ static void flush_cache_ents(void) */ static int dfscache_proc_show(struct seq_file *m, void *v) { - int bucket; - struct dfs_cache_entry *ce; + int i; + struct cache_entry *ce; struct dfs_cache_tgt *t; seq_puts(m, "DFS cache\n---------\n"); - mutex_lock(&dfs_cache_list_lock); - - rcu_read_lock(); - hash_for_each_rcu(dfs_cache_htable, bucket, ce, ce_hlist) { - seq_printf(m, - "cache entry: path=%s,type=%s,ttl=%d,etime=%ld," - "interlink=%s,path_consumed=%d,expired=%s\n", - ce->ce_path, - ce->ce_srvtype == DFS_TYPE_ROOT ? "root" : "link", - ce->ce_ttl, ce->ce_etime.tv_nsec, - IS_INTERLINK_SET(ce->ce_flags) ? "yes" : "no", - ce->ce_path_consumed, - cache_entry_expired(ce) ? "yes" : "no"); - - list_for_each_entry(t, &ce->ce_tlist, t_list) { - seq_printf(m, " %s%s\n", - t->t_name, - ce->ce_tgthint == t ? " (target hint)" : ""); + down_read(&list_sem); + for (i = 0; i < CACHE_HTABLE_SIZE; i++) { + struct hlist_head *l = &cache_htable[i]; + + hlist_for_each_entry(ce, l, hlist) { + if (hlist_unhashed(&ce->hlist)) + continue; + + seq_printf(m, + "cache entry: path=%s,type=%s,ttl=%d,etime=%ld," + "interlink=%s,path_consumed=%d,expired=%s\n", + ce->path, + ce->srvtype == DFS_TYPE_ROOT ? "root" : "link", + ce->ttl, ce->etime.tv_nsec, + IS_INTERLINK_SET(ce->flags) ? "yes" : "no", + ce->path_consumed, + cache_entry_expired(ce) ? "yes" : "no"); + + list_for_each_entry(t, &ce->tlist, t_list) { + seq_printf(m, " %s%s\n", + t->t_name, + ce->tgthint == t ? " (target hint)" : ""); + } } - } - rcu_read_unlock(); + up_read(&list_sem); - mutex_unlock(&dfs_cache_list_lock); return 0; } @@ -205,9 +190,10 @@ static ssize_t dfscache_proc_write(struct file *file, const char __user *buffer, return -EINVAL; cifs_dbg(FYI, "clearing dfs cache"); - mutex_lock(&dfs_cache_list_lock); + + down_write(&list_sem); flush_cache_ents(); - mutex_unlock(&dfs_cache_list_lock); + up_write(&list_sem); return count; } @@ -226,25 +212,25 @@ const struct file_operations dfscache_proc_fops = { }; #ifdef CONFIG_CIFS_DEBUG2 -static inline void dump_tgts(const struct dfs_cache_entry *ce) +static inline void dump_tgts(const struct cache_entry *ce) { struct dfs_cache_tgt *t; cifs_dbg(FYI, "target list:\n"); - list_for_each_entry(t, &ce->ce_tlist, t_list) { + list_for_each_entry(t, &ce->tlist, t_list) { cifs_dbg(FYI, " %s%s\n", t->t_name, - ce->ce_tgthint == t ? " (target hint)" : ""); + ce->tgthint == t ? " (target hint)" : ""); } } -static inline void dump_ce(const struct dfs_cache_entry *ce) +static inline void dump_ce(const struct cache_entry *ce) { cifs_dbg(FYI, "cache entry: path=%s,type=%s,ttl=%d,etime=%ld," - "interlink=%s,path_consumed=%d,expired=%s\n", ce->ce_path, - ce->ce_srvtype == DFS_TYPE_ROOT ? "root" : "link", ce->ce_ttl, - ce->ce_etime.tv_nsec, - IS_INTERLINK_SET(ce->ce_flags) ? "yes" : "no", - ce->ce_path_consumed, + "interlink=%s,path_consumed=%d,expired=%s\n", ce->path, + ce->srvtype == DFS_TYPE_ROOT ? "root" : "link", ce->ttl, + ce->etime.tv_nsec, + IS_INTERLINK_SET(ce->flags) ? "yes" : "no", + ce->path_consumed, cache_entry_expired(ce) ? "yes" : "no"); dump_tgts(ce); } @@ -285,24 +271,34 @@ static inline void dump_refs(const struct dfs_info3_param *refs, int numrefs) int dfs_cache_init(void) { int i; + int rc; - dfs_cache_slab = kmem_cache_create("cifs_dfs_cache", - sizeof(struct dfs_cache_entry), 0, - SLAB_HWCACHE_ALIGN, NULL); - if (!dfs_cache_slab) + dfscache_wq = alloc_workqueue("cifs-dfscache", + WQ_FREEZABLE | WQ_MEM_RECLAIM, 1); + if (!dfscache_wq) return -ENOMEM; - for (i = 0; i < DFS_CACHE_HTABLE_SIZE; i++) - INIT_HLIST_HEAD(&dfs_cache_htable[i]); + cache_slab = kmem_cache_create("cifs_dfs_cache", + sizeof(struct cache_entry), 0, + SLAB_HWCACHE_ALIGN, NULL); + if (!cache_slab) { + rc = -ENOMEM; + goto out_destroy_wq; + } + + for (i = 0; i < CACHE_HTABLE_SIZE; i++) + INIT_HLIST_HEAD(&cache_htable[i]); - INIT_LIST_HEAD(&dfs_cache.dc_vol_list); - mutex_init(&dfs_cache.dc_lock); - INIT_DELAYED_WORK(&dfs_cache.dc_refresh, refresh_cache_worker); - dfs_cache.dc_ttl = -1; - dfs_cache.dc_nlsc = load_nls_default(); + atomic_set(&cache_ttl, -1); + atomic_set(&cache_count, 0); + cache_nlsc = load_nls_default(); cifs_dbg(FYI, "%s: initialized DFS referral cache\n", __func__); return 0; + +out_destroy_wq: + destroy_workqueue(dfscache_wq); + return rc; } static inline unsigned int cache_entry_hash(const void *data, int size) @@ -310,7 +306,7 @@ static inline unsigned int cache_entry_hash(const void *data, int size) unsigned int h; h = jhash(data, size, 0); - return h & (DFS_CACHE_HTABLE_SIZE - 1); + return h & (CACHE_HTABLE_SIZE - 1); } /* Check whether second path component of @path is SYSVOL or NETLOGON */ @@ -325,9 +321,9 @@ static inline bool is_sysvol_or_netlogon(const char *path) } /* Return target hint of a DFS cache entry */ -static inline char *get_tgt_name(const struct dfs_cache_entry *ce) +static inline char *get_tgt_name(const struct cache_entry *ce) { - struct dfs_cache_tgt *t = ce->ce_tgthint; + struct dfs_cache_tgt *t = ce->tgthint; return t ? t->t_name : ERR_PTR(-ENOENT); } @@ -346,14 +342,14 @@ static inline struct timespec64 get_expire_time(int ttl) } /* Allocate a new DFS target */ -static inline struct dfs_cache_tgt *alloc_tgt(const char *name) +static inline struct dfs_cache_tgt *alloc_target(const char *name) { struct dfs_cache_tgt *t; - t = kmalloc(sizeof(*t), GFP_KERNEL); + t = kmalloc(sizeof(*t), GFP_ATOMIC); if (!t) return ERR_PTR(-ENOMEM); - t->t_name = kstrndup(name, strlen(name), GFP_KERNEL); + t->t_name = kstrndup(name, strlen(name), GFP_ATOMIC); if (!t->t_name) { kfree(t); return ERR_PTR(-ENOMEM); @@ -367,144 +363,126 @@ static inline struct dfs_cache_tgt *alloc_tgt(const char *name) * target hint. */ static int copy_ref_data(const struct dfs_info3_param *refs, int numrefs, - struct dfs_cache_entry *ce, const char *tgthint) + struct cache_entry *ce, const char *tgthint) { int i; - ce->ce_ttl = refs[0].ttl; - ce->ce_etime = get_expire_time(ce->ce_ttl); - ce->ce_srvtype = refs[0].server_type; - ce->ce_flags = refs[0].ref_flag; - ce->ce_path_consumed = refs[0].path_consumed; + ce->ttl = refs[0].ttl; + ce->etime = get_expire_time(ce->ttl); + ce->srvtype = refs[0].server_type; + ce->flags = refs[0].ref_flag; + ce->path_consumed = refs[0].path_consumed; for (i = 0; i < numrefs; i++) { struct dfs_cache_tgt *t; - t = alloc_tgt(refs[i].node_name); + t = alloc_target(refs[i].node_name); if (IS_ERR(t)) { free_tgts(ce); return PTR_ERR(t); } if (tgthint && !strcasecmp(t->t_name, tgthint)) { - list_add(&t->t_list, &ce->ce_tlist); + list_add(&t->t_list, &ce->tlist); tgthint = NULL; } else { - list_add_tail(&t->t_list, &ce->ce_tlist); + list_add_tail(&t->t_list, &ce->tlist); } - ce->ce_numtgts++; + ce->numtgts++; } - ce->ce_tgthint = list_first_entry_or_null(&ce->ce_tlist, - struct dfs_cache_tgt, t_list); + ce->tgthint = list_first_entry_or_null(&ce->tlist, + struct dfs_cache_tgt, t_list); return 0; } /* Allocate a new cache entry */ -static struct dfs_cache_entry * -alloc_cache_entry(const char *path, const struct dfs_info3_param *refs, - int numrefs) +static struct cache_entry *alloc_cache_entry(const char *path, + struct dfs_info3_param *refs, + int numrefs) { - struct dfs_cache_entry *ce; + struct cache_entry *ce; int rc; - ce = kmem_cache_zalloc(dfs_cache_slab, GFP_KERNEL); + ce = kmem_cache_zalloc(cache_slab, GFP_KERNEL); if (!ce) return ERR_PTR(-ENOMEM); - ce->ce_path = kstrdup_const(path, GFP_KERNEL); - if (!ce->ce_path) { - kmem_cache_free(dfs_cache_slab, ce); + ce->path = kstrndup(path, strlen(path), GFP_KERNEL); + if (!ce->path) { + kmem_cache_free(cache_slab, ce); return ERR_PTR(-ENOMEM); } - INIT_HLIST_NODE(&ce->ce_hlist); - INIT_LIST_HEAD(&ce->ce_tlist); + + INIT_HLIST_NODE(&ce->hlist); + INIT_LIST_HEAD(&ce->tlist); rc = copy_ref_data(refs, numrefs, ce, NULL); if (rc) { - kfree_const(ce->ce_path); - kmem_cache_free(dfs_cache_slab, ce); + kfree(ce->path); + kmem_cache_free(cache_slab, ce); ce = ERR_PTR(rc); } + return ce; } static void remove_oldest_entry(void) { - int bucket; - struct dfs_cache_entry *ce; - struct dfs_cache_entry *to_del = NULL; - - rcu_read_lock(); - hash_for_each_rcu(dfs_cache_htable, bucket, ce, ce_hlist) { - if (!to_del || timespec64_compare(&ce->ce_etime, - &to_del->ce_etime) < 0) - to_del = ce; + int i; + struct cache_entry *ce; + struct cache_entry *to_del = NULL; + + for (i = 0; i < CACHE_HTABLE_SIZE; i++) { + struct hlist_head *l = &cache_htable[i]; + + hlist_for_each_entry(ce, l, hlist) { + if (hlist_unhashed(&ce->hlist)) + continue; + if (!to_del || timespec64_compare(&ce->etime, + &to_del->etime) < 0) + to_del = ce; + } } + if (!to_del) { cifs_dbg(FYI, "%s: no entry to remove", __func__); - goto out; + return; } + cifs_dbg(FYI, "%s: removing entry", __func__); dump_ce(to_del); flush_cache_ent(to_del); -out: - rcu_read_unlock(); } /* Add a new DFS cache entry */ -static inline struct dfs_cache_entry * -add_cache_entry(unsigned int hash, const char *path, - const struct dfs_info3_param *refs, int numrefs) +static int add_cache_entry(const char *path, unsigned int hash, + struct dfs_info3_param *refs, int numrefs) { - struct dfs_cache_entry *ce; + struct cache_entry *ce; + int ttl; ce = alloc_cache_entry(path, refs, numrefs); if (IS_ERR(ce)) - return ce; + return PTR_ERR(ce); - hlist_add_head_rcu(&ce->ce_hlist, &dfs_cache_htable[hash]); - - mutex_lock(&dfs_cache.dc_lock); - if (dfs_cache.dc_ttl < 0) { - dfs_cache.dc_ttl = ce->ce_ttl; - queue_delayed_work(cifsiod_wq, &dfs_cache.dc_refresh, - dfs_cache.dc_ttl * HZ); + ttl = atomic_read(&cache_ttl); + if (ttl < 0) { + ttl = ce->ttl; + queue_delayed_work(dfscache_wq, &refresh_task, ttl * HZ); } else { - dfs_cache.dc_ttl = min_t(int, dfs_cache.dc_ttl, ce->ce_ttl); - mod_delayed_work(cifsiod_wq, &dfs_cache.dc_refresh, - dfs_cache.dc_ttl * HZ); + ttl = min_t(int, ttl, ce->ttl); + mod_delayed_work(dfscache_wq, &refresh_task, ttl * HZ); } - mutex_unlock(&dfs_cache.dc_lock); - return ce; -} + atomic_set(&cache_ttl, ttl); -static struct dfs_cache_entry *__find_cache_entry(unsigned int hash, - const char *path) -{ - struct dfs_cache_entry *ce; - bool found = false; - - rcu_read_lock(); - hlist_for_each_entry_rcu(ce, &dfs_cache_htable[hash], ce_hlist) { - if (!strcasecmp(path, ce->ce_path)) { -#ifdef CONFIG_CIFS_DEBUG2 - char *name = get_tgt_name(ce); + down_write(&list_sem); + hlist_add_head(&ce->hlist, &cache_htable[hash]); + dump_ce(ce); + up_write(&list_sem); - if (IS_ERR(name)) { - rcu_read_unlock(); - return ERR_CAST(name); - } - cifs_dbg(FYI, "%s: cache hit\n", __func__); - cifs_dbg(FYI, "%s: target hint: %s\n", __func__, name); -#endif - found = true; - break; - } - } - rcu_read_unlock(); - return found ? ce : ERR_PTR(-ENOENT); + return 0; } /* @@ -513,33 +491,45 @@ static struct dfs_cache_entry *__find_cache_entry(unsigned int hash, * Use whole path components in the match. * Return ERR_PTR(-ENOENT) if the entry is not found. */ -static inline struct dfs_cache_entry *find_cache_entry(const char *path, - unsigned int *hash) +static struct cache_entry *lookup_cache_entry(const char *path, + unsigned int *hash) { - *hash = cache_entry_hash(path, strlen(path)); - return __find_cache_entry(*hash, path); -} + struct cache_entry *ce; + unsigned int h; + bool found = false; -static inline void destroy_slab_cache(void) -{ - rcu_barrier(); - kmem_cache_destroy(dfs_cache_slab); + h = cache_entry_hash(path, strlen(path)); + + hlist_for_each_entry(ce, &cache_htable[h], hlist) { + if (!strcasecmp(path, ce->path)) { + found = true; + dump_ce(ce); + break; + } + } + + if (!found) + ce = ERR_PTR(-ENOENT); + if (hash) + *hash = h; + + return ce; } -static inline void free_vol(struct dfs_cache_vol_info *vi) +static inline void free_vol(struct vol_info *vi) { - list_del(&vi->vi_list); - kfree(vi->vi_fullpath); - kfree(vi->vi_mntdata); - cifs_cleanup_volume_info_contents(&vi->vi_vol); + list_del(&vi->list); + kfree(vi->fullpath); + kfree(vi->mntdata); + cifs_cleanup_volume_info_contents(&vi->smb_vol); kfree(vi); } static inline void free_vol_list(void) { - struct dfs_cache_vol_info *vi, *nvi; + struct vol_info *vi, *nvi; - list_for_each_entry_safe(vi, nvi, &dfs_cache.dc_vol_list, vi_list) + list_for_each_entry_safe(vi, nvi, &vol_list, list) free_vol(vi); } @@ -548,83 +538,76 @@ static inline void free_vol_list(void) */ void dfs_cache_destroy(void) { - cancel_delayed_work_sync(&dfs_cache.dc_refresh); - unload_nls(dfs_cache.dc_nlsc); + cancel_delayed_work_sync(&refresh_task); + unload_nls(cache_nlsc); free_vol_list(); - mutex_destroy(&dfs_cache.dc_lock); - flush_cache_ents(); - destroy_slab_cache(); - mutex_destroy(&dfs_cache_list_lock); + kmem_cache_destroy(cache_slab); + destroy_workqueue(dfscache_wq); cifs_dbg(FYI, "%s: destroyed DFS referral cache\n", __func__); } -static inline struct dfs_cache_entry * -__update_cache_entry(const char *path, const struct dfs_info3_param *refs, - int numrefs) +static int __update_cache_entry(const char *path, struct dfs_info3_param *refs, + int numrefs) { int rc; - unsigned int h; - struct dfs_cache_entry *ce; + struct cache_entry *ce; char *s, *th = NULL; - ce = find_cache_entry(path, &h); + ce = lookup_cache_entry(path, NULL); if (IS_ERR(ce)) - return ce; + return PTR_ERR(ce); - if (ce->ce_tgthint) { - s = ce->ce_tgthint->t_name; - th = kstrndup(s, strlen(s), GFP_KERNEL); + if (ce->tgthint) { + s = ce->tgthint->t_name; + th = kstrndup(s, strlen(s), GFP_ATOMIC); if (!th) - return ERR_PTR(-ENOMEM); + return -ENOMEM; } free_tgts(ce); - ce->ce_numtgts = 0; + ce->numtgts = 0; rc = copy_ref_data(refs, numrefs, ce, th); - kfree(th); - if (rc) - ce = ERR_PTR(rc); + kfree(th); - return ce; + return 0; } -/* Update an expired cache entry by getting a new DFS referral from server */ -static struct dfs_cache_entry * -update_cache_entry(const unsigned int xid, struct cifs_ses *ses, - const struct nls_table *nls_codepage, int remap, - const char *path, struct dfs_cache_entry *ce) +static int get_dfs_referral(const unsigned int xid, struct cifs_ses *ses, + const struct nls_table *nls_codepage, int remap, + const char *path, struct dfs_info3_param **refs, + int *numrefs) { - int rc; - struct dfs_info3_param *refs = NULL; - int numrefs = 0; + cifs_dbg(FYI, "%s: get an DFS referral for %s\n", __func__, path); - cifs_dbg(FYI, "%s: update expired cache entry\n", __func__); - /* - * Check if caller provided enough parameters to update an expired - * entry. - */ if (!ses || !ses->server || !ses->server->ops->get_dfs_refer) - return ERR_PTR(-ETIME); + return -EOPNOTSUPP; if (unlikely(!nls_codepage)) - return ERR_PTR(-ETIME); + return -EINVAL; - cifs_dbg(FYI, "%s: DFS referral request for %s\n", __func__, path); + *refs = NULL; + *numrefs = 0; - rc = ses->server->ops->get_dfs_refer(xid, ses, path, &refs, &numrefs, - nls_codepage, remap); - if (rc) - ce = ERR_PTR(rc); - else - ce = __update_cache_entry(path, refs, numrefs); + return ses->server->ops->get_dfs_refer(xid, ses, path, refs, numrefs, + nls_codepage, remap); +} - dump_refs(refs, numrefs); - free_dfs_info_array(refs, numrefs); +/* Update an expired cache entry by getting a new DFS referral from server */ +static inline int update_cache_entry(const char *path, + struct dfs_info3_param *refs, + int numrefs) +{ - return ce; + int rc; + + down_write(&list_sem); + rc = __update_cache_entry(path, refs, numrefs); + up_write(&list_sem); + + return rc; } /* @@ -636,95 +619,72 @@ update_cache_entry(const unsigned int xid, struct cifs_ses *ses, * For interlinks, __cifs_dfs_mount() and expand_dfs_referral() are supposed to * handle them properly. */ -static struct dfs_cache_entry * -do_dfs_cache_find(const unsigned int xid, struct cifs_ses *ses, - const struct nls_table *nls_codepage, int remap, - const char *path, bool noreq) +static int __dfs_cache_find(const unsigned int xid, struct cifs_ses *ses, + const struct nls_table *nls_codepage, int remap, + const char *path, bool noreq) { int rc; - unsigned int h; - struct dfs_cache_entry *ce; - struct dfs_info3_param *nrefs; - int numnrefs; + unsigned int hash; + struct cache_entry *ce; + struct dfs_info3_param *refs = NULL; + int numrefs = 0; + bool newent = false; cifs_dbg(FYI, "%s: search path: %s\n", __func__, path); - ce = find_cache_entry(path, &h); - if (IS_ERR(ce)) { - cifs_dbg(FYI, "%s: cache miss\n", __func__); - /* - * If @noreq is set, no requests will be sent to the server for - * either updating or getting a new DFS referral. - */ - if (noreq) - return ce; - /* - * No cache entry was found, so check for valid parameters that - * will be required to get a new DFS referral and then create a - * new cache entry. - */ - if (!ses || !ses->server || !ses->server->ops->get_dfs_refer) { - ce = ERR_PTR(-EOPNOTSUPP); - return ce; - } - if (unlikely(!nls_codepage)) { - ce = ERR_PTR(-EINVAL); - return ce; - } + down_read(&list_sem); - nrefs = NULL; - numnrefs = 0; + ce = lookup_cache_entry(path, &hash); - cifs_dbg(FYI, "%s: DFS referral request for %s\n", __func__, - path); + if (noreq) { + up_read(&list_sem); + return IS_ERR(ce) ? PTR_ERR(ce) : 0; + } - rc = ses->server->ops->get_dfs_refer(xid, ses, path, &nrefs, - &numnrefs, nls_codepage, - remap); - if (rc) { - ce = ERR_PTR(rc); - return ce; + if (!IS_ERR(ce)) { + if (!cache_entry_expired(ce)) { + dump_ce(ce); + up_read(&list_sem); + return 0; } + } else { + newent = true; + } - dump_refs(nrefs, numnrefs); + up_read(&list_sem); - cifs_dbg(FYI, "%s: new cache entry\n", __func__); + rc = get_dfs_referral(xid, ses, nls_codepage, remap, path, + &refs, &numrefs); + if (rc) + return rc; - if (dfs_cache_count >= DFS_CACHE_MAX_ENTRIES) { - cifs_dbg(FYI, "%s: reached max cache size (%d)", - __func__, DFS_CACHE_MAX_ENTRIES); - remove_oldest_entry(); - } - ce = add_cache_entry(h, path, nrefs, numnrefs); - free_dfs_info_array(nrefs, numnrefs); + dump_refs(refs, numrefs); - if (IS_ERR(ce)) - return ce; + if (!newent) { + rc = update_cache_entry(path, refs, numrefs); + goto out_free_refs; + } - dfs_cache_count++; + if (atomic_read(&cache_count) >= CACHE_MAX_ENTRIES) { + cifs_dbg(FYI, "%s: reached max cache size (%d)", __func__, + CACHE_MAX_ENTRIES); + down_write(&list_sem); + remove_oldest_entry(); + up_write(&list_sem); } - dump_ce(ce); + rc = add_cache_entry(path, hash, refs, numrefs); + if (!rc) + atomic_inc(&cache_count); - /* Just return the found cache entry in case @noreq is set */ - if (noreq) - return ce; - - if (cache_entry_expired(ce)) { - cifs_dbg(FYI, "%s: expired cache entry\n", __func__); - ce = update_cache_entry(xid, ses, nls_codepage, remap, path, - ce); - if (IS_ERR(ce)) { - cifs_dbg(FYI, "%s: failed to update expired entry\n", - __func__); - } - } - return ce; +out_free_refs: + free_dfs_info_array(refs, numrefs); + return rc; } -/* Set up a new DFS referral from a given cache entry */ -static int setup_ref(const char *path, const struct dfs_cache_entry *ce, - struct dfs_info3_param *ref, const char *tgt) +/* Set up a DFS referral from a given cache entry */ +static int setup_referral(const char *path, struct cache_entry *ce, + struct dfs_info3_param *ref, const char *target) { int rc; @@ -732,21 +692,20 @@ static int setup_ref(const char *path, const struct dfs_cache_entry *ce, memset(ref, 0, sizeof(*ref)); - ref->path_name = kstrndup(path, strlen(path), GFP_KERNEL); + ref->path_name = kstrndup(path, strlen(path), GFP_ATOMIC); if (!ref->path_name) return -ENOMEM; - ref->path_consumed = ce->ce_path_consumed; - - ref->node_name = kstrndup(tgt, strlen(tgt), GFP_KERNEL); + ref->node_name = kstrndup(target, strlen(target), GFP_ATOMIC); if (!ref->node_name) { rc = -ENOMEM; goto err_free_path; } - ref->ttl = ce->ce_ttl; - ref->server_type = ce->ce_srvtype; - ref->ref_flag = ce->ce_flags; + ref->path_consumed = ce->path_consumed; + ref->ttl = ce->ttl; + ref->server_type = ce->srvtype; + ref->ref_flag = ce->flags; return 0; @@ -757,8 +716,7 @@ static int setup_ref(const char *path, const struct dfs_cache_entry *ce, } /* Return target list of a DFS cache entry */ -static int get_tgt_list(const struct dfs_cache_entry *ce, - struct dfs_cache_tgt_list *tl) +static int get_targets(struct cache_entry *ce, struct dfs_cache_tgt_list *tl) { int rc; struct list_head *head = &tl->tl_list; @@ -768,27 +726,28 @@ static int get_tgt_list(const struct dfs_cache_entry *ce, memset(tl, 0, sizeof(*tl)); INIT_LIST_HEAD(head); - list_for_each_entry(t, &ce->ce_tlist, t_list) { - it = kzalloc(sizeof(*it), GFP_KERNEL); + list_for_each_entry(t, &ce->tlist, t_list) { + it = kzalloc(sizeof(*it), GFP_ATOMIC); if (!it) { rc = -ENOMEM; goto err_free_it; } it->it_name = kstrndup(t->t_name, strlen(t->t_name), - GFP_KERNEL); + GFP_ATOMIC); if (!it->it_name) { kfree(it); rc = -ENOMEM; goto err_free_it; } - if (ce->ce_tgthint == t) + if (ce->tgthint == t) list_add(&it->it_list, head); else list_add_tail(&it->it_list, head); } - tl->tl_numtgts = ce->ce_numtgts; + + tl->tl_numtgts = ce->numtgts; return 0; @@ -829,28 +788,35 @@ int dfs_cache_find(const unsigned int xid, struct cifs_ses *ses, { int rc; char *npath; - struct dfs_cache_entry *ce; - - if (unlikely(!is_path_valid(path))) - return -EINVAL; + struct cache_entry *ce; rc = get_normalized_path(path, &npath); if (rc) return rc; - mutex_lock(&dfs_cache_list_lock); - ce = do_dfs_cache_find(xid, ses, nls_codepage, remap, npath, false); - if (!IS_ERR(ce)) { - if (ref) - rc = setup_ref(path, ce, ref, get_tgt_name(ce)); - else - rc = 0; - if (!rc && tgt_list) - rc = get_tgt_list(ce, tgt_list); - } else { + rc = __dfs_cache_find(xid, ses, nls_codepage, remap, npath, false); + if (rc) + goto out_free_path; + + down_read(&list_sem); + + ce = lookup_cache_entry(npath, NULL); + if (IS_ERR(ce)) { + up_read(&list_sem); rc = PTR_ERR(ce); + goto out_free_path; } - mutex_unlock(&dfs_cache_list_lock); + + if (ref) + rc = setup_referral(path, ce, ref, get_tgt_name(ce)); + else + rc = 0; + if (!rc && tgt_list) + rc = get_targets(ce, tgt_list); + + up_read(&list_sem); + +out_free_path: free_normalized_path(path, npath); return rc; } @@ -876,31 +842,33 @@ int dfs_cache_noreq_find(const char *path, struct dfs_info3_param *ref, { int rc; char *npath; - struct dfs_cache_entry *ce; - - if (unlikely(!is_path_valid(path))) - return -EINVAL; + struct cache_entry *ce; rc = get_normalized_path(path, &npath); if (rc) return rc; - mutex_lock(&dfs_cache_list_lock); - ce = do_dfs_cache_find(0, NULL, NULL, 0, npath, true); + cifs_dbg(FYI, "%s: path: %s\n", __func__, npath); + + down_read(&list_sem); + + ce = lookup_cache_entry(npath, NULL); if (IS_ERR(ce)) { rc = PTR_ERR(ce); - goto out; + goto out_unlock; } if (ref) - rc = setup_ref(path, ce, ref, get_tgt_name(ce)); + rc = setup_referral(path, ce, ref, get_tgt_name(ce)); else rc = 0; if (!rc && tgt_list) - rc = get_tgt_list(ce, tgt_list); -out: - mutex_unlock(&dfs_cache_list_lock); + rc = get_targets(ce, tgt_list); + +out_unlock: + up_read(&list_sem); free_normalized_path(path, npath); + return rc; } @@ -929,44 +897,46 @@ int dfs_cache_update_tgthint(const unsigned int xid, struct cifs_ses *ses, { int rc; char *npath; - struct dfs_cache_entry *ce; + struct cache_entry *ce; struct dfs_cache_tgt *t; - if (unlikely(!is_path_valid(path))) - return -EINVAL; - rc = get_normalized_path(path, &npath); if (rc) return rc; - cifs_dbg(FYI, "%s: path: %s\n", __func__, npath); + cifs_dbg(FYI, "%s: update target hint - path: %s\n", __func__, npath); - mutex_lock(&dfs_cache_list_lock); - ce = do_dfs_cache_find(xid, ses, nls_codepage, remap, npath, false); + rc = __dfs_cache_find(xid, ses, nls_codepage, remap, npath, false); + if (rc) + goto out_free_path; + + down_write(&list_sem); + + ce = lookup_cache_entry(npath, NULL); if (IS_ERR(ce)) { rc = PTR_ERR(ce); - goto out; + goto out_unlock; } - rc = 0; - - t = ce->ce_tgthint; + t = ce->tgthint; if (likely(!strcasecmp(it->it_name, t->t_name))) - goto out; + goto out_unlock; - list_for_each_entry(t, &ce->ce_tlist, t_list) { + list_for_each_entry(t, &ce->tlist, t_list) { if (!strcasecmp(t->t_name, it->it_name)) { - ce->ce_tgthint = t; + ce->tgthint = t; cifs_dbg(FYI, "%s: new target hint: %s\n", __func__, it->it_name); break; } } -out: - mutex_unlock(&dfs_cache_list_lock); +out_unlock: + up_write(&list_sem); +out_free_path: free_normalized_path(path, npath); + return rc; } @@ -989,10 +959,10 @@ int dfs_cache_noreq_update_tgthint(const char *path, { int rc; char *npath; - struct dfs_cache_entry *ce; + struct cache_entry *ce; struct dfs_cache_tgt *t; - if (unlikely(!is_path_valid(path)) || !it) + if (!it) return -EINVAL; rc = get_normalized_path(path, &npath); @@ -1001,33 +971,33 @@ int dfs_cache_noreq_update_tgthint(const char *path, cifs_dbg(FYI, "%s: path: %s\n", __func__, npath); - mutex_lock(&dfs_cache_list_lock); + down_write(&list_sem); - ce = do_dfs_cache_find(0, NULL, NULL, 0, npath, true); + ce = lookup_cache_entry(npath, NULL); if (IS_ERR(ce)) { rc = PTR_ERR(ce); - goto out; + goto out_unlock; } rc = 0; - - t = ce->ce_tgthint; + t = ce->tgthint; if (unlikely(!strcasecmp(it->it_name, t->t_name))) - goto out; + goto out_unlock; - list_for_each_entry(t, &ce->ce_tlist, t_list) { + list_for_each_entry(t, &ce->tlist, t_list) { if (!strcasecmp(t->t_name, it->it_name)) { - ce->ce_tgthint = t; + ce->tgthint = t; cifs_dbg(FYI, "%s: new target hint: %s\n", __func__, it->it_name); break; } } -out: - mutex_unlock(&dfs_cache_list_lock); +out_unlock: + up_write(&list_sem); free_normalized_path(path, npath); + return rc; } @@ -1047,13 +1017,10 @@ int dfs_cache_get_tgt_referral(const char *path, { int rc; char *npath; - struct dfs_cache_entry *ce; - unsigned int h; + struct cache_entry *ce; if (!it || !ref) return -EINVAL; - if (unlikely(!is_path_valid(path))) - return -EINVAL; rc = get_normalized_path(path, &npath); if (rc) @@ -1061,21 +1028,22 @@ int dfs_cache_get_tgt_referral(const char *path, cifs_dbg(FYI, "%s: path: %s\n", __func__, npath); - mutex_lock(&dfs_cache_list_lock); + down_read(&list_sem); - ce = find_cache_entry(npath, &h); + ce = lookup_cache_entry(npath, NULL); if (IS_ERR(ce)) { rc = PTR_ERR(ce); - goto out; + goto out_unlock; } cifs_dbg(FYI, "%s: target name: %s\n", __func__, it->it_name); - rc = setup_ref(path, ce, ref, it->it_name); + rc = setup_referral(path, ce, ref, it->it_name); -out: - mutex_unlock(&dfs_cache_list_lock); +out_unlock: + up_read(&list_sem); free_normalized_path(path, npath); + return rc; } @@ -1085,7 +1053,7 @@ static int dup_vol(struct smb_vol *vol, struct smb_vol *new) if (vol->username) { new->username = kstrndup(vol->username, strlen(vol->username), - GFP_KERNEL); + GFP_KERNEL); if (!new->username) return -ENOMEM; } @@ -1103,7 +1071,7 @@ static int dup_vol(struct smb_vol *vol, struct smb_vol *new) } if (vol->domainname) { new->domainname = kstrndup(vol->domainname, - strlen(vol->domainname), GFP_KERNEL); + strlen(vol->domainname), GFP_KERNEL); if (!new->domainname) goto err_free_unc; } @@ -1150,7 +1118,7 @@ static int dup_vol(struct smb_vol *vol, struct smb_vol *new) int dfs_cache_add_vol(char *mntdata, struct smb_vol *vol, const char *fullpath) { int rc; - struct dfs_cache_vol_info *vi; + struct vol_info *vi; if (!vol || !fullpath || !mntdata) return -EINVAL; @@ -1161,38 +1129,38 @@ int dfs_cache_add_vol(char *mntdata, struct smb_vol *vol, const char *fullpath) if (!vi) return -ENOMEM; - vi->vi_fullpath = kstrndup(fullpath, strlen(fullpath), GFP_KERNEL); - if (!vi->vi_fullpath) { + vi->fullpath = kstrndup(fullpath, strlen(fullpath), GFP_KERNEL); + if (!vi->fullpath) { rc = -ENOMEM; goto err_free_vi; } - rc = dup_vol(vol, &vi->vi_vol); + rc = dup_vol(vol, &vi->smb_vol); if (rc) goto err_free_fullpath; - vi->vi_mntdata = mntdata; + vi->mntdata = mntdata; + + spin_lock(&vol_lock); + list_add_tail(&vi->list, &vol_list); + spin_unlock(&vol_lock); - mutex_lock(&dfs_cache.dc_lock); - list_add_tail(&vi->vi_list, &dfs_cache.dc_vol_list); - mutex_unlock(&dfs_cache.dc_lock); return 0; err_free_fullpath: - kfree(vi->vi_fullpath); + kfree(vi->fullpath); err_free_vi: kfree(vi); return rc; } -static inline struct dfs_cache_vol_info *find_vol(const char *fullpath) +static inline struct vol_info *find_vol(const char *fullpath) { - struct dfs_cache_vol_info *vi; + struct vol_info *vi; - list_for_each_entry(vi, &dfs_cache.dc_vol_list, vi_list) { - cifs_dbg(FYI, "%s: vi->vi_fullpath: %s\n", __func__, - vi->vi_fullpath); - if (!strcasecmp(vi->vi_fullpath, fullpath)) + list_for_each_entry(vi, &vol_list, list) { + cifs_dbg(FYI, "%s: vi->fullpath: %s\n", __func__, vi->fullpath); + if (!strcasecmp(vi->fullpath, fullpath)) return vi; } return ERR_PTR(-ENOENT); @@ -1209,28 +1177,30 @@ static inline struct dfs_cache_vol_info *find_vol(const char *fullpath) int dfs_cache_update_vol(const char *fullpath, struct TCP_Server_Info *server) { int rc; - struct dfs_cache_vol_info *vi; + struct vol_info *vi; if (!fullpath || !server) return -EINVAL; cifs_dbg(FYI, "%s: fullpath: %s\n", __func__, fullpath); - mutex_lock(&dfs_cache.dc_lock); + spin_lock(&vol_lock); vi = find_vol(fullpath); if (IS_ERR(vi)) { rc = PTR_ERR(vi); - goto out; + goto out_unlock; } cifs_dbg(FYI, "%s: updating volume info\n", __func__); - memcpy(&vi->vi_vol.dstaddr, &server->dstaddr, - sizeof(vi->vi_vol.dstaddr)); + memcpy(&vi->smb_vol.dstaddr, &server->dstaddr, + sizeof(vi->smb_vol.dstaddr)); + rc = 0; -out: - mutex_unlock(&dfs_cache.dc_lock); +out_unlock: + spin_unlock(&vol_lock); + cifs_dbg(FYI, "%s: successfully updated vol\n", __func__); return rc; } @@ -1241,18 +1211,20 @@ int dfs_cache_update_vol(const char *fullpath, struct TCP_Server_Info *server) */ void dfs_cache_del_vol(const char *fullpath) { - struct dfs_cache_vol_info *vi; + struct vol_info *vi; if (!fullpath || !*fullpath) return; cifs_dbg(FYI, "%s: fullpath: %s\n", __func__, fullpath); - mutex_lock(&dfs_cache.dc_lock); + spin_lock(&vol_lock); + vi = find_vol(fullpath); if (!IS_ERR(vi)) free_vol(vi); - mutex_unlock(&dfs_cache.dc_lock); + + spin_unlock(&vol_lock); } /* Get all tcons that are within a DFS namespace and can be refreshed */ @@ -1290,7 +1262,7 @@ static inline bool is_dfs_link(const char *path) return !!strchr(s + 1, '\\'); } -static inline char *get_dfs_root(const char *path) +static char *get_dfs_root(const char *path) { char *s, *npath; @@ -1309,12 +1281,37 @@ static inline char *get_dfs_root(const char *path) return npath; } +static inline void put_tcp_server(struct TCP_Server_Info *server) +{ + cifs_put_tcp_session(server, 0); +} + +static struct TCP_Server_Info *get_tcp_server(struct smb_vol *vol) +{ + struct TCP_Server_Info *server; + + server = cifs_find_tcp_session(vol); + if (IS_ERR_OR_NULL(server)) + return NULL; + + spin_lock(&GlobalMid_Lock); + if (server->tcpStatus != CifsGood) { + spin_unlock(&GlobalMid_Lock); + put_tcp_server(server); + return NULL; + } + spin_unlock(&GlobalMid_Lock); + + return server; +} + /* Find root SMB session out of a DFS link path */ -static struct cifs_ses *find_root_ses(struct dfs_cache_vol_info *vi, +static struct cifs_ses *find_root_ses(struct vol_info *vi, struct cifs_tcon *tcon, const char *path) { char *rpath; int rc; + struct cache_entry *ce; struct dfs_info3_param ref = {0}; char *mdata = NULL, *devname = NULL; struct TCP_Server_Info *server; @@ -1325,15 +1322,25 @@ static struct cifs_ses *find_root_ses(struct dfs_cache_vol_info *vi, if (IS_ERR(rpath)) return ERR_CAST(rpath); - memset(&vol, 0, sizeof(vol)); + down_read(&list_sem); - rc = dfs_cache_noreq_find(rpath, &ref, NULL); + ce = lookup_cache_entry(rpath, NULL); + if (IS_ERR(ce)) { + up_read(&list_sem); + ses = ERR_CAST(ce); + goto out; + } + + rc = setup_referral(path, ce, &ref, get_tgt_name(ce)); if (rc) { + up_read(&list_sem); ses = ERR_PTR(rc); goto out; } - mdata = cifs_compose_mount_options(vi->vi_mntdata, rpath, &ref, + up_read(&list_sem); + + mdata = cifs_compose_mount_options(vi->mntdata, rpath, &ref, &devname); free_dfs_info_param(&ref); @@ -1351,13 +1358,8 @@ static struct cifs_ses *find_root_ses(struct dfs_cache_vol_info *vi, goto out; } - server = cifs_find_tcp_session(&vol); - if (IS_ERR_OR_NULL(server)) { - ses = ERR_PTR(-EHOSTDOWN); - goto out; - } - if (server->tcpStatus != CifsGood) { - cifs_put_tcp_session(server, 0); + server = get_tcp_server(&vol); + if (!server) { ses = ERR_PTR(-EHOSTDOWN); goto out; } @@ -1373,17 +1375,15 @@ static struct cifs_ses *find_root_ses(struct dfs_cache_vol_info *vi, } /* Refresh DFS cache entry from a given tcon */ -static void do_refresh_tcon(struct dfs_cache *dc, struct dfs_cache_vol_info *vi, - struct cifs_tcon *tcon) +static int refresh_tcon(struct vol_info *vi, struct cifs_tcon *tcon) { int rc = 0; unsigned int xid; char *path, *npath; - unsigned int h; - struct dfs_cache_entry *ce; + struct cache_entry *ce; + struct cifs_ses *root_ses = NULL, *ses; struct dfs_info3_param *refs = NULL; int numrefs = 0; - struct cifs_ses *root_ses = NULL, *ses; xid = get_xid(); @@ -1391,19 +1391,23 @@ static void do_refresh_tcon(struct dfs_cache *dc, struct dfs_cache_vol_info *vi, rc = get_normalized_path(path, &npath); if (rc) - goto out; + goto out_free_xid; - mutex_lock(&dfs_cache_list_lock); - ce = find_cache_entry(npath, &h); - mutex_unlock(&dfs_cache_list_lock); + down_read(&list_sem); + ce = lookup_cache_entry(npath, NULL); if (IS_ERR(ce)) { rc = PTR_ERR(ce); - goto out; + up_read(&list_sem); + goto out_free_path; } - if (!cache_entry_expired(ce)) - goto out; + if (!cache_entry_expired(ce)) { + up_read(&list_sem); + goto out_free_path; + } + + up_read(&list_sem); /* If it's a DFS Link, then use root SMB session for refreshing it */ if (is_dfs_link(npath)) { @@ -1411,35 +1415,29 @@ static void do_refresh_tcon(struct dfs_cache *dc, struct dfs_cache_vol_info *vi, if (IS_ERR(ses)) { rc = PTR_ERR(ses); root_ses = NULL; - goto out; + goto out_free_path; } } else { ses = tcon->ses; } - if (unlikely(!ses->server->ops->get_dfs_refer)) { - rc = -EOPNOTSUPP; - } else { - rc = ses->server->ops->get_dfs_refer(xid, ses, path, &refs, - &numrefs, dc->dc_nlsc, - tcon->remap); - if (!rc) { - mutex_lock(&dfs_cache_list_lock); - ce = __update_cache_entry(npath, refs, numrefs); - mutex_unlock(&dfs_cache_list_lock); - dump_refs(refs, numrefs); - free_dfs_info_array(refs, numrefs); - if (IS_ERR(ce)) - rc = PTR_ERR(ce); - } + rc = get_dfs_referral(xid, ses, cache_nlsc, tcon->remap, npath, &refs, + &numrefs); + if (!rc) { + dump_refs(refs, numrefs); + rc = update_cache_entry(npath, refs, numrefs); + free_dfs_info_array(refs, numrefs); } -out: if (root_ses) cifs_put_smb_ses(root_ses); - free_xid(xid); +out_free_path: free_normalized_path(path, npath); + +out_free_xid: + free_xid(xid); + return rc; } /* @@ -1448,30 +1446,37 @@ static void do_refresh_tcon(struct dfs_cache *dc, struct dfs_cache_vol_info *vi, */ static void refresh_cache_worker(struct work_struct *work) { - struct dfs_cache *dc = container_of(work, struct dfs_cache, - dc_refresh.work); - struct dfs_cache_vol_info *vi; + struct vol_info *vi; struct TCP_Server_Info *server; - LIST_HEAD(list); + LIST_HEAD(tcons); struct cifs_tcon *tcon, *ntcon; + int rc; - mutex_lock(&dc->dc_lock); - - list_for_each_entry(vi, &dc->dc_vol_list, vi_list) { - server = cifs_find_tcp_session(&vi->vi_vol); - if (IS_ERR_OR_NULL(server)) + spin_lock(&vol_lock); + list_for_each_entry(vi, &vol_list, list) { + server = get_tcp_server(&vi->smb_vol); + if (!server) continue; - if (server->tcpStatus != CifsGood) - goto next; - get_tcons(server, &list); - list_for_each_entry_safe(tcon, ntcon, &list, ulist) { - do_refresh_tcon(dc, vi, tcon); + + get_tcons(server, &tcons); + rc = 0; + + list_for_each_entry_safe(tcon, ntcon, &tcons, ulist) { + /* + * Skip tcp server if any of its tcons failed to refresh + * (possibily due to reconnects). + */ + if (!rc) + rc = refresh_tcon(vi, tcon); + list_del_init(&tcon->ulist); cifs_put_tcon(tcon); } -next: - cifs_put_tcp_session(server, 0); + + put_tcp_server(server); } - queue_delayed_work(cifsiod_wq, &dc->dc_refresh, dc->dc_ttl * HZ); - mutex_unlock(&dc->dc_lock); + spin_unlock(&vol_lock); + + queue_delayed_work(dfscache_wq, &refresh_task, + atomic_read(&cache_ttl) * HZ); } -- 2.24.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 4/7] cifs: Clean up DFS referral cache 2019-11-22 15:30 ` [PATCH 4/7] cifs: Clean up DFS referral cache Paulo Alcantara (SUSE) @ 2019-11-25 11:54 ` Aurélien Aptel 0 siblings, 0 replies; 19+ messages in thread From: Aurélien Aptel @ 2019-11-25 11:54 UTC (permalink / raw) To: Paulo Alcantara (SUSE), smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) "Paulo Alcantara (SUSE)" <pc@cjr.nz> writes: > This patch does a couple of things: > > - Do some renaming in static code (cosmetic) > - Use rwlock for cache list > - Use spinlock for volume list > - Avoid lock contention in some places There is too much going on here for a proper review but nothing major caught my eye (cosmetic should have been separate imo). > - ce = find_cache_entry(path, &h); > - if (IS_ERR(ce)) { > - cifs_dbg(FYI, "%s: cache miss\n", __func__); > - /* > - * If @noreq is set, no requests will be sent to the server for > - * either updating or getting a new DFS referral. > - */ > - if (noreq) > - return ce; > - /* > - * No cache entry was found, so check for valid parameters that > - * will be required to get a new DFS referral and then create a > - * new cache entry. > - */ I think we should keep comments. Cheers, -- Aurélien Aptel / SUSE Labs Samba Team GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/7] cifs: Fix potential deadlock when updating vol in cifs_reconnect() 2019-11-22 15:30 [PATCH 0/7] DFS fixes Paulo Alcantara (SUSE) ` (3 preceding siblings ...) 2019-11-22 15:30 ` [PATCH 4/7] cifs: Clean up DFS referral cache Paulo Alcantara (SUSE) @ 2019-11-22 15:30 ` Paulo Alcantara (SUSE) 2019-11-25 12:01 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 6/7] cifs: Fix retrieval of DFS referrals in cifs_mount() Paulo Alcantara (SUSE) 2019-11-22 15:30 ` [PATCH 7/7] cifs: Always update signing key of first channel Paulo Alcantara (SUSE) 6 siblings, 1 reply; 19+ messages in thread From: Paulo Alcantara (SUSE) @ 2019-11-22 15:30 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) We can't hold the vol_lock spinlock while refreshing the DFS cache because cifs_reconnect() may call dfs_cache_update_vol() while we are walking through the volume list. Create a temp list with all volumes eligible for refreshing and then use it without any locks held. Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/dfs_cache.c | 45 +++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c index b082603c793a..5b9d7281dd67 100644 --- a/fs/cifs/dfs_cache.c +++ b/fs/cifs/dfs_cache.c @@ -49,6 +49,8 @@ struct vol_info { struct smb_vol smb_vol; char *mntdata; struct list_head list; + struct list_head rlist; + int vol_count; }; static struct kmem_cache *cache_slab __read_mostly; @@ -516,13 +518,15 @@ static struct cache_entry *lookup_cache_entry(const char *path, return ce; } -static inline void free_vol(struct vol_info *vi) +static void put_vol(struct vol_info *vi) { - list_del(&vi->list); - kfree(vi->fullpath); - kfree(vi->mntdata); - cifs_cleanup_volume_info_contents(&vi->smb_vol); - kfree(vi); + if (!--vi->vol_count) { + list_del_init(&vi->list); + kfree(vi->fullpath); + kfree(vi->mntdata); + cifs_cleanup_volume_info_contents(&vi->smb_vol); + kfree(vi); + } } static inline void free_vol_list(void) @@ -530,7 +534,7 @@ static inline void free_vol_list(void) struct vol_info *vi, *nvi; list_for_each_entry_safe(vi, nvi, &vol_list, list) - free_vol(vi); + put_vol(vi); } /** @@ -1140,6 +1144,7 @@ int dfs_cache_add_vol(char *mntdata, struct smb_vol *vol, const char *fullpath) goto err_free_fullpath; vi->mntdata = mntdata; + vi->vol_count++; spin_lock(&vol_lock); list_add_tail(&vi->list, &vol_list); @@ -1219,11 +1224,9 @@ void dfs_cache_del_vol(const char *fullpath) cifs_dbg(FYI, "%s: fullpath: %s\n", __func__, fullpath); spin_lock(&vol_lock); - vi = find_vol(fullpath); if (!IS_ERR(vi)) - free_vol(vi); - + put_vol(vi); spin_unlock(&vol_lock); } @@ -1446,18 +1449,31 @@ static int refresh_tcon(struct vol_info *vi, struct cifs_tcon *tcon) */ static void refresh_cache_worker(struct work_struct *work) { - struct vol_info *vi; + struct vol_info *vi, *nvi; struct TCP_Server_Info *server; + LIST_HEAD(vols); LIST_HEAD(tcons); struct cifs_tcon *tcon, *ntcon; int rc; + /* Get SMB volumes that are eligible (CifsGood) for refreshing */ spin_lock(&vol_lock); list_for_each_entry(vi, &vol_list, list) { server = get_tcp_server(&vi->smb_vol); if (!server) continue; + vi->vol_count++; + list_add_tail(&vi->rlist, &vols); + put_tcp_server(server); + } + spin_unlock(&vol_lock); + + list_for_each_entry_safe(vi, nvi, &vols, rlist) { + server = get_tcp_server(&vi->smb_vol); + if (!server) + goto next_vol; + get_tcons(server, &tcons); rc = 0; @@ -1474,8 +1490,13 @@ static void refresh_cache_worker(struct work_struct *work) } put_tcp_server(server); + +next_vol: + list_del_init(&vi->rlist); + spin_lock(&vol_lock); + put_vol(vi); + spin_unlock(&vol_lock); } - spin_unlock(&vol_lock); queue_delayed_work(dfscache_wq, &refresh_task, atomic_read(&cache_ttl) * HZ); -- 2.24.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 5/7] cifs: Fix potential deadlock when updating vol in cifs_reconnect() 2019-11-22 15:30 ` [PATCH 5/7] cifs: Fix potential deadlock when updating vol in cifs_reconnect() Paulo Alcantara (SUSE) @ 2019-11-25 12:01 ` Aurélien Aptel 0 siblings, 0 replies; 19+ messages in thread From: Aurélien Aptel @ 2019-11-25 12:01 UTC (permalink / raw) To: Paulo Alcantara (SUSE), smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) "Paulo Alcantara (SUSE)" <pc@cjr.nz> writes: > We can't hold the vol_lock spinlock while refreshing the DFS cache > because cifs_reconnect() may call dfs_cache_update_vol() while we are > walking through the volume list. > > Create a temp list with all volumes eligible for refreshing and then > use it without any locks held. Commit msg should mention it makes the vol_info refcounted. > Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > --- > fs/cifs/dfs_cache.c | 45 +++++++++++++++++++++++++++++++++------------ > 1 file changed, 33 insertions(+), 12 deletions(-) > > diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c > index b082603c793a..5b9d7281dd67 100644 > --- a/fs/cifs/dfs_cache.c > +++ b/fs/cifs/dfs_cache.c > @@ -49,6 +49,8 @@ struct vol_info { > struct smb_vol smb_vol; > char *mntdata; > struct list_head list; > + struct list_head rlist; > + int vol_count; > }; > > static struct kmem_cache *cache_slab __read_mostly; > @@ -516,13 +518,15 @@ static struct cache_entry *lookup_cache_entry(const char *path, > return ce; > } > > -static inline void free_vol(struct vol_info *vi) > +static void put_vol(struct vol_info *vi) > { > - list_del(&vi->list); > - kfree(vi->fullpath); > - kfree(vi->mntdata); > - cifs_cleanup_volume_info_contents(&vi->smb_vol); > - kfree(vi); > + if (!--vi->vol_count) { > + list_del_init(&vi->list); > + kfree(vi->fullpath); > + kfree(vi->mntdata); > + cifs_cleanup_volume_info_contents(&vi->smb_vol); > + kfree(vi); > + } > } Can we document that put_vol() assumes vol_lock is held? -- Aurélien Aptel / SUSE Labs Samba Team GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 6/7] cifs: Fix retrieval of DFS referrals in cifs_mount() 2019-11-22 15:30 [PATCH 0/7] DFS fixes Paulo Alcantara (SUSE) ` (4 preceding siblings ...) 2019-11-22 15:30 ` [PATCH 5/7] cifs: Fix potential deadlock when updating vol in cifs_reconnect() Paulo Alcantara (SUSE) @ 2019-11-22 15:30 ` Paulo Alcantara (SUSE) 2019-11-25 7:38 ` Steve French 2019-11-22 15:30 ` [PATCH 7/7] cifs: Always update signing key of first channel Paulo Alcantara (SUSE) 6 siblings, 1 reply; 19+ messages in thread From: Paulo Alcantara (SUSE) @ 2019-11-22 15:30 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE), stable, Matthew Ruffell Make sure that DFS referrals are sent to newly resolved root targets as in a multi tier DFS setup. Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Link: https://lkml.kernel.org/r/05aa2995-e85e-0ff4-d003-5bb08bd17a22@canonical.com Cc: stable@vger.kernel.org Tested-by: Matthew Ruffell <matthew.ruffell@canonical.com> --- fs/cifs/connect.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 668d477cc9c7..86d98d73749d 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -4776,6 +4776,17 @@ static int is_path_remote(struct cifs_sb_info *cifs_sb, struct smb_vol *vol, } #ifdef CONFIG_CIFS_DFS_UPCALL +static inline void set_root_tcon(struct cifs_sb_info *cifs_sb, + struct cifs_tcon *tcon, + struct cifs_tcon **root) +{ + spin_lock(&cifs_tcp_ses_lock); + tcon->tc_count++; + tcon->remap = cifs_remap(cifs_sb); + spin_unlock(&cifs_tcp_ses_lock); + *root = tcon; +} + int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *vol) { int rc = 0; @@ -4877,18 +4888,10 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *vol) /* Cache out resolved root server */ (void)dfs_cache_find(xid, ses, cifs_sb->local_nls, cifs_remap(cifs_sb), root_path + 1, NULL, NULL); - /* - * Save root tcon for additional DFS requests to update or create a new - * DFS cache entry, or even perform DFS failover. - */ - spin_lock(&cifs_tcp_ses_lock); - tcon->tc_count++; - tcon->dfs_path = root_path; + kfree(root_path); root_path = NULL; - tcon->remap = cifs_remap(cifs_sb); - spin_unlock(&cifs_tcp_ses_lock); - root_tcon = tcon; + set_root_tcon(cifs_sb, tcon, &root_tcon); for (count = 1; ;) { if (!rc && tcon) { @@ -4925,6 +4928,15 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *vol) mount_put_conns(cifs_sb, xid, server, ses, tcon); rc = mount_get_conns(vol, cifs_sb, &xid, &server, &ses, &tcon); + /* + * Ensure that DFS referrals go through new root server. + */ + if (!rc && tcon && + (tcon->share_flags & (SHI1005_FLAGS_DFS | + SHI1005_FLAGS_DFS_ROOT))) { + cifs_put_tcon(root_tcon); + set_root_tcon(cifs_sb, tcon, &root_tcon); + } } if (rc) { if (rc == -EACCES || rc == -EOPNOTSUPP) -- 2.24.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 6/7] cifs: Fix retrieval of DFS referrals in cifs_mount() 2019-11-22 15:30 ` [PATCH 6/7] cifs: Fix retrieval of DFS referrals in cifs_mount() Paulo Alcantara (SUSE) @ 2019-11-25 7:38 ` Steve French 0 siblings, 0 replies; 19+ messages in thread From: Steve French @ 2019-11-25 7:38 UTC (permalink / raw) To: Paulo Alcantara (SUSE); +Cc: CIFS, Stable, Matthew Ruffell merged into cifs-2.6.git for-next On Fri, Nov 22, 2019 at 9:31 AM Paulo Alcantara (SUSE) <pc@cjr.nz> wrote: > > Make sure that DFS referrals are sent to newly resolved root targets > as in a multi tier DFS setup. > > Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > Link: https://lkml.kernel.org/r/05aa2995-e85e-0ff4-d003-5bb08bd17a22@canonical.com > Cc: stable@vger.kernel.org > Tested-by: Matthew Ruffell <matthew.ruffell@canonical.com> > --- > fs/cifs/connect.c | 32 ++++++++++++++++++++++---------- > 1 file changed, 22 insertions(+), 10 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 668d477cc9c7..86d98d73749d 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -4776,6 +4776,17 @@ static int is_path_remote(struct cifs_sb_info *cifs_sb, struct smb_vol *vol, > } > > #ifdef CONFIG_CIFS_DFS_UPCALL > +static inline void set_root_tcon(struct cifs_sb_info *cifs_sb, > + struct cifs_tcon *tcon, > + struct cifs_tcon **root) > +{ > + spin_lock(&cifs_tcp_ses_lock); > + tcon->tc_count++; > + tcon->remap = cifs_remap(cifs_sb); > + spin_unlock(&cifs_tcp_ses_lock); > + *root = tcon; > +} > + > int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *vol) > { > int rc = 0; > @@ -4877,18 +4888,10 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *vol) > /* Cache out resolved root server */ > (void)dfs_cache_find(xid, ses, cifs_sb->local_nls, cifs_remap(cifs_sb), > root_path + 1, NULL, NULL); > - /* > - * Save root tcon for additional DFS requests to update or create a new > - * DFS cache entry, or even perform DFS failover. > - */ > - spin_lock(&cifs_tcp_ses_lock); > - tcon->tc_count++; > - tcon->dfs_path = root_path; > + kfree(root_path); > root_path = NULL; > - tcon->remap = cifs_remap(cifs_sb); > - spin_unlock(&cifs_tcp_ses_lock); > > - root_tcon = tcon; > + set_root_tcon(cifs_sb, tcon, &root_tcon); > > for (count = 1; ;) { > if (!rc && tcon) { > @@ -4925,6 +4928,15 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *vol) > mount_put_conns(cifs_sb, xid, server, ses, tcon); > rc = mount_get_conns(vol, cifs_sb, &xid, &server, &ses, > &tcon); > + /* > + * Ensure that DFS referrals go through new root server. > + */ > + if (!rc && tcon && > + (tcon->share_flags & (SHI1005_FLAGS_DFS | > + SHI1005_FLAGS_DFS_ROOT))) { > + cifs_put_tcon(root_tcon); > + set_root_tcon(cifs_sb, tcon, &root_tcon); > + } > } > if (rc) { > if (rc == -EACCES || rc == -EOPNOTSUPP) > -- > 2.24.0 > -- Thanks, Steve ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 7/7] cifs: Always update signing key of first channel 2019-11-22 15:30 [PATCH 0/7] DFS fixes Paulo Alcantara (SUSE) ` (5 preceding siblings ...) 2019-11-22 15:30 ` [PATCH 6/7] cifs: Fix retrieval of DFS referrals in cifs_mount() Paulo Alcantara (SUSE) @ 2019-11-22 15:30 ` Paulo Alcantara (SUSE) 2019-11-25 15:48 ` Aurélien Aptel 6 siblings, 1 reply; 19+ messages in thread From: Paulo Alcantara (SUSE) @ 2019-11-22 15:30 UTC (permalink / raw) To: smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) Update signing key of first channel whenever generating the master sigining/encryption/decryption keys rather than only in cifs_mount(). This also fixes reconnect when re-establishing smb sessions to other servers. Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/smb2transport.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c index 86501239cef5..387c88704c52 100644 --- a/fs/cifs/smb2transport.c +++ b/fs/cifs/smb2transport.c @@ -407,6 +407,10 @@ generate_smb3signingkey(struct cifs_ses *ses, SMB3_SIGN_KEY_SIZE); if (rc) return rc; + + memcpy(ses->chans[0].signkey, ses->smb3signingkey, + SMB3_SIGN_KEY_SIZE); + rc = generate_key(ses, ptriplet->encryption.label, ptriplet->encryption.context, ses->smb3encryptionkey, -- 2.24.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 7/7] cifs: Always update signing key of first channel 2019-11-22 15:30 ` [PATCH 7/7] cifs: Always update signing key of first channel Paulo Alcantara (SUSE) @ 2019-11-25 15:48 ` Aurélien Aptel 0 siblings, 0 replies; 19+ messages in thread From: Aurélien Aptel @ 2019-11-25 15:48 UTC (permalink / raw) To: Paulo Alcantara (SUSE), smfrench; +Cc: linux-cifs, Paulo Alcantara (SUSE) "Paulo Alcantara (SUSE)" <pc@cjr.nz> writes: > Update signing key of first channel whenever generating the master > sigining/encryption/decryption keys rather than only in cifs_mount(). Reviewed-by: Aurelien Aptel <aaptel@suse.com> -- Aurélien Aptel / SUSE Labs Samba Team GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-11-25 19:53 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-22 15:30 [PATCH 0/7] DFS fixes Paulo Alcantara (SUSE) 2019-11-22 15:30 ` [PATCH 1/7] cifs: Fix use-after-free bug in cifs_reconnect() Paulo Alcantara (SUSE) 2019-11-25 7:34 ` Steve French 2019-11-25 11:36 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 2/7] cifs: Fix lookup of root ses in DFS referral cache Paulo Alcantara (SUSE) 2019-11-25 7:33 ` Steve French 2019-11-25 11:37 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 3/7] cifs: Fix potential softlockups while refreshing DFS cache Paulo Alcantara (SUSE) 2019-11-25 11:41 ` Aurélien Aptel 2019-11-25 15:35 ` Steve French 2019-11-25 19:53 ` Pavel Shilovsky 2019-11-22 15:30 ` [PATCH 4/7] cifs: Clean up DFS referral cache Paulo Alcantara (SUSE) 2019-11-25 11:54 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 5/7] cifs: Fix potential deadlock when updating vol in cifs_reconnect() Paulo Alcantara (SUSE) 2019-11-25 12:01 ` Aurélien Aptel 2019-11-22 15:30 ` [PATCH 6/7] cifs: Fix retrieval of DFS referrals in cifs_mount() Paulo Alcantara (SUSE) 2019-11-25 7:38 ` Steve French 2019-11-22 15:30 ` [PATCH 7/7] cifs: Always update signing key of first channel Paulo Alcantara (SUSE) 2019-11-25 15:48 ` Aurélien Aptel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).