| Message ID | 20221229153356.8221-2-pc@cjr.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] cifs: ignore ipc reconnect failures during dfs failover | expand |
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 >
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
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);
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(-)