Bug 10283 - smbtorture test smb2.oplock.stream1 failed
smbtorture test smb2.oplock.stream1 failed
Status: RESOLVED DUPLICATE of bug 10797
Product: Samba 3.6
Classification: Unclassified
Component: SMB2
3.6.12
All All
: P5 normal
: ---
Assigned To: Jeremy Allison
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-21 07:43 UTC by Hemanth
Modified: 2014-09-29 23:21 UTC (History)
2 users (show)

See Also:


Attachments
packet capture taken during the test (38.75 KB, application/octet-stream)
2013-11-21 07:43 UTC, Hemanth
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hemanth 2013-11-21 07:43:11 UTC
We are using Samba 3.6.12 based build. And when we run smb torture test smb2.oplock.stream1 we have seen failure as shown below.

[root@garfield bin]# ./smbtorture //NAS-1/share1 smb2.oplock.stream1 -T samba3 -U DOMAIN\\user1%password -S required

Using seed 1384915066
time: 2013-11-19 18:37:46.501080
test: stream1
time: 2013-11-19 18:37:46.502211
0: Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 9
1: Opening stream: oplock_test\test_stream1.txt::$DATA with 9
2: Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 8
3: Opening stream: oplock_test\test_stream1.txt::$DATA with 8
Opening base file: oplock_test\test_stream1.txt with 9
4: Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 9
Opening base file: oplock_test\test_stream1.txt with 9
5: Opening stream: oplock_test\test_stream1.txt::$DATA with 9
Acking to level II [0x01] in oplock handler
Opening base file: oplock_test\test_stream1.txt with 9
6: Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 8
Opening base file: oplock_test\test_stream1.txt with 9
7: Opening stream: oplock_test\test_stream1.txt::$DATA with 8
Acking to level II [0x01] in oplock handler
Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 8
Opening base file: oplock_test\test_stream1.txt with 9
Opening stream again: oplock_test\test_stream1.txt with 9
Acking to level II [0x01] in oplock handler
time: 2013-11-19 18:37:46.816609
failure: stream1 [
(../source4/torture/smb2/oplock.c:2864): wrong value for io.smb2.out.oplock_level got 0x0 - should be 0x9
]

From the packet capture I could see the sequence as follows before hitting the problem.

- Client1 sends create request with oplock set to Exclusive on stream file.
- Create response is successful with exclusive oplock.
- Client2 sends create request with oplock set to Batch oplock on base file.
- Create response succeeds but get oplock as NONE.

Here test is failing as it is expecting oplock as Batch oplock(9) but not NONE.

I think this is what it is happening. On the initail stream file open, Samba is doing an internal/stat open for base file with access mask set to zero and oplock none. 

......
                /* Open the base file. */
                status = create_file_unixpath(conn, NULL, smb_fname_base, 0,===> access mask
                                              FILE_SHARE_READ
                                              | FILE_SHARE_WRITE
                                              | FILE_SHARE_DELETE,===>> shared mode
                                              base_create_disposition,
                                              0, 0, 0, 0, 0,==> oplock NULL, NULL,
                                              &base_fsp, NULL);

......

While iterating through the locks in find_oplock_types() it has found that a none oplock existing. 

Ideally this lock should have been filtered out as internal/stat open in find_oplock_types() as suggested by following piece of code.

...
 if (lck->share_modes[i].op_type == NO_OPLOCK &&
                                is_stat_open(lck->share_modes[i].access_mask)) {
                        /* We ignore stat opens in the table - they
                           always have NO_OPLOCK and never get or
                           cause breaks. JRA. */
                        continue;
                }

...

If you check the code for is_stat_open()..

....
bool is_stat_open(uint32 access_mask)
{
        return (access_mask &&
                ((access_mask & ~(SYNCHRONIZE_ACCESS| FILE_READ_ATTRIBUTES|
                                  FILE_WRITE_ATTRIBUTES))==0) &&
                ((access_mask & (SYNCHRONIZE_ACCESS|FILE_READ_ATTRIBUTES|
                                 FILE_WRITE_ATTRIBUTES)) != 0));
}
...

Here it is expecting that the access mask should be equal to (SYNCHRONIZE_ACCESS|FILE_READ_ATTRIBUTES| FILE_WRITE_ATTRIBUTES). Since we have set the access mask to zero while doing the stat open for base file, this check is bypassed and considered as an external open.

