Padding byte in cifs readx response

Christof Schmitt cs at samba.org
Fri Aug 15 16:38:19 MDT 2014


On Fri, Aug 15, 2014 at 03:32:20PM -0700, Christof Schmitt wrote:
> On Fri, Aug 15, 2014 at 11:18:20AM -0700, Jeremy Allison wrote:
> > On Fri, Aug 15, 2014 at 07:27:21PM +0200, Volker Lendecke wrote:
> > > On Fri, Aug 15, 2014 at 10:24:22AM -0700, Jeremy Allison wrote:
> > > > On Fri, Aug 15, 2014 at 07:22:16PM +0200, Volker Lendecke wrote:
> > > > > On Fri, Aug 15, 2014 at 10:17:15AM -0700, Jeremy Allison wrote:
> > > > > > On Thu, Aug 14, 2014 at 10:30:52PM -0700, Christof Schmitt wrote:
> > > > > > > On Wed, Aug 13, 2014 at 04:10:34PM +0200, Volker Lendecke wrote:
> > > > > > > > We just overflowed the 16MB nbss packet. I've attached one
> > > > > > > > question, a few R-Bs and a possible fix.
> > > > > > > 
> > > > > > > Thanks. The fix looks good, i included it in the patch series, see
> > > > > > > attachment.
> > > > > > > > 
> > > > > > > > VL: I think at least in the aio case the padding byte is left uninitialized.
> > > > > > > > Can you check that? Thanks!
> > > > > > > 
> > > > > > > Yes, i missed that. struct aio_extra is allocated and zeroed, but not
> > > > > > > the following data buffer. I added the explicit initialization of the
> > > > > > > padding byte.
> > > > > > 
> > > > > > LGTM Christof thanks ! Pushed.
> > > > > 
> > > > > Wait please!
> > > > > 
> > > > > I'm still dubious about the new_size += 1 in
> > > > > smb_splice_chain. Can you explain that? I'm really nervous
> > > > > about that piece of the code, &x is really from hell.
> > > > 
> > > > Ok, will hold off on that :-).
> > > > 
> > > > andX is indeed from the darkest pits of hades. But
> > > > I thought you'd looked at that already - my mistake,
> > > > sorry !
> > > 
> > > I would have thought that read&x already has that byte in
> > > smb_buf, so that new_size will automatically have it. The
> > > offset calculation -- sure. But the size increment?
> > 
> > Oh, that's a really good point. new_size is only
> > used for talloc_realloc the outbuf buffer, not
> > for setting any sizes within - so if it's off
> > by one all further chain entries will creep
> > forward by one also...
> > 
> > And chain_padding can only be up to 4 bytes,
> > so after 4 chained readX requests we'd be
> > screwed. Not an easy thing to test !
> 
> I took another look. The reason for changing the new_size was that it
> only accounted for the data without the padding byte, but the bytes
> pointer pointed to the padding byte. On a closer look this is not
> entirely correct, since num_bytes is also used later for the byte count
> field. Adjusting new_size instead might be a better approach. I will
> take another look at this area.

I *think* the attached patches should be correct, but please review them
carefully. Next week i should also be able to get more testing done.

Christof
-------------- next part --------------
>From e0b34262ec014bd818a4f2ab39cbbe7178a48f2d Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 14 Aug 2014 22:03:22 -0700
Subject: [PATCH 1/9] torture3: Allow padding byte for LARGE_READX responses

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/torture/torture.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index ba23a85..987b23c 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -7258,7 +7258,8 @@ static size_t calc_expected_return(struct cli_state *cli, size_t len_requested)
 		len_requested &= 0xFFFF;
 	}
 
