samba - Jun 2021 - Samba does not response STATUS_INVALID_PARAMETER when opening 2 objects with same lease key

Hi,
According to MS-SMB2 3.3.5.9.8
<https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/24cfce29-5d80-4651-ba5f-a2661c666b41>,
it should respond STATUS_INVALID_PARAMETER when a client opens 2 different
objects with the same lease key, but samba successfully opens both files
and sends a lease break to the client. And since there are 2 fsp with
different file_id sharing the same lease, the lease_timeout_handler does
not find the right fsp to downgrade the lease, hence it keeps the lease in
the breaking state if the client does not close the opens..
Looks like the cause is in function lease_match_parser, it first compares
the file id and NOT_GRANTED if it mismatches,
5361         for (i = 0; i < num_files; i++) {
5362                 const struct leases_db_file *f = &files[i];
5363
5364                 /* Everything should be the same. */
5365                 if (!file_id_equal(&state->id, &f->id)) {
5366                         /* This should catch all dynamic share cases.
*/
5367                         state->match_status
NT_STATUS_OPLOCK_NOT_GRANTED;
5368                         break;
5369                 }

Thanks,

On Wed, Jun 02, 2021 at 02:46:16PM -0700, haihua yang via samba
wrote:
>Hi,
>According to MS-SMB2 3.3.5.9.8
><https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/24cfce29-5d80-4651-ba5f-a2661c666b41>,
>it should respond STATUS_INVALID_PARAMETER when a client opens 2 different
>objects with the same lease key, but samba successfully opens both files
>and sends a lease break to the client. And since there are 2 fsp with
>different file_id sharing the same lease, the lease_timeout_handler does
>not find the right fsp to downgrade the lease, hence it keeps the lease in
>the breaking state if the client does not close the opens..
>Looks like the cause is in function lease_match_parser, it first compares
>the file id and NOT_GRANTED if it mismatches,
>5361         for (i = 0; i < num_files; i++) {
>5362                 const struct leases_db_file *f = &files[i];
>5363
>5364                 /* Everything should be the same. */
>5365                 if (!file_id_equal(&state->id, &f->id)) {
>5366                         /* This should catch all dynamic share cases.
>*/
>5367                         state->match_status
>NT_STATUS_OPLOCK_NOT_GRANTED;
>5368                         break;
>5369                 }
Do you have a wireshark trace we can look at or a torture test that
can reproduce this so we can track this down and add a regression
test for it ?

Thanks !

Jeremy.