Bug 14737 - Samba does not response STATUS_INVALID_PARAMETER when opening 2 objects with same lease key
Summary: Samba does not response STATUS_INVALID_PARAMETER when opening 2 objects with ...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.14.5
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jule Anger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-06-11 18:39 UTC by Jeremy Allison
Modified: 2022-04-04 12:50 UTC (History)
2 users (show)

See Also:


Attachments
wireshark pcap (4.15 KB, application/vnd.tcpdump.pcap)
2021-06-11 18:40 UTC, Jeremy Allison
no flags Details
git-am fix for 4.16.rcNext, 4.15.next. (9.82 KB, patch)
2022-02-18 20:41 UTC, Jeremy Allison
jra: review? (dmulder)
npower: review+
Details
git-am fix for 4.14.next. (9.80 KB, patch)
2022-02-18 20:42 UTC, Jeremy Allison
jra: review? (dmulder)
npower: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2021-06-11 18:39:46 UTC
From: haihua yang via samba <samba@lists.samba.org>
To: samba@lists.samba.org
Subject: [Samba] 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                 }
Comment 1 Jeremy Allison 2021-06-11 18:40:21 UTC
Created attachment 16653 [details]
wireshark pcap
Comment 2 Jeremy Allison 2022-02-16 21:57:50 UTC
OK, I took a close look at this and this code is actually due to the fact that Samba on UNIX can cope with "dynamic" shares. The most common example of this is the [homes] share, where the actual share path is created from the users home directory.

I'l look again more closely to see if this logic needs to be changed to correctly return STATUS_INVALID_PARAMETER.
Comment 3 Jeremy Allison 2022-02-17 19:16:05 UTC
Fix (with regression tests) now under ci here:

https://gitlab.com/samba-team/devel/samba/-/pipelines/473665121
Comment 4 Jeremy Allison 2022-02-17 20:52:41 UTC
Passes ci. MR is here:

https://gitlab.com/samba-team/samba/-/merge_requests/2391
Comment 5 Samba QA Contact 2022-02-18 20:13:03 UTC
This bug was referenced in samba master:

bf22548d11fe67ea3f4ec10dff81773d626e4703
ca3896b6f8bbcad68f042720feceedfa29ddbd83
408be54323861c24b6377b804be4428cf45b471e
Comment 6 Jeremy Allison 2022-02-18 20:41:03 UTC
Created attachment 17169 [details]
git-am fix for 4.16.rcNext, 4.15.next.

Cherry-picked from master. Applies cleanly to 4.16.rcNext, 4.15.next.
Comment 7 Jeremy Allison 2022-02-18 20:42:08 UTC
Created attachment 17170 [details]
git-am fix for 4.14.next.

Back-port from master.
Comment 8 Noel Power 2022-03-07 10:45:44 UTC
Comment on attachment 17169 [details]
git-am fix for 4.16.rcNext, 4.15.next.

lgtm
Comment 9 Noel Power 2022-03-07 10:47:07 UTC
assign to Jule for inclusion in 4.16, 4.15, 4.14
Comment 10 Jule Anger 2022-03-07 10:54:59 UTC
Pushed to autobuild-v4-{16,15,14}-test.
Comment 11 Samba QA Contact 2022-03-07 11:55:10 UTC
This bug was referenced in samba v4-16-test:

423bbea002e6877c7d7cffde69fe66d52dcf0d96
7995e03b39e7cb972dc76be574d25eed7c8c2da7
de8fc990b21aeb6741e76bfd33772384b66682d9
Comment 12 Samba QA Contact 2022-03-07 14:15:06 UTC
This bug was referenced in samba v4-14-test:

5b67cf9fbbf14d4b46de7f1a567d8781187a5264
4d80d0e33fc7f9c5ca89a2c9db86f307d12de488
0e793fe124bee7b8b8aba023796a7578ec42c6a8
Comment 13 Samba QA Contact 2022-03-07 14:55:10 UTC
This bug was referenced in samba v4-15-test:

0fd20764df159ca0bec7316ff7323d90e99aa636
58605094f14ad5321efb2e6bf29e746e5384ca9f
7417480d1603dd7966b41d56e1f37b357b95896e
Comment 14 Jule Anger 2022-03-07 15:02:51 UTC
Closing out bug report.

Thanks!
Comment 15 Samba QA Contact 2022-03-08 14:57:24 UTC
This bug was referenced in samba v4-16-stable (Release samba-4.16.0rc5):

423bbea002e6877c7d7cffde69fe66d52dcf0d96
7995e03b39e7cb972dc76be574d25eed7c8c2da7
de8fc990b21aeb6741e76bfd33772384b66682d9
Comment 16 Samba QA Contact 2022-03-15 13:22:04 UTC
This bug was referenced in samba v4-15-stable (Release samba-4.15.6):

0fd20764df159ca0bec7316ff7323d90e99aa636
58605094f14ad5321efb2e6bf29e746e5384ca9f
7417480d1603dd7966b41d56e1f37b357b95896e
Comment 17 Samba QA Contact 2022-04-04 12:50:06 UTC
This bug was referenced in samba v4-14-stable (Release samba-4.14.13):

5b67cf9fbbf14d4b46de7f1a567d8781187a5264
4d80d0e33fc7f9c5ca89a2c9db86f307d12de488
0e793fe124bee7b8b8aba023796a7578ec42c6a8