* [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts()
@ 2022-04-21 0:05 Paulo Alcantara
2022-04-21 0:05 ` [PATCH 2/2] cifs: use correct lock type in cifs_reconnect() Paulo Alcantara
2022-04-21 2:38 ` [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts() Leif Sahlberg
0 siblings, 2 replies; 6+ messages in thread
From: Paulo Alcantara @ 2022-04-21 0:05 UTC (permalink / raw)
To: linux-cifs, smfrench; +Cc: Paulo Alcantara
Either mount(2) or automount might not have server->origin_fullpath
set yet while refresh_cache_worker() is attempting to refresh DFS
referrals. Add missing NULL check and locking around it.
This fixes bellow crash:
[ 1070.276835] general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI
[ 1070.277676] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
[ 1070.278219] CPU: 1 PID: 8506 Comm: kworker/u8:1 Not tainted 5.18.0-rc3 #10
[ 1070.278701] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014
[ 1070.279495] Workqueue: cifs-dfscache refresh_cache_worker [cifs]
[ 1070.280044] RIP: 0010:strcasecmp+0x34/0x150
[ 1070.280359] Code: 00 00 00 fc ff df 41 54 55 48 89 fd 53 48 83 ec 10 eb 03 4c 89 fe 48 89 ef 48 83 c5 01 48 89 f8 48 89 fa 48 c1 e8 03 83 e2 07 <42> 0f b6 04 28 38 d0 7f 08 84 c0 0f 85 bc 00 00 00 0f b6 45 ff 44
[ 1070.281729] RSP: 0018:ffffc90008367958 EFLAGS: 00010246
[ 1070.282114] RAX: 0000000000000000 RBX: dffffc0000000000 RCX: 0000000000000000
[ 1070.282691] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 1070.283273] RBP: 0000000000000001 R08: 0000000000000000 R09: ffffffff873eda27
[ 1070.283857] R10: ffffc900083679a0 R11: 0000000000000001 R12: ffff88812624c000
[ 1070.284436] R13: dffffc0000000000 R14: ffff88810e6e9a88 R15: ffff888119bb9000
[ 1070.284990] FS: 0000000000000000(0000) GS:ffff888151200000(0000) knlGS:0000000000000000
[ 1070.285625] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1070.286100] CR2: 0000561a4d922418 CR3: 000000010aecc000 CR4: 0000000000350ee0
[ 1070.286683] Call Trace:
[ 1070.286890] <TASK>
[ 1070.287070] refresh_cache_worker+0x895/0xd20 [cifs]
[ 1070.287475] ? __refresh_tcon.isra.0+0xfb0/0xfb0 [cifs]
[ 1070.287905] ? __lock_acquire+0xcd1/0x6960
[ 1070.288247] ? is_dynamic_key+0x1a0/0x1a0
[ 1070.288591] ? lockdep_hardirqs_on_prepare+0x410/0x410
[ 1070.289012] ? lock_downgrade+0x6f0/0x6f0
[ 1070.289318] process_one_work+0x7bd/0x12d0
[ 1070.289637] ? worker_thread+0x160/0xec0
[ 1070.289970] ? pwq_dec_nr_in_flight+0x230/0x230
[ 1070.290318] ? _raw_spin_lock_irq+0x5e/0x90
[ 1070.290619] worker_thread+0x5ac/0xec0
[ 1070.290891] ? process_one_work+0x12d0/0x12d0
[ 1070.291199] kthread+0x2a5/0x350
[ 1070.291430] ? kthread_complete_and_exit+0x20/0x20
[ 1070.291770] ret_from_fork+0x22/0x30
[ 1070.292050] </TASK>
[ 1070.292223] Modules linked in: bpfilter cifs cifs_arc4 cifs_md4
[ 1070.292765] ---[ end trace 0000000000000000 ]---
[ 1070.293108] RIP: 0010:strcasecmp+0x34/0x150
[ 1070.293471] Code: 00 00 00 fc ff df 41 54 55 48 89 fd 53 48 83 ec 10 eb 03 4c 89 fe 48 89 ef 48 83 c5 01 48 89 f8 48 89 fa 48 c1 e8 03 83 e2 07 <42> 0f b6 04 28 38 d0 7f 08 84 c0 0f 85 bc 00 00 00 0f b6 45 ff 44
[ 1070.297718] RSP: 0018:ffffc90008367958 EFLAGS: 00010246
[ 1070.298622] RAX: 0000000000000000 RBX: dffffc0000000000 RCX: 0000000000000000
[ 1070.299428] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 1070.300296] RBP: 0000000000000001 R08: 0000000000000000 R09: ffffffff873eda27
[ 1070.301204] R10: ffffc900083679a0 R11: 0000000000000001 R12: ffff88812624c000
[ 1070.301932] R13: dffffc0000000000 R14: ffff88810e6e9a88 R15: ffff888119bb9000
[ 1070.302645] FS: 0000000000000000(0000) GS:ffff888151200000(0000) knlGS:0000000000000000
[ 1070.303462] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1070.304131] CR2: 0000561a4d922418 CR3: 000000010aecc000 CR4: 0000000000350ee0
[ 1070.305004] Kernel panic - not syncing: Fatal exception
[ 1070.305711] Kernel Offset: disabled
[ 1070.305971] ---[ end Kernel panic - not syncing: Fatal exception ]---
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
fs/cifs/connect.c | 2 ++
fs/cifs/dfs_cache.c | 19 ++++++++++++-------
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 902e8c6c0f9c..2c24d433061a 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3675,9 +3675,11 @@ static void setup_server_referral_paths(struct mount_ctx *mnt_ctx)
{
struct TCP_Server_Info *server = mnt_ctx->server;
+ mutex_lock(&server->refpath_lock);
server->origin_fullpath = mnt_ctx->origin_fullpath;
server->leaf_fullpath = mnt_ctx->leaf_fullpath;
server->current_fullpath = mnt_ctx->leaf_fullpath;
+ mutex_unlock(&server->refpath_lock);
mnt_ctx->origin_fullpath = mnt_ctx->leaf_fullpath = NULL;
}
diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c
index 30e040da4f09..956f8e5cf3e7 100644
--- a/fs/cifs/dfs_cache.c
+++ b/fs/cifs/dfs_cache.c
@@ -1422,12 +1422,14 @@ static int refresh_tcon(struct cifs_ses **sessions, struct cifs_tcon *tcon, bool
struct TCP_Server_Info *server = tcon->ses->server;
mutex_lock(&server->refpath_lock);
- if (strcasecmp(server->leaf_fullpath, server->origin_fullpath))
- __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, force_refresh);
+ if (server->origin_fullpath) {
+ if (server->leaf_fullpath && strcasecmp(server->leaf_fullpath,
+ server->origin_fullpath))
+ __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, force_refresh);
+ __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, force_refresh);
+ }
mutex_unlock(&server->refpath_lock);
- __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, force_refresh);
-
return 0;
}
@@ -1530,11 +1532,14 @@ static void refresh_mounts(struct cifs_ses **sessions)
list_del_init(&tcon->ulist);
mutex_lock(&server->refpath_lock);
- if (strcasecmp(server->leaf_fullpath, server->origin_fullpath))
- __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, false);
+ if (server->origin_fullpath) {
+ if (server->leaf_fullpath && strcasecmp(server->leaf_fullpath,
+ server->origin_fullpath))
+ __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, false);
+ __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, false);
+ }
mutex_unlock(&server->refpath_lock);
- __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, false);
cifs_put_tcon(tcon);
}
}
--
2.35.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] cifs: use correct lock type in cifs_reconnect()
2022-04-21 0:05 [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts() Paulo Alcantara
@ 2022-04-21 0:05 ` Paulo Alcantara
2022-04-21 2:39 ` Leif Sahlberg
2022-04-21 2:38 ` [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts() Leif Sahlberg
1 sibling, 1 reply; 6+ messages in thread
From: Paulo Alcantara @ 2022-04-21 0:05 UTC (permalink / raw)
To: linux-cifs, smfrench; +Cc: Paulo Alcantara
TCP_Server_Info::origin_fullpath and TCP_Server_Info::leaf_fullpath
are protected by refpath_lock mutex and not cifs_tcp_ses_lock
spinlock.
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
fs/cifs/connect.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 2c24d433061a..42e14f408856 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -534,12 +534,19 @@ int cifs_reconnect(struct TCP_Server_Info *server, bool mark_smb_session)
{
/* If tcp session is not an dfs connection, then reconnect to last target server */
spin_lock(&cifs_tcp_ses_lock);
- if (!server->is_dfs_conn || !server->origin_fullpath || !server->leaf_fullpath) {
+ if (!server->is_dfs_conn) {
spin_unlock(&cifs_tcp_ses_lock);
return __cifs_reconnect(server, mark_smb_session);
}
spin_unlock(&cifs_tcp_ses_lock);
+ mutex_lock(&server->refpath_lock);
+ if (!server->origin_fullpath || !server->leaf_fullpath) {
+ mutex_unlock(&server->refpath_lock);
+ return __cifs_reconnect(server, mark_smb_session);
+ }
+ mutex_unlock(&server->refpath_lock);
+
return reconnect_dfs_server(server);
}
#else
--
2.35.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts()
2022-04-21 0:05 [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts() Paulo Alcantara
2022-04-21 0:05 ` [PATCH 2/2] cifs: use correct lock type in cifs_reconnect() Paulo Alcantara
@ 2022-04-21 2:38 ` Leif Sahlberg
2022-04-21 3:55 ` Steve French
1 sibling, 1 reply; 6+ messages in thread
From: Leif Sahlberg @ 2022-04-21 2:38 UTC (permalink / raw)
To: Paulo Alcantara; +Cc: CIFS, French, Steve
Looks good. Reviewed-by me
On Thu, Apr 21, 2022 at 10:06 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> Either mount(2) or automount might not have server->origin_fullpath
> set yet while refresh_cache_worker() is attempting to refresh DFS
> referrals. Add missing NULL check and locking around it.
>
> This fixes bellow crash:
>
> [ 1070.276835] general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI
> [ 1070.277676] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> [ 1070.278219] CPU: 1 PID: 8506 Comm: kworker/u8:1 Not tainted 5.18.0-rc3 #10
> [ 1070.278701] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014
> [ 1070.279495] Workqueue: cifs-dfscache refresh_cache_worker [cifs]
> [ 1070.280044] RIP: 0010:strcasecmp+0x34/0x150
> [ 1070.280359] Code: 00 00 00 fc ff df 41 54 55 48 89 fd 53 48 83 ec 10 eb 03 4c 89 fe 48 89 ef 48 83 c5 01 48 89 f8 48 89 fa 48 c1 e8 03 83 e2 07 <42> 0f b6 04 28 38 d0 7f 08 84 c0 0f 85 bc 00 00 00 0f b6 45 ff 44
> [ 1070.281729] RSP: 0018:ffffc90008367958 EFLAGS: 00010246
> [ 1070.282114] RAX: 0000000000000000 RBX: dffffc0000000000 RCX: 0000000000000000
> [ 1070.282691] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> [ 1070.283273] RBP: 0000000000000001 R08: 0000000000000000 R09: ffffffff873eda27
> [ 1070.283857] R10: ffffc900083679a0 R11: 0000000000000001 R12: ffff88812624c000
> [ 1070.284436] R13: dffffc0000000000 R14: ffff88810e6e9a88 R15: ffff888119bb9000
> [ 1070.284990] FS: 0000000000000000(0000) GS:ffff888151200000(0000) knlGS:0000000000000000
> [ 1070.285625] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1070.286100] CR2: 0000561a4d922418 CR3: 000000010aecc000 CR4: 0000000000350ee0
> [ 1070.286683] Call Trace:
> [ 1070.286890] <TASK>
> [ 1070.287070] refresh_cache_worker+0x895/0xd20 [cifs]
> [ 1070.287475] ? __refresh_tcon.isra.0+0xfb0/0xfb0 [cifs]
> [ 1070.287905] ? __lock_acquire+0xcd1/0x6960
> [ 1070.288247] ? is_dynamic_key+0x1a0/0x1a0
> [ 1070.288591] ? lockdep_hardirqs_on_prepare+0x410/0x410
> [ 1070.289012] ? lock_downgrade+0x6f0/0x6f0
> [ 1070.289318] process_one_work+0x7bd/0x12d0
> [ 1070.289637] ? worker_thread+0x160/0xec0
> [ 1070.289970] ? pwq_dec_nr_in_flight+0x230/0x230
> [ 1070.290318] ? _raw_spin_lock_irq+0x5e/0x90
> [ 1070.290619] worker_thread+0x5ac/0xec0
> [ 1070.290891] ? process_one_work+0x12d0/0x12d0
> [ 1070.291199] kthread+0x2a5/0x350
> [ 1070.291430] ? kthread_complete_and_exit+0x20/0x20
> [ 1070.291770] ret_from_fork+0x22/0x30
> [ 1070.292050] </TASK>
> [ 1070.292223] Modules linked in: bpfilter cifs cifs_arc4 cifs_md4
> [ 1070.292765] ---[ end trace 0000000000000000 ]---
> [ 1070.293108] RIP: 0010:strcasecmp+0x34/0x150
> [ 1070.293471] Code: 00 00 00 fc ff df 41 54 55 48 89 fd 53 48 83 ec 10 eb 03 4c 89 fe 48 89 ef 48 83 c5 01 48 89 f8 48 89 fa 48 c1 e8 03 83 e2 07 <42> 0f b6 04 28 38 d0 7f 08 84 c0 0f 85 bc 00 00 00 0f b6 45 ff 44
> [ 1070.297718] RSP: 0018:ffffc90008367958 EFLAGS: 00010246
> [ 1070.298622] RAX: 0000000000000000 RBX: dffffc0000000000 RCX: 0000000000000000
> [ 1070.299428] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> [ 1070.300296] RBP: 0000000000000001 R08: 0000000000000000 R09: ffffffff873eda27
> [ 1070.301204] R10: ffffc900083679a0 R11: 0000000000000001 R12: ffff88812624c000
> [ 1070.301932] R13: dffffc0000000000 R14: ffff88810e6e9a88 R15: ffff888119bb9000
> [ 1070.302645] FS: 0000000000000000(0000) GS:ffff888151200000(0000) knlGS:0000000000000000
> [ 1070.303462] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1070.304131] CR2: 0000561a4d922418 CR3: 000000010aecc000 CR4: 0000000000350ee0
> [ 1070.305004] Kernel panic - not syncing: Fatal exception
> [ 1070.305711] Kernel Offset: disabled
> [ 1070.305971] ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
> fs/cifs/connect.c | 2 ++
> fs/cifs/dfs_cache.c | 19 ++++++++++++-------
> 2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 902e8c6c0f9c..2c24d433061a 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3675,9 +3675,11 @@ static void setup_server_referral_paths(struct mount_ctx *mnt_ctx)
> {
> struct TCP_Server_Info *server = mnt_ctx->server;
>
> + mutex_lock(&server->refpath_lock);
> server->origin_fullpath = mnt_ctx->origin_fullpath;
> server->leaf_fullpath = mnt_ctx->leaf_fullpath;
> server->current_fullpath = mnt_ctx->leaf_fullpath;
> + mutex_unlock(&server->refpath_lock);
> mnt_ctx->origin_fullpath = mnt_ctx->leaf_fullpath = NULL;
> }
>
> diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c
> index 30e040da4f09..956f8e5cf3e7 100644
> --- a/fs/cifs/dfs_cache.c
> +++ b/fs/cifs/dfs_cache.c
> @@ -1422,12 +1422,14 @@ static int refresh_tcon(struct cifs_ses **sessions, struct cifs_tcon *tcon, bool
> struct TCP_Server_Info *server = tcon->ses->server;
>
> mutex_lock(&server->refpath_lock);
> - if (strcasecmp(server->leaf_fullpath, server->origin_fullpath))
> - __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, force_refresh);
> + if (server->origin_fullpath) {
> + if (server->leaf_fullpath && strcasecmp(server->leaf_fullpath,
> + server->origin_fullpath))
> + __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, force_refresh);
> + __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, force_refresh);
> + }
> mutex_unlock(&server->refpath_lock);
>
> - __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, force_refresh);
> -
> return 0;
> }
>
> @@ -1530,11 +1532,14 @@ static void refresh_mounts(struct cifs_ses **sessions)
> list_del_init(&tcon->ulist);
>
> mutex_lock(&server->refpath_lock);
> - if (strcasecmp(server->leaf_fullpath, server->origin_fullpath))
> - __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, false);
> + if (server->origin_fullpath) {
> + if (server->leaf_fullpath && strcasecmp(server->leaf_fullpath,
> + server->origin_fullpath))
> + __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, false);
> + __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, false);
> + }
> mutex_unlock(&server->refpath_lock);
>
> - __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, false);
> cifs_put_tcon(tcon);
> }
> }
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cifs: use correct lock type in cifs_reconnect()
2022-04-21 0:05 ` [PATCH 2/2] cifs: use correct lock type in cifs_reconnect() Paulo Alcantara
@ 2022-04-21 2:39 ` Leif Sahlberg
2022-04-21 3:56 ` Steve French
0 siblings, 1 reply; 6+ messages in thread
From: Leif Sahlberg @ 2022-04-21 2:39 UTC (permalink / raw)
To: Paulo Alcantara; +Cc: CIFS, French, Steve
looks good to me.
Reviewed-by me
On Thu, Apr 21, 2022 at 10:06 AM Paulo Alcantara <pc@cjr.nz> wrote:
>
> TCP_Server_Info::origin_fullpath and TCP_Server_Info::leaf_fullpath
> are protected by refpath_lock mutex and not cifs_tcp_ses_lock
> spinlock.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
> fs/cifs/connect.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 2c24d433061a..42e14f408856 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -534,12 +534,19 @@ int cifs_reconnect(struct TCP_Server_Info *server, bool mark_smb_session)
> {
> /* If tcp session is not an dfs connection, then reconnect to last target server */
> spin_lock(&cifs_tcp_ses_lock);
> - if (!server->is_dfs_conn || !server->origin_fullpath || !server->leaf_fullpath) {
> + if (!server->is_dfs_conn) {
> spin_unlock(&cifs_tcp_ses_lock);
> return __cifs_reconnect(server, mark_smb_session);
> }
> spin_unlock(&cifs_tcp_ses_lock);
>
> + mutex_lock(&server->refpath_lock);
> + if (!server->origin_fullpath || !server->leaf_fullpath) {
> + mutex_unlock(&server->refpath_lock);
> + return __cifs_reconnect(server, mark_smb_session);
> + }
> + mutex_unlock(&server->refpath_lock);
> +
> return reconnect_dfs_server(server);
> }
> #else
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts()
2022-04-21 2:38 ` [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts() Leif Sahlberg
@ 2022-04-21 3:55 ` Steve French
0 siblings, 0 replies; 6+ messages in thread
From: Steve French @ 2022-04-21 3:55 UTC (permalink / raw)
To: Leif Sahlberg; +Cc: Paulo Alcantara, CIFS
Added CC:Stable and Reviewed-by and pushed to cifs-2.6.git for-next
pending testing
On Wed, Apr 20, 2022 at 9:38 PM Leif Sahlberg <lsahlber@redhat.com> wrote:
>
> Looks good. Reviewed-by me
>
>
> On Thu, Apr 21, 2022 at 10:06 AM Paulo Alcantara <pc@cjr.nz> wrote:
> >
> > Either mount(2) or automount might not have server->origin_fullpath
> > set yet while refresh_cache_worker() is attempting to refresh DFS
> > referrals. Add missing NULL check and locking around it.
> >
> > This fixes bellow crash:
> >
> > [ 1070.276835] general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI
> > [ 1070.277676] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> > [ 1070.278219] CPU: 1 PID: 8506 Comm: kworker/u8:1 Not tainted 5.18.0-rc3 #10
> > [ 1070.278701] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.15.0-0-g2dd4b9b-rebuilt.opensuse.org 04/01/2014
> > [ 1070.279495] Workqueue: cifs-dfscache refresh_cache_worker [cifs]
> > [ 1070.280044] RIP: 0010:strcasecmp+0x34/0x150
> > [ 1070.280359] Code: 00 00 00 fc ff df 41 54 55 48 89 fd 53 48 83 ec 10 eb 03 4c 89 fe 48 89 ef 48 83 c5 01 48 89 f8 48 89 fa 48 c1 e8 03 83 e2 07 <42> 0f b6 04 28 38 d0 7f 08 84 c0 0f 85 bc 00 00 00 0f b6 45 ff 44
> > [ 1070.281729] RSP: 0018:ffffc90008367958 EFLAGS: 00010246
> > [ 1070.282114] RAX: 0000000000000000 RBX: dffffc0000000000 RCX: 0000000000000000
> > [ 1070.282691] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> > [ 1070.283273] RBP: 0000000000000001 R08: 0000000000000000 R09: ffffffff873eda27
> > [ 1070.283857] R10: ffffc900083679a0 R11: 0000000000000001 R12: ffff88812624c000
> > [ 1070.284436] R13: dffffc0000000000 R14: ffff88810e6e9a88 R15: ffff888119bb9000
> > [ 1070.284990] FS: 0000000000000000(0000) GS:ffff888151200000(0000) knlGS:0000000000000000
> > [ 1070.285625] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 1070.286100] CR2: 0000561a4d922418 CR3: 000000010aecc000 CR4: 0000000000350ee0
> > [ 1070.286683] Call Trace:
> > [ 1070.286890] <TASK>
> > [ 1070.287070] refresh_cache_worker+0x895/0xd20 [cifs]
> > [ 1070.287475] ? __refresh_tcon.isra.0+0xfb0/0xfb0 [cifs]
> > [ 1070.287905] ? __lock_acquire+0xcd1/0x6960
> > [ 1070.288247] ? is_dynamic_key+0x1a0/0x1a0
> > [ 1070.288591] ? lockdep_hardirqs_on_prepare+0x410/0x410
> > [ 1070.289012] ? lock_downgrade+0x6f0/0x6f0
> > [ 1070.289318] process_one_work+0x7bd/0x12d0
> > [ 1070.289637] ? worker_thread+0x160/0xec0
> > [ 1070.289970] ? pwq_dec_nr_in_flight+0x230/0x230
> > [ 1070.290318] ? _raw_spin_lock_irq+0x5e/0x90
> > [ 1070.290619] worker_thread+0x5ac/0xec0
> > [ 1070.290891] ? process_one_work+0x12d0/0x12d0
> > [ 1070.291199] kthread+0x2a5/0x350
> > [ 1070.291430] ? kthread_complete_and_exit+0x20/0x20
> > [ 1070.291770] ret_from_fork+0x22/0x30
> > [ 1070.292050] </TASK>
> > [ 1070.292223] Modules linked in: bpfilter cifs cifs_arc4 cifs_md4
> > [ 1070.292765] ---[ end trace 0000000000000000 ]---
> > [ 1070.293108] RIP: 0010:strcasecmp+0x34/0x150
> > [ 1070.293471] Code: 00 00 00 fc ff df 41 54 55 48 89 fd 53 48 83 ec 10 eb 03 4c 89 fe 48 89 ef 48 83 c5 01 48 89 f8 48 89 fa 48 c1 e8 03 83 e2 07 <42> 0f b6 04 28 38 d0 7f 08 84 c0 0f 85 bc 00 00 00 0f b6 45 ff 44
> > [ 1070.297718] RSP: 0018:ffffc90008367958 EFLAGS: 00010246
> > [ 1070.298622] RAX: 0000000000000000 RBX: dffffc0000000000 RCX: 0000000000000000
> > [ 1070.299428] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> > [ 1070.300296] RBP: 0000000000000001 R08: 0000000000000000 R09: ffffffff873eda27
> > [ 1070.301204] R10: ffffc900083679a0 R11: 0000000000000001 R12: ffff88812624c000
> > [ 1070.301932] R13: dffffc0000000000 R14: ffff88810e6e9a88 R15: ffff888119bb9000
> > [ 1070.302645] FS: 0000000000000000(0000) GS:ffff888151200000(0000) knlGS:0000000000000000
> > [ 1070.303462] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 1070.304131] CR2: 0000561a4d922418 CR3: 000000010aecc000 CR4: 0000000000350ee0
> > [ 1070.305004] Kernel panic - not syncing: Fatal exception
> > [ 1070.305711] Kernel Offset: disabled
> > [ 1070.305971] ---[ end Kernel panic - not syncing: Fatal exception ]---
> >
> > Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> > ---
> > fs/cifs/connect.c | 2 ++
> > fs/cifs/dfs_cache.c | 19 ++++++++++++-------
> > 2 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 902e8c6c0f9c..2c24d433061a 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -3675,9 +3675,11 @@ static void setup_server_referral_paths(struct mount_ctx *mnt_ctx)
> > {
> > struct TCP_Server_Info *server = mnt_ctx->server;
> >
> > + mutex_lock(&server->refpath_lock);
> > server->origin_fullpath = mnt_ctx->origin_fullpath;
> > server->leaf_fullpath = mnt_ctx->leaf_fullpath;
> > server->current_fullpath = mnt_ctx->leaf_fullpath;
> > + mutex_unlock(&server->refpath_lock);
> > mnt_ctx->origin_fullpath = mnt_ctx->leaf_fullpath = NULL;
> > }
> >
> > diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c
> > index 30e040da4f09..956f8e5cf3e7 100644
> > --- a/fs/cifs/dfs_cache.c
> > +++ b/fs/cifs/dfs_cache.c
> > @@ -1422,12 +1422,14 @@ static int refresh_tcon(struct cifs_ses **sessions, struct cifs_tcon *tcon, bool
> > struct TCP_Server_Info *server = tcon->ses->server;
> >
> > mutex_lock(&server->refpath_lock);
> > - if (strcasecmp(server->leaf_fullpath, server->origin_fullpath))
> > - __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, force_refresh);
> > + if (server->origin_fullpath) {
> > + if (server->leaf_fullpath && strcasecmp(server->leaf_fullpath,
> > + server->origin_fullpath))
> > + __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, force_refresh);
> > + __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, force_refresh);
> > + }
> > mutex_unlock(&server->refpath_lock);
> >
> > - __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, force_refresh);
> > -
> > return 0;
> > }
> >
> > @@ -1530,11 +1532,14 @@ static void refresh_mounts(struct cifs_ses **sessions)
> > list_del_init(&tcon->ulist);
> >
> > mutex_lock(&server->refpath_lock);
> > - if (strcasecmp(server->leaf_fullpath, server->origin_fullpath))
> > - __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, false);
> > + if (server->origin_fullpath) {
> > + if (server->leaf_fullpath && strcasecmp(server->leaf_fullpath,
> > + server->origin_fullpath))
> > + __refresh_tcon(server->leaf_fullpath + 1, sessions, tcon, false);
> > + __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, false);
> > + }
> > mutex_unlock(&server->refpath_lock);
> >
> > - __refresh_tcon(server->origin_fullpath + 1, sessions, tcon, false);
> > cifs_put_tcon(tcon);
> > }
> > }
> > --
> > 2.35.3
> >
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cifs: use correct lock type in cifs_reconnect()
2022-04-21 2:39 ` Leif Sahlberg
@ 2022-04-21 3:56 ` Steve French
0 siblings, 0 replies; 6+ messages in thread
From: Steve French @ 2022-04-21 3:56 UTC (permalink / raw)
To: Leif Sahlberg; +Cc: Paulo Alcantara, CIFS
Added CC:Stable and Reviewed-by and pushed to cifs-2.6.git for-next
pending testing
On Wed, Apr 20, 2022 at 9:39 PM Leif Sahlberg <lsahlber@redhat.com> wrote:
>
> looks good to me.
> Reviewed-by me
>
> On Thu, Apr 21, 2022 at 10:06 AM Paulo Alcantara <pc@cjr.nz> wrote:
> >
> > TCP_Server_Info::origin_fullpath and TCP_Server_Info::leaf_fullpath
> > are protected by refpath_lock mutex and not cifs_tcp_ses_lock
> > spinlock.
> >
> > Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> > ---
> > fs/cifs/connect.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 2c24d433061a..42e14f408856 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -534,12 +534,19 @@ int cifs_reconnect(struct TCP_Server_Info *server, bool mark_smb_session)
> > {
> > /* If tcp session is not an dfs connection, then reconnect to last target server */
> > spin_lock(&cifs_tcp_ses_lock);
> > - if (!server->is_dfs_conn || !server->origin_fullpath || !server->leaf_fullpath) {
> > + if (!server->is_dfs_conn) {
> > spin_unlock(&cifs_tcp_ses_lock);
> > return __cifs_reconnect(server, mark_smb_session);
> > }
> > spin_unlock(&cifs_tcp_ses_lock);
> >
> > + mutex_lock(&server->refpath_lock);
> > + if (!server->origin_fullpath || !server->leaf_fullpath) {
> > + mutex_unlock(&server->refpath_lock);
> > + return __cifs_reconnect(server, mark_smb_session);
> > + }
> > + mutex_unlock(&server->refpath_lock);
> > +
> > return reconnect_dfs_server(server);
> > }
> > #else
> > --
> > 2.35.3
> >
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-21 3:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-21 0:05 [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts() Paulo Alcantara
2022-04-21 0:05 ` [PATCH 2/2] cifs: use correct lock type in cifs_reconnect() Paulo Alcantara
2022-04-21 2:39 ` Leif Sahlberg
2022-04-21 3:56 ` Steve French
2022-04-21 2:38 ` [PATCH 1/2] cifs: fix NULL ptr dereference in refresh_mounts() Leif Sahlberg
2022-04-21 3:55 ` Steve French
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.