[PATCH] s3: vfs: time_audit: fix handling of token_blob in smb_time_audit_offload_read_recv()

Ralph Wuerthner ralphw at de.ibm.com
Fri Aug 10 08:09:23 UTC 2018


On 09.08.2018 22:05, Christof Schmitt wrote:
> On Thu, Aug 09, 2018 at 12:46:32PM -0700, Christof Schmitt via samba-technical wrote:
>> On Thu, Aug 09, 2018 at 04:15:23PM +0200, Ralph Wuerthner via samba-technical wrote:
>>> Hi list,
>>>
>>> please see attached patch to fix a defect within vfs_time_audit.
>>> When using vfs_time_audit smbtorture test case
>>> smb2.ioctl.req_resume_key is failing with the following error
>>> message:
>>>
>>> Using seed 1533822459
>>> time: 2018-08-09 15:47:39.669901
>>> test: req_resume_key
>>> time: 2018-08-09 15:47:39.670594
>>> time: 2018-08-09 15:47:39.712858
>>> failure: req_resume_key [
>>> ../source4/torture/smb2/ioctl.c:104: status was
>>> NT_STATUS_INTERNAL_ERROR, expected NT_STATUS_OK:
>>> FSCTL_SRV_REQUEST_RESUME_KEY
>>> ]
>>>
>>> A similar failure can be seen by using vfs_fruit and vfs_time_audit
>>> when running vfs.fruit.copyfile:
>>>
>>> Using seed 1533822525
>>> time: 2018-08-09 15:48:45.461933
>>> test: copyfile
>>> time: 2018-08-09 15:48:45.464498
>>> WARNING!: ../source4/torture/vfs/fruit.c:2482: status was
>>> NT_STATUS_NO_MEMORY, expected NT_STATUS_OK:
>>> FSCTL_SRV_REQUEST_RESUME_KEY
>>> time: 2018-08-09 15:48:45.548604
>>> failure: copyfile [
>>> ../source4/torture/vfs/fruit.c:2637: setup copy chunk error
>>> ]
>>
>> Good catch! I think the intention of the code was to first use a local
>> variable and then assign the result to the pointer. That pattern is
>> common in the Samba code.
>>
>>> Dump question: wouldn't it make sense to run all self tests with
>>> vfs_time_audit enabled as this module is by definition a no-op?
>>
>> Good question. At least in master, the "simpleserver" environment
>> already includes time_audit. The gap is that smb2.ioctl is not running
>> against the simpleserver.
>>
>> Attached are two patches, one is a proposed shorter fix, the other one is
>> for running smb2.ioctl also against simpleserver. If you only apply the
>> second patch, you can easily see the failing test:
>>
>> ./configure --picky-developer --enable-selftest
>> make -j
>> make test TESTS=smb2.ioctl
>>
>> If you agree with the fix, then we should open a bugzilla and also
>> backport the fix to 4.8 (i have not check 4.7 yet).
> 
> Just confirmed that 4.7 has the same problem. We should backport this
> to 4.7, 4.8 and 4.9rc.
> 

I just opened Samba bugzilla 13568 to track the backports.

>> Christof
> 
>>  From a93ccc62f3eb34a2e66fbdd4700701440b76f834 Mon Sep 17 00:00:00 2001
>> From: Christof Schmitt <cs at samba.org>
>> Date: Thu, 9 Aug 2018 12:39:34 -0700
>> Subject: [PATCH 1/2] s3: vfs: time_audit: fix handling of token_blob in
>>   smb_time_audit_offload_read_recv()
>>
>> Found by Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
>>
>> Signed-off-by: Christof Schmitt <cs at samba.org>
>> ---
>>   source3/modules/vfs_time_audit.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/source3/modules/vfs_time_audit.c b/source3/modules/vfs_time_audit.c
>> index 64e1f5d..5754a07 100644
>> --- a/source3/modules/vfs_time_audit.c
>> +++ b/source3/modules/vfs_time_audit.c
>> @@ -2016,6 +2016,8 @@ static NTSTATUS smb_time_audit_offload_read_recv(
>>   		return NT_STATUS_NO_MEMORY;
>>   	}
>>   
>> +	*_token_blob = token_blob;
>> +
>>   	tevent_req_received(req);
>>   	return NT_STATUS_OK;
>>   }
>> -- 
>> 1.8.3.1
>>
>>
>>  From 86f0afdc6d99e16371cd0a1bb71fa85764a110ef Mon Sep 17 00:00:00 2001
>> From: Christof Schmitt <cs at samba.org>
>> Date: Thu, 9 Aug 2018 12:40:11 -0700
>> Subject: [PATCH 2/2] selftest: Also run smb2.ioctl against simpleserver
>>
>> Signed-off-by: Christof Schmitt <cs at samba.org>
>> ---
>>   source3/selftest/tests.py | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>> index d3c5487..d552f7b 100755
>> --- a/source3/selftest/tests.py
>> +++ b/source3/selftest/tests.py
>> @@ -516,6 +516,7 @@ for t in tests:
>>           plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/fs_specific -U$USERNAME%$PASSWORD', 'fs_specific')
>>           plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>>           plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
>> +        plansmbtorture4testsuite(t, "simpleserver", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>>       elif t == "smb2.lock":
>>           plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio')
>>           plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>> -- 
>> 1.8.3.1
>>
> 
Regards

Ralph




More information about the samba-technical mailing list