* [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover
@ 2022-12-29 15:33 Paulo Alcantara
2022-12-29 15:33 ` [PATCH 2/2] cifs: fix race in assemble_neg_contexts() Paulo Alcantara
2022-12-29 20:10 ` [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover Enzo Matsumiya
0 siblings, 2 replies; 6+ messages in thread
From: Paulo Alcantara @ 2022-12-29 15:33 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs, Paulo Alcantara
If it failed to reconnect ipc used for getting referrals, we can just
ignore it as it is not required for reconnecting the share. The worst
case would be not being able to detect or chase nested links as long
as dfs root server is unreachable.
Before patch:
$ mount.cifs //root/dfs/link /mnt -o echo_interval=10,...
-> target share: /fs0/share
disconnect root & fs0
$ ls /mnt
ls: cannot access '/mnt': Host is down
connect fs0
$ ls /mnt
ls: cannot access '/mnt': Resource temporarily unavailable
After patch:
$ mount.cifs //root/dfs/link /mnt -o echo_interval=10,...
-> target share: /fs0/share
disconnect root & fs0
$ ls /mnt
ls: cannot access '/mnt': Host is down
connect fs0
$ ls /mnt
bar.rtf dir1 foo
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
fs/cifs/dfs.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/cifs/dfs.c b/fs/cifs/dfs.c
index b541e68378f6..30086f2060a1 100644
--- a/fs/cifs/dfs.c
+++ b/fs/cifs/dfs.c
@@ -401,8 +401,7 @@ static int __tree_connect_dfs_target(const unsigned int xid, struct cifs_tcon *t
if (ipc->need_reconnect) {
scnprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$", server->hostname);
rc = ops->tree_connect(xid, ipc->ses, tree, ipc, cifs_sb->local_nls);
- if (rc)
- break;
+ cifs_dbg(FYI, "%s: reconnect ipc: %d\n", __func__, rc);
}
scnprintf(tree, MAX_TREE_SIZE, "\\%s", share);
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] cifs: fix race in assemble_neg_contexts()
2022-12-29 15:33 [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover Paulo Alcantara
@ 2022-12-29 15:33 ` Paulo Alcantara
2022-12-29 20:14 ` Enzo Matsumiya
2022-12-29 20:10 ` [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover Enzo Matsumiya
1 sibling, 1 reply; 6+ messages in thread
From: Paulo Alcantara @ 2022-12-29 15:33 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs, Paulo Alcantara
Serialise access of TCP_Server_Info::hostname in
assemble_neg_contexts() by holding the server's mutex otherwise it
might end up accessing an already-freed hostname pointer from
cifs_reconnect() or cifs_resolve_server().
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
fs/cifs/smb2pdu.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index a5695748a89b..2c484d47c592 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -541,9 +541,10 @@ static void
assemble_neg_contexts(struct smb2_negotiate_req *req,
struct TCP_Server_Info *server, unsigned int *total_len)
{
- char *pneg_ctxt;
- char *hostname = NULL;
unsigned int ctxt_len, neg_context_count;
+ struct TCP_Server_Info *pserver;
+ char *pneg_ctxt;
+ char *hostname;
if (*total_len > 200) {
/* In case length corrupted don't want to overrun smb buffer */
@@ -574,8 +575,9 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
* secondary channels don't have the hostname field populated
* use the hostname field in the primary channel instead
*/
- hostname = CIFS_SERVER_IS_CHAN(server) ?
- server->primary_server->hostname : server->hostname;
+ pserver = CIFS_SERVER_IS_CHAN(server) ? server->primary_server : server;
+ cifs_server_lock(pserver);
+ hostname = pserver->hostname;
if (hostname && (hostname[0] != 0)) {
ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt,
hostname);
@@ -584,6 +586,7 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
neg_context_count = 3;
} else
neg_context_count = 2;
+ cifs_server_unlock(pserver);
build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
*total_len += sizeof(struct smb2_posix_neg_context);
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover
2022-12-29 15:33 [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover Paulo Alcantara
2022-12-29 15:33 ` [PATCH 2/2] cifs: fix race in assemble_neg_contexts() Paulo Alcantara
@ 2022-12-29 20:10 ` Enzo Matsumiya
[not found] ` <CAH2r5mv-2MPzd-zJSxDXh5avC4Bhp-BJG9nmr2f=1FR5m6B3Zg@mail.gmail.com>
1 sibling, 1 reply; 6+ messages in thread
From: Enzo Matsumiya @ 2022-12-29 20:10 UTC (permalink / raw)
To: Paulo Alcantara; +Cc: smfrench, linux-cifs
On 12/29, Paulo Alcantara wrote:
>If it failed to reconnect ipc used for getting referrals, we can just
>ignore it as it is not required for reconnecting the share. The worst
>case would be not being able to detect or chase nested links as long
>as dfs root server is unreachable.
>
>Before patch:
>
> $ mount.cifs //root/dfs/link /mnt -o echo_interval=10,...
> -> target share: /fs0/share
>
> disconnect root & fs0
>
> $ ls /mnt
> ls: cannot access '/mnt': Host is down
>
> connect fs0
>
> $ ls /mnt
> ls: cannot access '/mnt': Resource temporarily unavailable
>
>After patch:
>
> $ mount.cifs //root/dfs/link /mnt -o echo_interval=10,...
> -> target share: /fs0/share
>
> disconnect root & fs0
>
> $ ls /mnt
> ls: cannot access '/mnt': Host is down
>
> connect fs0
>
> $ ls /mnt
> bar.rtf dir1 foo
>
>Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
>---
> fs/cifs/dfs.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/fs/cifs/dfs.c b/fs/cifs/dfs.c
>index b541e68378f6..30086f2060a1 100644
>--- a/fs/cifs/dfs.c
>+++ b/fs/cifs/dfs.c
>@@ -401,8 +401,7 @@ static int __tree_connect_dfs_target(const unsigned int xid, struct cifs_tcon *t
> if (ipc->need_reconnect) {
> scnprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$", server->hostname);
> rc = ops->tree_connect(xid, ipc->ses, tree, ipc, cifs_sb->local_nls);
>- if (rc)
>- break;
>+ cifs_dbg(FYI, "%s: reconnect ipc: %d\n", __func__, rc);
> }
>
> scnprintf(tree, MAX_TREE_SIZE, "\\%s", share);
>--
>2.39.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cifs: fix race in assemble_neg_contexts()
2022-12-29 15:33 ` [PATCH 2/2] cifs: fix race in assemble_neg_contexts() Paulo Alcantara
@ 2022-12-29 20:14 ` Enzo Matsumiya
[not found] ` <CAH2r5mso6PLpKpVxtcUHW4RQ1jc-Tmj5ALOEba2z+40uSDf0JA@mail.gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Enzo Matsumiya @ 2022-12-29 20:14 UTC (permalink / raw)
To: Paulo Alcantara; +Cc: smfrench, linux-cifs
On 12/29, Paulo Alcantara wrote:
>Serialise access of TCP_Server_Info::hostname in
>assemble_neg_contexts() by holding the server's mutex otherwise it
>might end up accessing an already-freed hostname pointer from
>cifs_reconnect() or cifs_resolve_server().
>
>Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Couldn't reproduce this one as easy as the other one, but it makes sense
anyway.
Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
>---
> fs/cifs/smb2pdu.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
>diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
>index a5695748a89b..2c484d47c592 100644
>--- a/fs/cifs/smb2pdu.c
>+++ b/fs/cifs/smb2pdu.c
>@@ -541,9 +541,10 @@ static void
> assemble_neg_contexts(struct smb2_negotiate_req *req,
> struct TCP_Server_Info *server, unsigned int *total_len)
> {
>- char *pneg_ctxt;
>- char *hostname = NULL;
> unsigned int ctxt_len, neg_context_count;
>+ struct TCP_Server_Info *pserver;
>+ char *pneg_ctxt;
>+ char *hostname;
>
> if (*total_len > 200) {
> /* In case length corrupted don't want to overrun smb buffer */
>@@ -574,8 +575,9 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
> * secondary channels don't have the hostname field populated
> * use the hostname field in the primary channel instead
> */
>- hostname = CIFS_SERVER_IS_CHAN(server) ?
>- server->primary_server->hostname : server->hostname;
>+ pserver = CIFS_SERVER_IS_CHAN(server) ? server->primary_server : server;
>+ cifs_server_lock(pserver);
>+ hostname = pserver->hostname;
> if (hostname && (hostname[0] != 0)) {
> ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt,
> hostname);
>@@ -584,6 +586,7 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
> neg_context_count = 3;
> } else
> neg_context_count = 2;
>+ cifs_server_unlock(pserver);
>
> build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
> *total_len += sizeof(struct smb2_posix_neg_context);
>--
>2.39.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover
[not found] ` <CAH2r5mv-2MPzd-zJSxDXh5avC4Bhp-BJG9nmr2f=1FR5m6B3Zg@mail.gmail.com>
@ 2023-01-04 6:13 ` Steve French
0 siblings, 0 replies; 6+ messages in thread
From: Steve French @ 2023-01-04 6:13 UTC (permalink / raw)
To: Enzo Matsumiya; +Cc: Paulo Alcantara, linux-cifs
On Wed, Jan 4, 2023 at 12:13 AM Steve French <smfrench@gmail.com> wrote:
>
> merged into cifs-2.6.git for-next
>
> On Thu, Dec 29, 2022 at 2:10 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>>
>> On 12/29, Paulo Alcantara wrote:
>> >If it failed to reconnect ipc used for getting referrals, we can just
>> >ignore it as it is not required for reconnecting the share. The worst
>> >case would be not being able to detect or chase nested links as long
>> >as dfs root server is unreachable.
>> >
>> >Before patch:
>> >
>> > $ mount.cifs //root/dfs/link /mnt -o echo_interval=10,...
>> > -> target share: /fs0/share
>> >
>> > disconnect root & fs0
>> >
>> > $ ls /mnt
>> > ls: cannot access '/mnt': Host is down
>> >
>> > connect fs0
>> >
>> > $ ls /mnt
>> > ls: cannot access '/mnt': Resource temporarily unavailable
>> >
>> >After patch:
>> >
>> > $ mount.cifs //root/dfs/link /mnt -o echo_interval=10,...
>> > -> target share: /fs0/share
>> >
>> > disconnect root & fs0
>> >
>> > $ ls /mnt
>> > ls: cannot access '/mnt': Host is down
>> >
>> > connect fs0
>> >
>> > $ ls /mnt
>> > bar.rtf dir1 foo
>> >
>> >Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
>>
>> Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
>>
>> >---
>> > fs/cifs/dfs.c | 3 +--
>> > 1 file changed, 1 insertion(+), 2 deletions(-)
>> >
>> >diff --git a/fs/cifs/dfs.c b/fs/cifs/dfs.c
>> >index b541e68378f6..30086f2060a1 100644
>> >--- a/fs/cifs/dfs.c
>> >+++ b/fs/cifs/dfs.c
>> >@@ -401,8 +401,7 @@ static int __tree_connect_dfs_target(const unsigned int xid, struct cifs_tcon *t
>> > if (ipc->need_reconnect) {
>> > scnprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$", server->hostname);
>> > rc = ops->tree_connect(xid, ipc->ses, tree, ipc, cifs_sb->local_nls);
>> >- if (rc)
>> >- break;
>> >+ cifs_dbg(FYI, "%s: reconnect ipc: %d\n", __func__, rc);
>> > }
>> >
>> > scnprintf(tree, MAX_TREE_SIZE, "\\%s", share);
>> >--
>> >2.39.0
>> >
>
>
>
> --
> Thanks,
>
> Steve
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cifs: fix race in assemble_neg_contexts()
[not found] ` <CAH2r5mso6PLpKpVxtcUHW4RQ1jc-Tmj5ALOEba2z+40uSDf0JA@mail.gmail.com>
@ 2023-01-04 6:14 ` Steve French
0 siblings, 0 replies; 6+ messages in thread
From: Steve French @ 2023-01-04 6:14 UTC (permalink / raw)
To: Enzo Matsumiya; +Cc: Paulo Alcantara, linux-cifs
On Wed, Jan 4, 2023 at 12:12 AM Steve French <smfrench@gmail.com> wrote:
>
> merged into cifs-2.6.git for-next
>
> On Thu, Dec 29, 2022 at 2:14 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>>
>> On 12/29, Paulo Alcantara wrote:
>> >Serialise access of TCP_Server_Info::hostname in
>> >assemble_neg_contexts() by holding the server's mutex otherwise it
>> >might end up accessing an already-freed hostname pointer from
>> >cifs_reconnect() or cifs_resolve_server().
>> >
>> >Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
>>
>> Couldn't reproduce this one as easy as the other one, but it makes sense
>> anyway.
>>
>> Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
>>
>> >---
>> > fs/cifs/smb2pdu.c | 11 +++++++----
>> > 1 file changed, 7 insertions(+), 4 deletions(-)
>> >
>> >diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
>> >index a5695748a89b..2c484d47c592 100644
>> >--- a/fs/cifs/smb2pdu.c
>> >+++ b/fs/cifs/smb2pdu.c
>> >@@ -541,9 +541,10 @@ static void
>> > assemble_neg_contexts(struct smb2_negotiate_req *req,
>> > struct TCP_Server_Info *server, unsigned int *total_len)
>> > {
>> >- char *pneg_ctxt;
>> >- char *hostname = NULL;
>> > unsigned int ctxt_len, neg_context_count;
>> >+ struct TCP_Server_Info *pserver;
>> >+ char *pneg_ctxt;
>> >+ char *hostname;
>> >
>> > if (*total_len > 200) {
>> > /* In case length corrupted don't want to overrun smb buffer */
>> >@@ -574,8 +575,9 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
>> > * secondary channels don't have the hostname field populated
>> > * use the hostname field in the primary channel instead
>> > */
>> >- hostname = CIFS_SERVER_IS_CHAN(server) ?
>> >- server->primary_server->hostname : server->hostname;
>> >+ pserver = CIFS_SERVER_IS_CHAN(server) ? server->primary_server : server;
>> >+ cifs_server_lock(pserver);
>> >+ hostname = pserver->hostname;
>> > if (hostname && (hostname[0] != 0)) {
>> > ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt,
>> > hostname);
>> >@@ -584,6 +586,7 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
>> > neg_context_count = 3;
>> > } else
>> > neg_context_count = 2;
>> >+ cifs_server_unlock(pserver);
>> >
>> > build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
>> > *total_len += sizeof(struct smb2_posix_neg_context);
>> >--
>> >2.39.0
>> >
>
>
>
> --
> Thanks,
>
> Steve
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-04 6:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-29 15:33 [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover Paulo Alcantara
2022-12-29 15:33 ` [PATCH 2/2] cifs: fix race in assemble_neg_contexts() Paulo Alcantara
2022-12-29 20:14 ` Enzo Matsumiya
[not found] ` <CAH2r5mso6PLpKpVxtcUHW4RQ1jc-Tmj5ALOEba2z+40uSDf0JA@mail.gmail.com>
2023-01-04 6:14 ` Steve French
2022-12-29 20:10 ` [PATCH 1/2] cifs: ignore ipc reconnect failures during dfs failover Enzo Matsumiya
[not found] ` <CAH2r5mv-2MPzd-zJSxDXh5avC4Bhp-BJG9nmr2f=1FR5m6B3Zg@mail.gmail.com>
2023-01-04 6:13 ` Steve French
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).