* [PATCH] cifs: Fix preauth hash corruption
@ 2021-03-10 12:20 Vincent Whitchurch
2021-03-10 15:15 ` Aurélien Aptel
2021-03-10 18:51 ` Steve French
0 siblings, 2 replies; 5+ messages in thread
From: Vincent Whitchurch @ 2021-03-10 12:20 UTC (permalink / raw)
To: sfrench; +Cc: linux-cifs, aaptel, kernel, Vincent Whitchurch
smb311_update_preauth_hash() uses the shash in server->secmech without
appropriate locking, and this can lead to sessions corrupting each
other's preauth hashes.
The following script can easily trigger the problem:
#!/bin/sh -e
NMOUNTS=10
for i in $(seq $NMOUNTS);
mkdir -p /tmp/mnt$i
umount /tmp/mnt$i 2>/dev/null || :
done
while :; do
for i in $(seq $NMOUNTS); do
mount -t cifs //192.168.0.1/test /tmp/mnt$i -o ... &
done
wait
for i in $(seq $NMOUNTS); do
umount /tmp/mnt$i
done
done
Usually within seconds this leads to one or more of the mounts failing
with the following errors, and a "Bad SMB2 signature for message" is
seen in the server logs:
CIFS: VFS: \\192.168.0.1 failed to connect to IPC (rc=-13)
CIFS: VFS: cifs_mount failed w/return code = -13
Fix it by holding the server mutex just like in the other places where
the shashes are used.
Fixes: 8bd68c6e47abff34e4 ("CIFS: implement v3.11 preauth integrity")
Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
---
fs/cifs/transport.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index e90a1d1380b0..aa9c0f6bc263 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -1196,9 +1196,12 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
/*
* Compounding is never used during session establish.
*/
- if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP))
+ if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) {
+ mutex_lock(&server->srv_mutex);
smb311_update_preauth_hash(ses, rqst[0].rq_iov,
rqst[0].rq_nvec);
+ mutex_unlock(&server->srv_mutex);
+ }
for (i = 0; i < num_rqst; i++) {
rc = wait_for_response(server, midQ[i]);
@@ -1266,7 +1269,9 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
.iov_base = resp_iov[0].iov_base,
.iov_len = resp_iov[0].iov_len
};
+ mutex_lock(&server->srv_mutex);
smb311_update_preauth_hash(ses, &iov, 1);
+ mutex_unlock(&server->srv_mutex);
}
out:
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: Fix preauth hash corruption
2021-03-10 12:20 [PATCH] cifs: Fix preauth hash corruption Vincent Whitchurch
@ 2021-03-10 15:15 ` Aurélien Aptel
2021-03-10 18:51 ` Steve French
1 sibling, 0 replies; 5+ messages in thread
From: Aurélien Aptel @ 2021-03-10 15:15 UTC (permalink / raw)
To: Vincent Whitchurch, sfrench; +Cc: linux-cifs, kernel, Vincent Whitchurch
Good catch, thanks Vincent.
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
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] 5+ messages in thread
* Re: [PATCH] cifs: Fix preauth hash corruption
2021-03-10 12:20 [PATCH] cifs: Fix preauth hash corruption Vincent Whitchurch
2021-03-10 15:15 ` Aurélien Aptel
@ 2021-03-10 18:51 ` Steve French
2021-03-11 10:00 ` Aurélien Aptel
1 sibling, 1 reply; 5+ messages in thread
From: Steve French @ 2021-03-10 18:51 UTC (permalink / raw)
To: Vincent Whitchurch; +Cc: Steve French, CIFS, Aurélien Aptel, kernel
cc: stable?
On Wed, Mar 10, 2021 at 6:21 AM Vincent Whitchurch
<vincent.whitchurch@axis.com> wrote:
>
> smb311_update_preauth_hash() uses the shash in server->secmech without
> appropriate locking, and this can lead to sessions corrupting each
> other's preauth hashes.
>
> The following script can easily trigger the problem:
>
> #!/bin/sh -e
>
> NMOUNTS=10
> for i in $(seq $NMOUNTS);
> mkdir -p /tmp/mnt$i
> umount /tmp/mnt$i 2>/dev/null || :
> done
> while :; do
> for i in $(seq $NMOUNTS); do
> mount -t cifs //192.168.0.1/test /tmp/mnt$i -o ... &
> done
> wait
> for i in $(seq $NMOUNTS); do
> umount /tmp/mnt$i
> done
> done
>
> Usually within seconds this leads to one or more of the mounts failing
> with the following errors, and a "Bad SMB2 signature for message" is
> seen in the server logs:
>
> CIFS: VFS: \\192.168.0.1 failed to connect to IPC (rc=-13)
> CIFS: VFS: cifs_mount failed w/return code = -13
>
> Fix it by holding the server mutex just like in the other places where
> the shashes are used.
>
> Fixes: 8bd68c6e47abff34e4 ("CIFS: implement v3.11 preauth integrity")
> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com>
> ---
> fs/cifs/transport.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index e90a1d1380b0..aa9c0f6bc263 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -1196,9 +1196,12 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> /*
> * Compounding is never used during session establish.
> */
> - if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP))
> + if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) {
> + mutex_lock(&server->srv_mutex);
> smb311_update_preauth_hash(ses, rqst[0].rq_iov,
> rqst[0].rq_nvec);
> + mutex_unlock(&server->srv_mutex);
> + }
>
> for (i = 0; i < num_rqst; i++) {
> rc = wait_for_response(server, midQ[i]);
> @@ -1266,7 +1269,9 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
> .iov_base = resp_iov[0].iov_base,
> .iov_len = resp_iov[0].iov_len
> };
> + mutex_lock(&server->srv_mutex);
> smb311_update_preauth_hash(ses, &iov, 1);
> + mutex_unlock(&server->srv_mutex);
> }
>
> out:
> --
> 2.28.0
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: Fix preauth hash corruption
2021-03-10 18:51 ` Steve French
@ 2021-03-11 10:00 ` Aurélien Aptel
2021-03-12 18:00 ` Pavel Shilovsky
0 siblings, 1 reply; 5+ messages in thread
From: Aurélien Aptel @ 2021-03-11 10:00 UTC (permalink / raw)
To: Steve French, Vincent Whitchurch; +Cc: Steve French, CIFS, kernel
Steve French <smfrench@gmail.com> writes:
> cc: stable?
Sounds good
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] 5+ messages in thread
* Re: [PATCH] cifs: Fix preauth hash corruption
2021-03-11 10:00 ` Aurélien Aptel
@ 2021-03-12 18:00 ` Pavel Shilovsky
0 siblings, 0 replies; 5+ messages in thread
From: Pavel Shilovsky @ 2021-03-12 18:00 UTC (permalink / raw)
To: Aurélien Aptel
Cc: Steve French, Vincent Whitchurch, Steve French, CIFS, kernel
Thanks for the patch! Agree with a stable tag.
--
Best regards,
Pavel Shilovsky
чт, 11 мар. 2021 г. в 02:01, Aurélien Aptel <aaptel@suse.com>:
>
> Steve French <smfrench@gmail.com> writes:
> > cc: stable?
>
> Sounds good
>
> 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] 5+ messages in thread
end of thread, other threads:[~2021-03-12 18:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 12:20 [PATCH] cifs: Fix preauth hash corruption Vincent Whitchurch
2021-03-10 15:15 ` Aurélien Aptel
2021-03-10 18:51 ` Steve French
2021-03-11 10:00 ` Aurélien Aptel
2021-03-12 18:00 ` Pavel Shilovsky
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.