Microsoft leases test run by Steve French and a new smbtorture leases test I wrote clearly show SMB2 servers only check the lease key when deciding to send lease break requests to a client, not both the lease-key and client guid. More info to follow including wireshark traces, a torture test and a proposed fix for master and 4.2.0.
Created attachment 10718 [details] Steve's wireshark trace showing the problem using Microsoft tests.
Created attachment 10719 [details] Raw patch of torture test showing the problem.
Created attachment 10720 [details] Raw patch for master.
Comment on attachment 10720 [details] Raw patch for master. With this patch we still pass all the smb2.leases tests.
Comment on attachment 10718 [details] Steve's wireshark trace showing the problem using Microsoft tests. This trace shows a Windows client not responding to a lease break we send in frame 150 that it doesn't expect. We wait the lease timeout and then reply with SHARING_VIOLATION in frame 270. Against a Windows server this test immediately responds with SHARING_VIOLATION after the second open with an incompatible share mode.
Comment on attachment 10718 [details] Steve's wireshark trace showing the problem using Microsoft tests. Note that in both opens the same lease key is used, but they are coming from *different* client guids. The Windows server doesn't break in this case, as can be seen by running the torture test patch in this bug against a Windows 2K12 server.
Marking as blocker. The only extra thing I need to know is if this only occurs on SHARING_VIOLATION errors or if it's generic across opens on same lease key/different client guid that would also cause break messages. I think I can extend test7 to investigate this, will do so tomorrow.
JRA's patch didn't fix the test case in my setup because it doesn't reach the point of comparing the lease keys (the code he changed) - because the lease being sent in from the SMB2 create code is null, because the caller is setting oplocklevel to 0. Still debugging ....
Jeremy, Windows 2008R2 seems to behave like samba. Windows 2012[R2] doesn't break handle leases on sharing violation. Isn't that the behavior change we already found and discussed with dochelp?
No I don't think so. That was not breaking leases on byte range locks, this is more generic. Also, the problem with this one is that when we do send the break the client (at least the MS-test client) isn't expecting it and times out the break request. I'm going to flesh out test7 a little more today to understand whether we need this behavior (ignoring client guid) only on SHARING_VIOLATION errors, or more generically. I'll be testing against W2K12.
I don't expect Windows 2012R2 to behave any differently in this test case than Windows 8.1. The good news is that JRA's latest patch modification (followon to patch he posted last night) does get past the premature lease break (ie we shouldn't send a lease break when we are going to return a sharing violation anyway on the 2nd opener). But it still fails (at least test 446) now later on. Near the end of the test - Looks like the SetInfo FILE_RENAME_INFO does not trigger a lease break and the test case expects it too.
(In reply to Steve French from comment #11) > I don't expect Windows 2012R2 to behave any differently in this test case than > Windows 8.1. So I just tested Win2k8r2 with breaking7, and it *does* issue a lease break for this case (open with same lease key, but different client guid). So we need to know if the Win2k12 behavior is a regression. We have to ask dochelp I think. This also means (as I'm assuming Microsoft tested Win8+ clients against Win2k8r2) that the behavior required by the Microsoft test cases is merely an identity against W2k12, and is only testing the current state of the implementation, bugs notwithstanding (as we've already discovered the Microsoft tests are sending lease requests without setting requested_oplock_level = SMB2_OPLOCK_LEVEL_LEASE, but setting it to SMB2_OPLOCK_LEVEL_NONE - so a Win2k12 server isn't checking that field).
Ok, reducing the severity of this bug, and removing it as a blocker for 4.2.0 release. We're behaving exactly as W2K8R2 does here, in that we look at both the client guid+lease key when determining if we need to issue an oplock break on sharing violation. W2K12R2 seems to only look at lease key, not client guid. However, this only seems to be triggered by the Microsoft tests that Stevef is running, which also have their own problems (sending lease blobs whist not setting oplock level lease in open requests for example), so I'm inclined to think this bug is around getting us to pass the MS tests, rather than a real logic error in our lease implementation. I'll raise this on dochelp.
Created attachment 10725 [details] Raw patch - Extended torture test smb2.lease.breaking7
Created attachment 10726 [details] wireshark trace of breaking7 against Win2k8r2
Created attachment 10727 [details] wireshark trace of breaking7 against Win2k12r2
Data sent to dochelp.
Hi all, Whist Steve French was running the Microsoft leases tests against Samba, we discovered a change in how Windows handles lease breaks. In Windows 2008r2, when a client opens 2 connections with different client guids, and then opens the same file on each connection with the same lease key with incompatible share modes - the server issues a lease break to the client before returning with a sharing violation error. In Windows 2012r2 in the same situation, the server *does not* issue a lease break to the client before returning with a sharing violation error - even though the opens are issues on different connections with different client guids. Both 2008r2 and 2012r2 issue lease breaks when an open is issued with an incompatible share mode but the lease key is different, as expected (this is also tested in the wireshark trace). This seems to me to be a regression in server behavior, in that leases should be a combination of both client guid+lease key, and the server should always be looking at both when deciding what actions to take on a lease. Currently Samba behaves as Windows 2008r2 in this matter, but the Microsft SMB2 tests seem to be expecting the Windows 2012r2 behavior, which I think is a problem for implementors. Attached are 2 wireshark traces showing the problem. As SMB traffic isn't allowed on the LAN where I am these were taken locally between VM's on ports 4444 and 4442, so use wireshark's 'Decode As' -> NBSS to see as SMB2 traffic. Cheers, Jeremy.