cc: stable?
On Wed, Mar 10, 2021 at 6:21 AM Vincent Whitchurch
<vincent.whitchurch@xxxxxxxx> 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@xxxxxxxx>
> ---
> 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
