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

[Christof Schmitt]

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).

Christof
-------------- next part --------------
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