cifs: Fix preauth hash corruption

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(-)

Good catch, thanks Vincent.
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

Cheers,

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
>

Steve French <smfrench@gmail.com> writes:
> cc: stable?

Sounds good

Cheers,

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)
>