-	return MIN(len_requested, max_pdu - (MIN_SMB_SIZE + VWV(12)));
+	return MIN(len_requested,
+		   max_pdu - (MIN_SMB_SIZE + VWV(12) + 1 /* padding byte */));
 }
 
 static bool check_read_call(struct cli_state *cli,
-- 
1.7.1


>From b7a61281ea65a8c2bbe4f3e0524cc810e6a467a4 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 8 Aug 2014 10:48:55 -0700
Subject: [PATCH 2/9] selftest: Add readx test for dc to known fail

The new 16bit alignment check will fail.

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 selftest/knownfail |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 214a170..880b600 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -179,6 +179,7 @@
 ^samba4.blackbox.upgradeprovision.alpha13.ldapcmp_sd\(none\) # Due to something rewriting the NT ACL on DNS objects
 ^samba4.blackbox.upgradeprovision.alpha13.ldapcmp_full_sd\(none\) # Due to something rewriting the NT ACL on DNS objects
 ^samba4.blackbox.upgradeprovision.release-4-0-0.ldapcmp_sd\(none\) # Due to something rewriting the NT ACL on DNS objects
+^samba4.raw.read.readx\(dc\) # fails readx 16bit alignment requirement
 ^samba3.smb2.create.gentest
 ^samba3.smb2.create.blob
 ^samba3.smb2.create.open
-- 
1.7.1


>From 2ff06e8559699ba4b9dd8173917b604082d6740c Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 14 Aug 2014 22:04:33 -0700
Subject: [PATCH 3/9] smbd: Add padding byte to readx response

MS-CIFS 2.2.4.42.2 states: "Pad (1 byte): This field is optional. When
using the NT LAN Manager dialect, this field can be used to align the
Data field to a 16-bit boundary relative to the start of the SMB Header.
If Unicode strings are being used, this field MUST be present. When
used, this field MUST be one padding byte long."

Always add the padding byte to all readx responses to avoid additional
complexity.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/smbd/aio.c     |   11 +++++++----
 source3/smbd/pipes.c   |   11 +++++++----
 source3/smbd/process.c |   14 +++++++++++++-
 source3/smbd/reply.c   |   18 +++++++++++-------
 4 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/source3/smbd/aio.c b/source3/smbd/aio.c
index 2235c32..f5e4cc6 100644
--- a/source3/smbd/aio.c
+++ b/source3/smbd/aio.c
@@ -196,7 +196,7 @@ NTSTATUS schedule_aio_read_and_X(connection_struct *conn,
 	/* The following is safe from integer wrap as we've already checked
 	   smb_maxcnt is 128k or less. Wct is 12 for read replies */
 
-	bufsize = smb_size + 12 * 2 + smb_maxcnt;
+	bufsize = smb_size + 12 * 2 + smb_maxcnt + 1 /* padding byte */;
 
 	if ((aio_ex = create_aio_extra(NULL, fsp, bufsize)) == NULL) {
 		DEBUG(10,("schedule_aio_read_and_X: malloc fail.\n"));
@@ -206,6 +206,7 @@ NTSTATUS schedule_aio_read_and_X(connection_struct *conn,
 	construct_reply_common_req(smbreq, (char *)aio_ex->outbuf.data);
 	srv_set_message((char *)aio_ex->outbuf.data, 12, 0, True);
 	SCVAL(aio_ex->outbuf.data,smb_vwv0,0xFF); /* Never a chained reply. */
+	SCVAL(smb_buf(aio_ex->outbuf.data), 0, 0); /* padding byte */
 
 	init_strict_lock_struct(fsp, (uint64_t)smbreq->smbpid,
 		(uint64_t)startpos, (uint64_t)smb_maxcnt, READ_LOCK,
@@ -221,7 +222,8 @@ NTSTATUS schedule_aio_read_and_X(connection_struct *conn,
 	aio_ex->offset = startpos;
 
 	req = SMB_VFS_PREAD_SEND(aio_ex, fsp->conn->sconn->ev_ctx,
-				 fsp, smb_buf(aio_ex->outbuf.data),
+				 fsp,
+				 smb_buf(aio_ex->outbuf.data) + 1 /* pad */,
 				 smb_maxcnt, startpos);
 	if (req == NULL) {
 		DEBUG(0,("schedule_aio_read_and_X: aio_read failed. "
@@ -256,7 +258,7 @@ static void aio_pread_smb1_done(struct tevent_req *req)
 	files_struct *fsp = aio_ex->fsp;
 	int outsize;
 	char *outbuf = (char *)aio_ex->outbuf.data;
-	char *data = smb_buf(outbuf);
+	char *data = smb_buf(outbuf) + 1 /* padding byte */;
 	ssize_t nread;
 	int err;
 
@@ -285,7 +287,8 @@ static void aio_pread_smb1_done(struct tevent_req *req)
 		ERROR_NT(map_nt_error_from_unix(err));
 		outsize = srv_set_message(outbuf,0,0,true);
 	} else {
-		outsize = srv_set_message(outbuf, 12, nread, False);
+		outsize = srv_set_message(outbuf, 12,
+					  nread + 1 /* padding byte */, false);
 		SSVAL(outbuf,smb_vwv2, 0xFFFF); /* Remaining - must be * -1. */
 		SSVAL(outbuf,smb_vwv5, nread);
 		SSVAL(outbuf,smb_vwv6, smb_offset(data,outbuf));
diff --git a/source3/smbd/pipes.c b/source3/smbd/pipes.c
index f1f61bb..110951c 100644
--- a/source3/smbd/pipes.c
+++ b/source3/smbd/pipes.c
@@ -422,11 +422,12 @@ void reply_pipe_read_and_X(struct smb_request *req)
 	state->smb_maxcnt = SVAL(req->vwv+5, 0);
 	state->smb_mincnt = SVAL(req->vwv+6, 0);
 
-	reply_outbuf(req, 12, state->smb_maxcnt);
+	reply_outbuf(req, 12, state->smb_maxcnt + 1 /* padding byte */);
 	SSVAL(req->outbuf, smb_vwv0, 0xff); /* andx chain ends */
 	SSVAL(req->outbuf, smb_vwv1, 0);    /* no andx offset */
+	SCVAL(smb_buf(req->outbuf), 0, 0); /* padding byte */
 
-	data = (uint8_t *)smb_buf(req->outbuf);
+	data = (uint8_t *)smb_buf(req->outbuf) + 1 /* padding byte */;
 
 	/*
 	 * We have to tell the upper layers that we're async.
@@ -467,7 +468,8 @@ static void pipe_read_andx_done(struct tevent_req *subreq)
 	req->outbuf = state->outbuf;
 	state->outbuf = NULL;
 
-	srv_set_message((char *)req->outbuf, 12, nread, False);
+	srv_set_message((char *)req->outbuf, 12, nread + 1 /* padding byte */,
+			false);
 
 #if 0
 	/*
@@ -488,7 +490,8 @@ static void pipe_read_andx_done(struct tevent_req *subreq)
 	      (smb_wct - 4)	/* offset from smb header to wct */
 	      + 1 		/* the wct field */
 	      + 12 * sizeof(uint16_t) /* vwv */
-	      + 2);		/* the buflen field */
+	      + 2		/* the buflen field */
+	      + 1);		/* padding byte */
 	SSVAL(req->outbuf,smb_vwv11,state->smb_maxcnt);
 
 	DEBUG(3,("readX-IPC min=%d max=%d nread=%d\n",
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 7148462..cd3dab7 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -2075,6 +2075,17 @@ static bool smb_splice_chain(uint8_t **poutbuf, const uint8_t *andx_buf)
 	new_size = old_size + chain_padding + 1 + wct * sizeof(uint16_t) + 2;
 	new_size += num_bytes;
 
+	if (smb_command == SMBreadX) {
+		/*
+		 * The readx data buffer starts with a padding byte
+		 * and the bytes pointer points to that. num_bytes
+		 * contains only the data, so adjust the allocation
+		 * size here to have space for padding byte and data
+		 * buffer.
+		 */
+		new_size += 1;
+	}
+
 	if ((smb_command != SMBwriteX) && (new_size > 0xffff)) {
 		DEBUG(1, ("smb_splice_chain: %u bytes won't fit\n",
 			  (unsigned)new_size));
@@ -2144,7 +2155,8 @@ static bool smb_splice_chain(uint8_t **poutbuf, const uint8_t *andx_buf)
 
 		bytes_addr = outbuf + ofs	 /* vwv start */
 			+ sizeof(uint16_t) * wct /* vwv array */
-			+ sizeof(uint16_t);	 /* bcc */
+			+ sizeof(uint16_t)	 /* bcc */
+			+ 1;			 /* padding byte */
 
 		SSVAL(outbuf + ofs, 6 * sizeof(uint16_t),
 		      bytes_addr - outbuf - 4);
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 4cb446f..85c1531 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -3693,11 +3693,14 @@ static int setup_readX_header(struct smb_request *req, char *outbuf,
 	      (smb_wct - 4)	/* offset from smb header to wct */
 	      + 1 		/* the wct field */
 	      + 12 * sizeof(uint16_t) /* vwv */
-	      + 2);		/* the buflen field */
+	      + 2		/* the buflen field */
+	      + 1);		/* padding byte */
 	SSVAL(outbuf,smb_vwv7,(smb_maxcnt >> 16));
 	SSVAL(outbuf,smb_vwv11,smb_maxcnt);
+	SCVAL(smb_buf(outbuf), 0, 0); /* padding byte */
 	/* Reset the outgoing length, set_message truncates at 0x1FFFF. */
-	_smb_setlen_large(outbuf,(smb_size + 12*2 + smb_maxcnt - 4));
+	_smb_setlen_large(outbuf,
+			  smb_size + 12*2 + smb_maxcnt - 4 + 1 /* pad */);
 	return outsize;
 }
 
@@ -3734,7 +3737,7 @@ static void send_file_readX(connection_struct *conn, struct smb_request *req,
 	    (fsp->base_fsp == NULL) &&
 	    (fsp->wcp == NULL) &&
 	    lp_use_sendfile(SNUM(conn), xconn->smb1.signing_state) ) {
-		uint8 headerbuf[smb_size + 12 * 2];
+		uint8 headerbuf[smb_size + 12 * 2 + 1 /* padding byte */];
 		DATA_BLOB header;
 
 		if(fsp_stat(fsp) == -1) {
@@ -3848,7 +3851,7 @@ static void send_file_readX(connection_struct *conn, struct smb_request *req,
 normal_read:
 
 	if ((smb_maxcnt & 0xFF0000) > 0x10000) {
-		uint8 headerbuf[smb_size + 2*12];
+		uint8 headerbuf[smb_size + 2*12 + 1 /* padding byte */];
 		ssize_t ret;
 
 		construct_reply_common_req(req, (char *)headerbuf);
@@ -3887,11 +3890,12 @@ normal_read:
 
 nosendfile_read:
 
-	reply_outbuf(req, 12, smb_maxcnt);
+	reply_outbuf(req, 12, smb_maxcnt + 1 /* padding byte */);
 	SSVAL(req->outbuf, smb_vwv0, 0xff); /* andx chain ends */
 	SSVAL(req->outbuf, smb_vwv1, 0);    /* no andx offset */
 
-	nread = read_file(fsp, smb_buf(req->outbuf), startpos, smb_maxcnt);
+	nread = read_file(fsp, smb_buf(req->outbuf) + 1 /* padding byte */,
+			  startpos, smb_maxcnt);
 	saved_errno = errno;
 
 	SMB_VFS_STRICT_UNLOCK(conn, fsp, &lock);
@@ -3969,7 +3973,7 @@ static size_t calc_read_size(const struct smb_request *req,
 	size_t max_pdu = calc_max_read_pdu(req);
 	size_t total_size = 0;
 	size_t hdr_len = MIN_SMB_SIZE + VWV(12);
-	size_t max_len = max_pdu - hdr_len;
+	size_t max_len = max_pdu - hdr_len - 1 /* padding byte */;
 
 	/*
 	 * Windows explicitly ignores upper size of 0xFFFF.
-- 
1.7.1


>From 29fb7a98fe7be054e58740f1922ea102c060be2d Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 7 Aug 2014 14:19:57 -0700
Subject: [PATCH 4/9] s4:libcli/raw: Make flags2 and offset available to callers of readx

This will be used by smbtorture.

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source4/libcli/raw/interfaces.h   |    2 ++
 source4/libcli/raw/rawreadwrite.c |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/source4/libcli/raw/interfaces.h b/source4/libcli/raw/interfaces.h
index 12c0377..9003c12 100644
--- a/source4/libcli/raw/interfaces.h
+++ b/source4/libcli/raw/interfaces.h
@@ -1809,6 +1809,8 @@ union smb_read {
 			uint16_t remaining;
 			uint16_t compaction_mode;
 			uint32_t nread;
+			uint16_t flags2;
+			uint16_t data_offset;
 		} out;
 	} readx, generic;
 
diff --git a/source4/libcli/raw/rawreadwrite.c b/source4/libcli/raw/rawreadwrite.c
index d3f5518..fb44ba4 100644
--- a/source4/libcli/raw/rawreadwrite.c
+++ b/source4/libcli/raw/rawreadwrite.c
@@ -155,6 +155,8 @@ _PUBLIC_ NTSTATUS smb_raw_read_recv(struct smbcli_request *req, union smb_read *
 		parms->readx.out.remaining       = SVAL(req->in.vwv, VWV(2));
 		parms->readx.out.compaction_mode = SVAL(req->in.vwv, VWV(3));
 		parms->readx.out.nread = SVAL(req->in.vwv, VWV(5));
+		parms->readx.out.flags2 = req->flags2;
+		parms->readx.out.data_offset = SVAL(req->in.vwv, VWV(6));
 
 		/* handle oversize replies for non-chained readx replies with
 		   CAP_LARGE_READX. The snia spec has must to answer for. */
-- 
1.7.1


>From a5c5d818e0afd132b356ccf97b69384f09e7fa4a Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 7 Aug 2014 14:25:13 -0700
Subject: [PATCH 5/9] torture: Add test for 16 bit alignment of readx data

MS-CIFS requires a one byte pad to guarantee 16 bit alignment of the
data:

Pad (1 byte): This field is optional. When using the NT LAN Manager
dialect, this field can be used to align the Data field to a 16-bit
boundary relative to the start of the SMB Header. If Unicode strings are
being used, this field MUST be present. When used, this field MUST be
one padding byte long.

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source4/torture/raw/read.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/source4/torture/raw/read.c b/source4/torture/raw/read.c
index 59089bf..f176e7f 100644
--- a/source4/torture/raw/read.c
+++ b/source4/torture/raw/read.c
@@ -47,6 +47,13 @@
 		goto done; \
 	}} while (0)
 
+#define CHECK_READX_ALIGN(io) do {			      \
+	if ((io.readx.out.flags2 & FLAGS2_UNICODE_STRINGS) && \
+	    (io.readx.out.data_offset % 2 != 0)) { \
+		ret = false; \
+		torture_fail_goto(tctx, done, "data not 16 bit aligned\n"); \
+	}} while (0)
+
 #define BASEDIR "\\testread"
 
 
@@ -399,6 +406,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.nread, 0);
 	CHECK_VALUE(io.readx.out.remaining, 0xFFFF);
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
+	CHECK_READX_ALIGN(io);
 
 	printf("Trying zero file read\n");
 	io.readx.in.mincnt = 0;
@@ -408,6 +416,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.nread, 0);
 	CHECK_VALUE(io.readx.out.remaining, 0xFFFF);
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
+	CHECK_READX_ALIGN(io);
 
 	printf("Trying bad fnum\n");
 	io.readx.in.file.fnum = fnum+1;
@@ -429,6 +438,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.nread, strlen(test_data));
 	CHECK_VALUE(io.readx.out.remaining, 0xFFFF);
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
+	CHECK_READX_ALIGN(io);
 	if (memcmp(buf, test_data, strlen(test_data)) != 0) {
 		ret = false;
 		printf("incorrect data at %d!? (%s:%s)\n", __LINE__, test_data, buf);
@@ -444,6 +454,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.nread, strlen(test_data)-1);
 	CHECK_VALUE(io.readx.out.remaining, 0xFFFF);
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
+	CHECK_READX_ALIGN(io);
 	if (memcmp(buf, test_data+1, strlen(test_data)-1) != 0) {
 		ret = false;
 		printf("incorrect data at %d!? (%s:%s)\n", __LINE__, test_data+1, buf);
@@ -460,6 +471,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 		CHECK_VALUE(io.readx.out.nread, 0);
 		CHECK_VALUE(io.readx.out.remaining, 0xFFFF);
 		CHECK_VALUE(io.readx.out.compaction_mode, 0);
+		CHECK_READX_ALIGN(io);
 	}
 
 	printf("Trying mincnt past EOF\n");
@@ -472,6 +484,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.remaining, 0xFFFF);
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
 	CHECK_VALUE(io.readx.out.nread, strlen(test_data));
+	CHECK_READX_ALIGN(io);
 	if (memcmp(buf, test_data, strlen(test_data)) != 0) {
 		ret = false;
 		printf("incorrect data at %d!? (%s:%s)\n", __LINE__, test_data, buf);
@@ -493,6 +506,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
 	CHECK_VALUE(io.readx.out.nread, io.readx.in.maxcnt);
 	CHECK_BUFFER(buf, seed, io.readx.out.nread);
+	CHECK_READX_ALIGN(io);
 
 	printf("Trying page + 1 sized read (check alignment)\n");
 	io.readx.in.offset = 0;
@@ -504,6 +518,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
 	CHECK_VALUE(io.readx.out.nread, io.readx.in.maxcnt);
 	CHECK_BUFFER(buf, seed, io.readx.out.nread);
+	CHECK_READX_ALIGN(io);
 
 	printf("Trying large read (UINT16_MAX)\n");
 	io.readx.in.offset = 0;
@@ -515,6 +530,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
 	CHECK_VALUE(io.readx.out.nread, io.readx.in.maxcnt);
 	CHECK_BUFFER(buf, seed, io.readx.out.nread);
+	CHECK_READX_ALIGN(io);
 
 	printf("Trying extra large read\n");
 	io.readx.in.offset = 0;
@@ -531,6 +547,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 		CHECK_VALUE(io.readx.out.nread, 0x10000);
 	}
 	CHECK_BUFFER(buf, seed, io.readx.out.nread);
+	CHECK_READX_ALIGN(io);
 
 	printf("Trying mincnt > maxcnt\n");
 	memset(buf, 0, maxsize);
@@ -543,6 +560,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
 	CHECK_VALUE(io.readx.out.nread, io.readx.in.maxcnt);
 	CHECK_BUFFER(buf, seed, io.readx.out.nread);
+	CHECK_READX_ALIGN(io);
 
 	printf("Trying mincnt < maxcnt\n");
 	memset(buf, 0, maxsize);
@@ -555,6 +573,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	CHECK_VALUE(io.readx.out.compaction_mode, 0);
 	CHECK_VALUE(io.readx.out.nread, io.readx.in.maxcnt);
 	CHECK_BUFFER(buf, seed, io.readx.out.nread);
+	CHECK_READX_ALIGN(io);
 
 	if (cli->transport->negotiate.capabilities & CAP_LARGE_READX) {
 		printf("Trying large readx\n");
@@ -564,11 +583,13 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 		status = smb_raw_read(cli->tree, &io);
 		CHECK_STATUS(status, NT_STATUS_OK);
 		CHECK_VALUE(io.readx.out.nread, 0xFFFF);
+		CHECK_READX_ALIGN(io);
 
 		io.readx.in.maxcnt = 0x10000;
 		status = smb_raw_read(cli->tree, &io);
 		CHECK_STATUS(status, NT_STATUS_OK);
 		CHECK_VALUE(io.readx.out.nread, 0x10000);
+		CHECK_READX_ALIGN(io);
 
 		io.readx.in.maxcnt = 0x10001;
 		status = smb_raw_read(cli->tree, &io);
@@ -610,6 +631,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	status = smb_raw_read(cli->tree, &io);
 	CHECK_STATUS(status, NT_STATUS_OK);
 	CHECK_VALUE(io.readx.out.nread, 0);
+	CHECK_READX_ALIGN(io);
 
 	if (NT_STATUS_IS_ERR(smbcli_lock64(cli->tree, fnum, io.readx.in.offset, 1, 0, WRITE_LOCK))) {
 		printf("Failed to lock file at %d\n", __LINE__);
@@ -620,6 +642,7 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	status = smb_raw_read(cli->tree, &io);
 	CHECK_STATUS(status, NT_STATUS_OK);
 	CHECK_VALUE(io.readx.out.nread, 0);
+	CHECK_READX_ALIGN(io);
 
 done:
 	smbcli_close(cli->tree, fnum);
-- 
1.7.1


>From 6c5f8a54a764be4c1db82ef7bb7345e8dcf7edcd Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 7 Aug 2014 14:31:42 -0700
Subject: [PATCH 6/9] torture: Use torture_fail macro in check_buffer for read requests

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source4/torture/raw/read.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/source4/torture/raw/read.c b/source4/torture/raw/read.c
index f176e7f..8551c0b 100644
--- a/source4/torture/raw/read.c
+++ b/source4/torture/raw/read.c
@@ -42,9 +42,9 @@
 	}} while (0)
 
 #define CHECK_BUFFER(buf, seed, len) do { \
-	if (!check_buffer(buf, seed, len, __LINE__)) { \
+	if (!check_buffer(tctx, buf, seed, len, __LINE__)) {	\
 		ret = false; \
-		goto done; \
+		torture_fail_goto(tctx, done, "buffer check failed\n"); \
 	}} while (0)
 
 #define CHECK_READX_ALIGN(io) do {			      \
@@ -70,15 +70,17 @@ static void setup_buffer(uint8_t *buf, unsigned int seed, int len)
 /*
   check a random buffer based on a seed
 */
-static bool check_buffer(uint8_t *buf, unsigned int seed, int len, int line)
+static bool check_buffer(struct torture_context *tctx, uint8_t *buf,
+			 unsigned int seed, int len, int line)
 {
 	int i;
 	srandom(seed);
 	for (i=0;i<len;i++) {
 		uint8_t v = random();
 		if (buf[i] != v) {
-			printf("Buffer incorrect at line %d! ofs=%d v1=0x%x v2=0x%x\n", 
-			       line, i, buf[i], v);
+			torture_warning(tctx, "Buffer incorrect at line %d! "
+					"ofs=%d v1=0x%x v2=0x%x\n", line, i,
+					buf[i], v);
 			return false;
 		}
 	}
-- 
1.7.1


>From b535cc572caa49a34397ec983870602e208c157e Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 7 Aug 2014 14:40:00 -0700
Subject: [PATCH 7/9] torture: Use torture_assert macro for status check in raw.read

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source4/torture/raw/read.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/source4/torture/raw/read.c b/source4/torture/raw/read.c
index 8551c0b..365b385 100644
--- a/source4/torture/raw/read.c
+++ b/source4/torture/raw/read.c
@@ -26,12 +26,9 @@
 #include "torture/raw/proto.h"
 
 #define CHECK_STATUS(status, correct) do { \
-	if (!NT_STATUS_EQUAL(status, correct)) { \
-		printf("(%s) Incorrect status %s - should be %s\n", \
-		       __location__, nt_errstr(status), nt_errstr(correct)); \
-		ret = false; \
-		goto done; \
-	}} while (0)
+		torture_assert_ntstatus_equal_goto(tctx, status, correct, ret, \
+						   done, "incorrect status"); \
+	} while (0)
 
 #define CHECK_VALUE(v, correct) do { \
 	if ((v) != (correct)) { \
-- 
1.7.1


>From c247e37abdc5f459a249bca889e22520628f2f7e Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 7 Aug 2014 14:44:23 -0700
Subject: [PATCH 8/9] torture: Use torture_assert macro for value check in raw.read

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source4/torture/raw/read.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/source4/torture/raw/read.c b/source4/torture/raw/read.c
index 365b385..6abf08b 100644
--- a/source4/torture/raw/read.c
+++ b/source4/torture/raw/read.c
@@ -31,12 +31,9 @@
 	} while (0)
 
 #define CHECK_VALUE(v, correct) do { \
-	if ((v) != (correct)) { \
-		printf("(%s) Incorrect value %s=%ld - should be %ld\n", \
-		       __location__, #v, (long)v, (long)correct); \
-		ret = false; \
-		goto done; \
-	}} while (0)
+		torture_assert_int_equal_goto(tctx, (v), (correct), ret, done, \
+					      "Incorrect value"); \
+	} while (0)
 
 #define CHECK_BUFFER(buf, seed, len) do { \
 	if (!check_buffer(tctx, buf, seed, len, __LINE__)) {	\
-- 
1.7.1


>From 39b083cbbe984a90aa33e665a9c62b56cac9a433 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 7 Aug 2014 15:42:05 -0700
Subject: [PATCH 9/9] torture: Also run raw.read against the aio share

This tests the changes in the aio code path.

Signed-off-by: Christof Schmitt <cs at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source3/selftest/tests.py |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index e9b91d4..796865c 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -355,6 +355,10 @@ for t in tests:
         plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio')
         plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
         plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
+    elif t == "raw.read":
+        plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
+        plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio')
+        plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
     elif t == "raw.search":
         plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
 # test the dirsort module.
-- 
1.7.1



More information about the samba-technical mailing list