2017-07-10 7:04 GMT-07:00 Germano Percossi <germano.percossi@xxxxxxxxxx>:
> Hi Pavel,
>
> I have one comment/question.
>
> On 07/08/2017 10:32 PM, Pavel Shilovsky wrote:
>> According to the MS-SMB2 spec (3.2.5.1.6) once the client receives
>> STATUS_NETWORK_SESSION_EXPIRED error code from a server it should
>> reconnect the current SMB session. Currently the client doesn't do
>> that. This can result in subsequent client requests failing by
>> the server. The patch adds an additional logic to the demultiplex
>> thread to identify expired sessions and reconnect them.
>>
>> Cc: <stable@xxxxxxxxxxxxxxx>
>> Signed-off-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
>> ---
>> fs/cifs/cifsglob.h | 2 ++
>> fs/cifs/cifssmb.c | 7 +++++++
>> fs/cifs/connect.c | 7 +++++++
>> fs/cifs/smb2ops.c | 23 +++++++++++++++++++++++
>> 4 files changed, 39 insertions(+)
>>
> [cut]
>> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
>> index fbb0d4c..72a53bd 100644
>> --- a/fs/cifs/cifssmb.c
>> +++ b/fs/cifs/cifssmb.c
>> @@ -1460,6 +1460,13 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
>> return length;
>> server->total_read += length;
>>
>> + if (server->ops->is_session_expired &&
>> + server->ops->is_session_expired(buf)) {
>> + cifs_reconnect(server);
>> + wake_up(&server->response_q);
>> + return -1;
>> + }
>> +
>
> Here (but the same applied to the other similar code blocks), as soon as
> the session is found expired, there is an attempt to reconnect and -1 is
> returned, regardless of the reconnect outcome.
>
> However, according to the specs, if the re-authentication attempt fails
> the client must fail the operation and terminate the session.
> Is the latter done implicitly, because I cannot see it?
>
> Cheers,
> Germano
>
Hi Germano,
cifs_reconnect does shutdown and then connect again the socket. Shutting down the socket implicitly terminates the existing SMB session which will be scheduled for re-negotiation. The negotiation is done in a separate thread: see cifs_reconnect() calling mod_delayed_work() that schedules server->echo work to be executed immediately; the latter then starts negotiating SMB session.
cifs_reconnect() itself should be a void function because it always returns 0 (patches are welcome to clean it up). Demultiplex thread treats -1 error code as an indicator that the request should not be processed further and we need to continue polling the socket for data.
--
Best regards,
Pavel Shilovsky
��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
