| Message ID | 20200224131510.20608-1-metze@samba.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Avoid reconnects of failed session setups on soft mounts | expand |
Stefan Metzmacher <metze@samba.org> writes: > The first patches consolidate the retry/reconnect handling between SMB1 > and SMB2/3. This code is very tricky, we had to tweak it multiple times to deal with races, deadlocks and DFS (with DFS we have to failover to a different server and have to update TCP_Server_Info fields) and we can't test/repro those easily so we need to be very careful. I will start looking but it will take some time. In any case amazing work in unifying these things, reconnect code path is horribly complex. Also, you should try to run checkpatch.pl (from the kernel repo) and sparse (kernel static analysis). You need a recent version of sparse to work on the kernel (get&build the git repo somewhere). I like to use something this when working on a series: git rebase -x 'bash -c "./scripts/checkpatch.pl <(git format-patch -1 --stdout)" && touch fs/cifs/*.[ch] && make CHECK=path_to_your_sparse_bin C=1' HEAD~13 (on one line) Cheers,
Commit b0dd940e582b6a6 ("cifs: fail i/o on soft mounts if sessionsetup errors out") didn't fix the whole problem. New i/o requests still try reconnects on the wire. But we need to avoid that in order to avoid locking out the user account. The first patches consolidate the retry/reconnect handling between SMB1 and SMB2/3. Then we map STATUS_ACCOUNT_LOCKED_OUT also to -EACCES, as it's the follow up of STATUS_LOGON_FAILURE when the wrong password was used too often. Finally if we get -EACCES for a session setup on a soft mount, we'll turn the session into a CifsInvalidCredentials state and return -EKEYREVOKED for all i/o on the session, without trying to reconnect the network connection. In future we could add ways to recover such sessions. cifs_demultiplex_thread() is already prepared for that. I tested this with a trivial /proc/fs/cifs/ResetInvalidCredentials to reset all sessions, but I don't think that's something that should be used in production. For now the only way out is 'umount' (as before). metze