Padding byte in cifs readx response

Christof Schmitt cs at samba.org
Thu Aug 14 23:30:52 MDT 2014


On Wed, Aug 13, 2014 at 04:10:34PM +0200, Volker Lendecke wrote:
> On Tue, Aug 12, 2014 at 04:16:37PM -0700, Christof Schmitt wrote:
> > I now see one more problem with the smbtorture3 LARGE_READX test (see
> > the last patch that disables that test, but this needs to be fixed).
> 
> 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.

Christof

> 
> Volker
> 
> -- 
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de

> From f4247e57db77ae8401a2d2a9199820a045bf0d70 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Thu, 7 Aug 2014 15:20:23 -0700
> Subject: [PATCH 01/10] VL 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>
> 
> 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.

> ---
>  source3/smbd/aio.c     |   10 ++++++----
>  source3/smbd/pipes.c   |   11 +++++++----
>  source3/smbd/process.c |    6 +++++-
>  source3/smbd/reply.c   |   16 ++++++++++------
>  4 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/source3/smbd/aio.c b/source3/smbd/aio.c
> index 2235c32..8d32d44 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"));
> @@ -221,7 +221,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 +257,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 +286,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..ab2093c 100644
> --- a/source3/smbd/process.c
> +++ b/source3/smbd/process.c
> @@ -2075,6 +2075,10 @@ 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) {
> +		new_size += 1; /* add padding byte */
> +	}
> +
>  	if ((smb_command != SMBwriteX) && (new_size > 0xffff)) {
>  		DEBUG(1, ("smb_splice_chain: %u bytes won't fit\n",
>  			  (unsigned)new_size));
> @@ -2147,7 +2151,7 @@ static bool smb_splice_chain(uint8_t **poutbuf, const uint8_t *andx_buf)
>  			+ sizeof(uint16_t);	 /* bcc */
>  
>  		SSVAL(outbuf + ofs, 6 * sizeof(uint16_t),
> -		      bytes_addr - outbuf - 4);
> +		      bytes_addr - outbuf - 4 + 1 /* padding byte */);
>  	}
>  
>  	ofs += sizeof(uint16_t) * wct;
> diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
> index 4cb446f..d5c755c 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);
> -- 
> 1.7.9.5
> 
> 
> From 546c1bb1b35de92753a9934dbb2cd344d1665e49 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 02/10] 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(+)
> 
> 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.9.5
> 
> 
> From ea065662aed4a506bfbdb889a3a329d56ed01634 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 03/10] 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 file changed, 23 insertions(+)
> 
> 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.9.5
> 
> 
> From e708d9c99231d6d5ea7ee783184e260f25ffec59 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 04/10] 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 file 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.9.5
> 
> 
> From 913df0785ca305e2ad5e3e39c3eabf6dcb9621f6 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 05/10] 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 file 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.9.5
> 
> 
> From e3ccaf920177bb4a0e108d68bad757c4de138367 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 06/10] 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 file 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.9.5
> 
> 
> From f4b070bd26e88b9b9cdefd84bf401ff3774116ad 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 07/10] 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 file changed, 4 insertions(+)
> 
> 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.9.5
> 
> 
> From daba95a790919ed8429cd535f129cd8012e6ae26 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 08/10] selftest: Add failing readx test to known fail
> 
> The new 16bit alignment check now fails.
> 
> Signed-off-by: Christof Schmitt <cs at samba.org>
> Reviewed-by: Volker Lendecke <vl at samba.org>
> ---
>  selftest/knownfail |    1 +
>  1 file changed, 1 insertion(+)
> 
> 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.9.5
> 
> 
> From dc6326080b2d935f335ec8731ceb0945ccdecb81 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Tue, 12 Aug 2014 11:43:23 -0700
> Subject: [PATCH 09/10] FIXME: smbtorture3 LARGE_READX fails
> 
> ---
>  selftest/knownfail |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 880b600..1c9f4ce 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -301,3 +301,5 @@
>  ^samba.blackbox.wbinfo\(s3member:local\).wbinfo -G check for sane mapping\(s3member:local\)
>  ^samba.ntlm_auth.\(dc:local\).ntlm_auth against winbindd with failed require-membership-of
>  ^samba.ntlm_auth.\(dc:local\).ntlm_auth with NTLMSSP gss-spnego-client and gss-spnego server against winbind with failed require-membership-of
> +#
> +^samba3.smbtorture_s3.plain\(s3dc\).LARGE_READX.smbtorture
> -- 
> 1.7.9.5
> 
> 
> From 68e931df225b37efd12bb3b55e111167f9208c5c Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 13 Aug 2014 16:08:40 +0200
> Subject: [PATCH 10/10] fix?
> 
> ---
>  source3/smbd/reply.c      |    2 +-
>  source3/torture/torture.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
> index d5c755c..1b3159d 100644
> --- a/source3/smbd/reply.c
> +++ b/source3/smbd/reply.c
> @@ -3972,7 +3972,7 @@ static size_t calc_read_size(const struct smb_request *req,
>  	struct smbXsrv_connection *xconn = req->xconn;
>  	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 hdr_len = MIN_SMB_SIZE + VWV(12) + 1;
>  	size_t max_len = max_pdu - hdr_len;
>  
>  	/*
> diff --git a/source3/torture/torture.c b/source3/torture/torture.c
> index ba23a85..951e6df 100644
> --- a/source3/torture/torture.c
> +++ b/source3/torture/torture.c
> @@ -7258,7 +7258,7 @@ 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));
>  }
>  
>  static bool check_read_call(struct cli_state *cli,
> -- 
> 1.7.9.5
> 

-------------- next part --------------
>From c05e8074c9465ab3effb4926116aa5fa8ac94046 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 72c161c0532136546c3b5b266deb25f8adb9f1db 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 cfe185095a331c55c447e5dde47ca4de7d4b2268 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 |    6 +++++-
 source3/smbd/reply.c   |   18 +++++++++++-------
 4 files changed, 30 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..ab2093c 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -2075,6 +2075,10 @@ 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) {
+		new_size += 1; /* add padding byte */
+	}
+
 	if ((smb_command != SMBwriteX) && (new_size > 0xffff)) {
 		DEBUG(1, ("smb_splice_chain: %u bytes won't fit\n",
 			  (unsigned)new_size));
@@ -2147,7 +2151,7 @@ static bool smb_splice_chain(uint8_t **poutbuf, const uint8_t *andx_buf)
 			+ sizeof(uint16_t);	 /* bcc */
 
 		SSVAL(outbuf + ofs, 6 * sizeof(uint16_t),
-		      bytes_addr - outbuf - 4);
+		      bytes_addr - outbuf - 4 + 1 /* padding byte */);
 	}
 
 	ofs += sizeof(uint16_t) * wct;
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 fcddcff0b74ead843ae23ebd0757ee83458ea5b8 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 bcd8b045805fb41226fa14de0285b173927e691e 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 505682453d5c4dda503a0da653a945c309e0e8b3 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 d8e81deb5817244e7f294b62d614f01e625932a1 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 82f8cbf2a8603c60fb41d7f7a60b83f0266d6919 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 aae16346cafe644f582dfe9acdaa66c7666e22c9 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