In grant_fsp_oplock_type(), it checks that if it already found an existing none_oplock. If so it sets the oplock to NO_OPLOCK and sends back to client. Here is snippet which does that..
        /*
         * Match what was requested (fsp->oplock_type) with
         * what was found in the existing share modes.
         */

        if (got_a_none_oplock) {
                fsp->oplock_type = NO_OPLOCK;


After using the access mask as (SYNCHRONIZE_ACCESS|FILE_READ_ATTRIBUTES| FILE_WRITE_ATTRIBUTES) while doing internal opens on base files has fixed the issue.

[root@garfield bin]# ./smbtorture //NAS-1/share1 smb2.oplock.stream1 -T samba3 -U DOMAIN\\user1%password -S required

Using seed 1384916246
time: 2013-11-19 18:57:26.522783
test: stream1
time: 2013-11-19 18:57:26.523917
0: Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 9
1: Opening stream: oplock_test\test_stream1.txt::$DATA with 9
2: Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 8
3: Opening stream: oplock_test\test_stream1.txt::$DATA with 8
Opening base file: oplock_test\test_stream1.txt with 9
4: Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 9
Opening base file: oplock_test\test_stream1.txt with 9
5: Opening stream: oplock_test\test_stream1.txt::$DATA with 9
Acking to level II [0x01] in oplock handler
Opening base file: oplock_test\test_stream1.txt with 9
6: Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 8
Opening base file: oplock_test\test_stream1.txt with 9
7: Opening stream: oplock_test\test_stream1.txt::$DATA with 8
Acking to level II [0x01] in oplock handler
Opening stream: oplock_test\test_stream1.txt:Stream One:$DATA with 8
Opening base file: oplock_test\test_stream1.txt with 9
Opening stream again: oplock_test\test_stream1.txt with 9
Acking to level II [0x01] in oplock handler
time: 2013-11-19 18:57:28.558954
success: stream1

Here is the patch which I am proposing to fix the issue.

--- open.c.orig 2013-11-20 02:20:01.027670605 -0800
+++ open.c  2013-11-20 02:19:42.897949370 -0800
@@ -3261,7 +3261,10 @@ static NTSTATUS create_file_unixpath(con
                }

                /* Open the base file. */
-               status = create_file_unixpath(conn, NULL, smb_fname_base, 0,
+               status = create_file_unixpath(conn, NULL, smb_fname_base,
+                                            (SYNCHRONIZE_ACCESS
+                                             | FILE_READ_ATTRIBUTES
+                                             | FILE_WRITE_ATTRIBUTES),
                                              FILE_SHARE_READ
                                              | FILE_SHARE_WRITE
                                              | FILE_SHARE_DELETE,

Please review this and let me know if my understanding and fix is correct to address the issue.

Thanks,
Hemanth.
Comment 1 Hemanth 2013-11-21 07:43:59 UTC
Created attachment 9453 [details]
packet capture taken during the test
Comment 2 Jeremy Allison 2013-11-23 00:19:56 UTC
(In reply to comment #0)

> 
> Here is the patch which I am proposing to fix the issue.
> 
> --- open.c.orig 2013-11-20 02:20:01.027670605 -0800
> +++ open.c  2013-11-20 02:19:42.897949370 -0800
> @@ -3261,7 +3261,10 @@ static NTSTATUS create_file_unixpath(con
>                 }
> 
>                 /* Open the base file. */
> -               status = create_file_unixpath(conn, NULL, smb_fname_base, 0,
> +               status = create_file_unixpath(conn, NULL, smb_fname_base,
> +                                            (SYNCHRONIZE_ACCESS
> +                                             | FILE_READ_ATTRIBUTES
> +                                             | FILE_WRITE_ATTRIBUTES),
>                                               FILE_SHARE_READ
>                                               | FILE_SHARE_WRITE
>                                               | FILE_SHARE_DELETE,
> 
> Please review this and let me know if my understanding and fix is correct to
> address the issue.

That looks correct to me. What does the corresponding code look like in 4.0.x and 4.1.x ? I think we already pass the test in these branches so it must be corrected there already.

Jeremy.
Comment 3 Hemanth 2013-11-23 00:46:25 UTC
Jeremy,

I have checked the code in 4.1.2, I could see that access mask as zero. 

/* Open the base file. */
		status = create_file_unixpath(conn, NULL, smb_fname_base, 0,
					      FILE_SHARE_READ
					      | FILE_SHARE_WRITE
					      | FILE_SHARE_DELETE,
					      base_create_disposition,
					      0, 0, 0, 0, 0, NULL, NULL,
					      &base_fsp, NULL);
		TALLOC_FREE(smb_fname_base);


But I am not sure whether the test getting passed against 4.1.x.
Comment 4 Hemanth 2014-09-29 23:21:24 UTC
Fix for bug 10797 addressed this test failure. closing it as dupe.

*** This bug has been marked as a duplicate of bug 10797 ***