The Samba-Bugzilla – Attachment 18257 Details for
Bug 15579
error output with wspsearch
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
prep patches + backport of master patch to 4.20
array-check-4.20-backport-option2.patch (text/plain), 19.01 KB, created by
Noel Power
on 2024-02-20 12:03:37 UTC
(
hide
)
Description:
prep patches + backport of master patch to 4.20
Filename:
MIME Type:
Creator:
Noel Power
Created:
2024-02-20 12:03:37 UTC
Size:
19.01 KB
patch
obsolete
>From 4fb578cce1ba0b605423ecb3bd79a47c1fa07f60 Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Mon, 8 Jan 2024 15:12:35 +0000 >Subject: [PATCH 1/7] s3/rpc_client: change type of offset to uint64_t > >Offset can be a 32 or 64 bit address depending on the indexing addressing >mode negotiated by the client >With a 32 bit param we can only specify a 32 bit base address. This change >alone doesn't affect anything as it is the client itself that choses and >passes the base address offset and wspsearch is the only current user of >this code. >In this case even with 64bit addressing negotiated the address passed >represents only the lower 32-bits part of the address. >However, for coverage purposes it would be better for the client to use an >address that covers the full 64bit range of the address (when 64 bit >addressing is negotiated). >This change will alow the wspsearch client in a future commit to pass a >base address value with both the hi and low 32 bits values set to make up >the full 64 bit address. > >Signed-off-by: Noel Power <noel.power@suse.com> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >(cherry picked from commit a61eb7032896265eaef3ba225aafd6f293e7569d) >--- > source3/rpc_client/wsp_cli.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > >diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c >index b21f55c86f0..450bd2525e0 100644 >--- a/source3/rpc_client/wsp_cli.c >+++ b/source3/rpc_client/wsp_cli.c >@@ -761,7 +761,7 @@ void create_seekat_getrows_request(TALLOC_CTX * ctx, > > static bool extract_rowbuf_variable_type(TALLOC_CTX *ctx, > uint16_t type, >- uint32_t offset, >+ uint64_t offset, > DATA_BLOB *rows_buf, uint32_t len, > struct wsp_cbasestoragevariant *val) > { >@@ -770,7 +770,7 @@ static bool extract_rowbuf_variable_type(TALLOC_CTX *ctx, > ndr_flags_type ndr_flags = NDR_SCALARS | NDR_BUFFERS; > DATA_BLOB variant_blob = data_blob_null; > if (offset >= rows_buf->length) { >- DBG_ERR("offset %d outside buffer range (buf len - %zu)", >+ DBG_ERR("offset %"PRIu64" outside buffer range (buf len - %zu)", > offset, > rows_buf->length); > return false; >@@ -902,7 +902,7 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx, > bool is_64bit, > struct ndr_pull *ndr_pull, > ndr_flags_type flags, >- uint32_t offset, >+ uint64_t offset, > DATA_BLOB *rows_buf, > uint64_t *pcount, > uint64_t **pvec_address/*, >@@ -1010,7 +1010,7 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx, > bool is_64bit, > struct ndr_pull *ndr_pull, > ndr_flags_type flags, >- uint32_t offset, >+ uint64_t offset, > DATA_BLOB *rows_buf, > uint32_t len, > struct wsp_cbasestoragevariant *val) >@@ -1116,7 +1116,7 @@ static enum ndr_err_code extract_crowvariant(TALLOC_CTX *ctx, > bool is_64bit, > struct ndr_pull *ndr_pull, > ndr_flags_type flags, >- uint32_t offset, >+ uint64_t offset, > DATA_BLOB *rows_buf, uint32_t len, > struct wsp_cbasestoragevariant *val) > { >-- >2.35.3 > > >From b739db32828e51b1861a76a98d0e119d4b300dc3 Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Wed, 10 Jan 2024 10:59:23 +0000 >Subject: [PATCH 2/7] s3/rpc_client: Remove stray unnecessary comment > >Signed-off-by: Noel Power <noel.power@suse.com> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >(cherry picked from commit efa60ff3105ac80ffff6d2a5d82dd0615ddb7578) >--- > source3/rpc_client/wsp_cli.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > >diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c >index 450bd2525e0..7fd221dd731 100644 >--- a/source3/rpc_client/wsp_cli.c >+++ b/source3/rpc_client/wsp_cli.c >@@ -905,8 +905,7 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx, > uint64_t offset, > DATA_BLOB *rows_buf, > uint64_t *pcount, >- uint64_t **pvec_address/*, >- struct wsp_cbasestoragevariant ***variant_array*/) >+ uint64_t **pvec_address) > { > bool is_vector = tablevar->vtype & VT_VECTOR; > uint64_t count; >-- >2.35.3 > > >From 17e3f5556e2229f44415f72f3334a02ed90cabfe Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Mon, 8 Jan 2024 15:56:38 +0000 >Subject: [PATCH 3/7] s3/utils: use full 64 bit address for getrows (with 64bit > offsets) > >if 64bit offsets are used the hi 32-bits of address are stored in >the ulreserved2 member of the message header field and the low 32-bits >are stored in the ulclientbase member of the cpmgetrows message > >Signed-off-by: Noel Power <noel.power@suse.com> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> >(cherry picked from commit 6ecb614b8ec6953ba15e8061fce9b395615b035a) >--- > source3/utils/wspsearch.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > >diff --git a/source3/utils/wspsearch.c b/source3/utils/wspsearch.c >index 2c56c97736b..063b952d468 100644 >--- a/source3/utils/wspsearch.c >+++ b/source3/utils/wspsearch.c >@@ -350,6 +350,10 @@ static NTSTATUS create_getrows(TALLOC_CTX *ctx, > uint32_t INITIAL_ROWS = 32; > uint32_t requested_rows = INITIAL_ROWS; > uint32_t rows_printed; >+ uint64_t baseaddress; >+ uint32_t offset_lowbits = 0xdeabd860; >+ uint32_t offset_hibits = 0xfeeddeaf; >+ > TALLOC_CTX *row_ctx; > bool loop_again; > >@@ -377,10 +381,24 @@ static NTSTATUS create_getrows(TALLOC_CTX *ctx, > skip, > requested_rows, > 40, >- 0xDEAbd860, >+ offset_lowbits, > bindings->brow, > 0); > >+ if (is_64bit) { >+ /* >+ * MS-WSP 2.2.2 >+ * ulreservered holds the high 32-bits part of >+ * a 64-bit offset if 64-bit offsets are being used. >+ */ >+ request->header.ulreserved2 = offset_hibits; >+ baseaddress = request->header.ulreserved2; >+ baseaddress <<= 32; >+ baseaddress += offset_lowbits; >+ } else { >+ baseaddress = offset_lowbits; >+ } >+ > status = wsp_request_response(request, > wsp_ctx, > request, >@@ -419,7 +437,7 @@ static NTSTATUS create_getrows(TALLOC_CTX *ctx, > is_64bit, > disp_all_cols, > bindings, 40, >- 0xDEAbd860, >+ baseaddress, > response->message.cpmgetrows.rowsreturned, > &rows_printed); > if (!NT_STATUS_IS_OK(status)) { >-- >2.35.3 > > >From 994f5e9e662c52707def09b61a72e9f838c4ff83 Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Wed, 10 Jan 2024 14:43:58 +0000 >Subject: [PATCH 4/7] s3/rpc_client: cleanup unmarshalling of variant types > from row columns > >Prior to this change fn 'extract_variant_addresses' actually returns offsets >to the variant stored not the addresses, additionally the param in the >signature of the method is named offset where the param in reality is a >base address. >This change makes fn 'extract_variant_addresses' actually return addresses >instead of offsets and also changes the name of the incoming param. The >resulting changes are propaged to callers which hopefully makes what the >code is actually doing a little clearer > >Signed-off-by: Noel Power <noel.power@suse.com> >Reviewed-by: Andrew Bartlett <abartlet@samba.org> > >Autobuild-User(master): Noel Power <npower@samba.org> >Autobuild-Date(master): Tue Jan 30 17:22:37 UTC 2024 on atb-devel-224 > >(cherry picked from commit 9b2f2302ee4828ae54f5903a3bf649ffd255fb4a) >--- > source3/rpc_client/wsp_cli.c | 64 ++++++++++++++++++++---------------- > 1 file changed, 36 insertions(+), 28 deletions(-) > >diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c >index 7fd221dd731..d8a9aca46ff 100644 >--- a/source3/rpc_client/wsp_cli.c >+++ b/source3/rpc_client/wsp_cli.c >@@ -894,7 +894,7 @@ static bool convert_variant_array_to_vector(TALLOC_CTX *ctx, > * an array of n elements for a vector or array of 1 element > * if non-vector item. > * >- * addresses stored in pvec_address are adjusted by offset >+ * addresses stored in pvec_address > * > */ > static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx, >@@ -902,7 +902,7 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx, > bool is_64bit, > struct ndr_pull *ndr_pull, > ndr_flags_type flags, >- uint64_t offset, >+ uint64_t baseaddress, > DATA_BLOB *rows_buf, > uint64_t *pcount, > uint64_t **pvec_address) >@@ -957,12 +957,10 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx, > addr = addr_32; > } > >- addr = addr - offset; >- >- if (addr >= rows_buf->length) { >+ if ((addr - baseaddress) >= rows_buf->length) { > DBG_ERR("offset %"PRIu64" outside buffer range " > "(buf len - %zu)\n", >- addr, >+ addr - baseaddress, > rows_buf->length); > err = NDR_ERR_VALIDATE; > goto out; >@@ -979,22 +977,27 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx, > if (is_vector == false) { > vec_address[0] = addr; > } else { >- uint64_t array_addr = addr; >+ uint64_t array_offset = addr - baseaddress; > uint64_t i; > for (i = 0; i < count; i++) { > if (is_64bit) { > vec_address[i] = > PULL_LE_I64(rows_buf->data, >- array_addr); >- array_addr = array_addr + 8; >+ array_offset); >+ array_offset = array_offset + 8; > } else { > vec_address[i] = > (uint32_t)PULL_LE_I32(rows_buf->data, >- array_addr); >- array_addr = array_addr + 4; >+ array_offset); >+ array_offset = array_offset + 4; >+ } >+ if (array_offset >= rows_buf->length) { >+ DBG_ERR("offset %"PRIu64" outside buffer range " >+ "(buf len - %zu)\n", >+ array_offset, >+ rows_buf->length); >+ err = NDR_ERR_VALIDATE; > } >- /* adjust address */ >- vec_address[i] -= offset; > } > } > err = NDR_ERR_SUCCESS; >@@ -1009,7 +1012,7 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx, > bool is_64bit, > struct ndr_pull *ndr_pull, > ndr_flags_type flags, >- uint64_t offset, >+ uint64_t baseaddress, > DATA_BLOB *rows_buf, > uint32_t len, > struct wsp_cbasestoragevariant *val) >@@ -1028,7 +1031,7 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx, > is_64bit, > ndr_pull, > flags, >- offset, >+ baseaddress, > rows_buf, > &count, > &vec_address); >@@ -1062,12 +1065,12 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx, > > for (i = 0; i < count; i++) { > uint32_t tmplen = len; >- uint64_t addr; >- addr = vec_address[i]; >- if (addr >= rows_buf->length) { >+ uint64_t buf_offset; >+ buf_offset = vec_address[i] - baseaddress; >+ if (buf_offset >= rows_buf->length) { > DBG_ERR("offset %"PRIu64" outside buffer range " > "(buf len - %zu)\n", >- addr, >+ buf_offset, > rows_buf->length); > err = NDR_ERR_VALIDATE; > goto out; >@@ -1083,11 +1086,11 @@ static enum ndr_err_code extract_crowvariant_variable(TALLOC_CTX *ctx, > * from the point the value is stored at > * till the end of the buffer > */ >- tmplen = rows_buf->length - addr; >+ tmplen = rows_buf->length - buf_offset; > } > if (!extract_rowbuf_variable_type(ctx, > tablevar->vtype & ~VT_VECTOR, >- addr, >+ buf_offset, > rows_buf, > tmplen, > variant_array[i])) { >@@ -1115,7 +1118,7 @@ static enum ndr_err_code extract_crowvariant(TALLOC_CTX *ctx, > bool is_64bit, > struct ndr_pull *ndr_pull, > ndr_flags_type flags, >- uint64_t offset, >+ uint64_t baseaddress, > DATA_BLOB *rows_buf, uint32_t len, > struct wsp_cbasestoragevariant *val) > { >@@ -1135,7 +1138,7 @@ static enum ndr_err_code extract_crowvariant(TALLOC_CTX *ctx, > is_64bit, > ndr_pull, > flags, >- offset, >+ baseaddress, > rows_buf, > len, > val); >@@ -1158,7 +1161,6 @@ out: > > static enum ndr_err_code process_columns(TALLOC_CTX *ctx, > bool is_64bit, >- uint32_t cbreserved, > uint64_t baseaddress, > struct wsp_cpmsetbindingsin *bindingin, > DATA_BLOB *rows_buf, >@@ -1224,7 +1226,6 @@ static enum ndr_err_code process_columns(TALLOC_CTX *ctx, > val_offset)); > } > if (tab_col->valueused) { >- uint64_t offset = baseaddress + cbreserved; > uint32_t len = 0; > val_offset = nrow_offset + tab_col->valueoffset.value; > if (val_offset >= rows_buf->length) { >@@ -1284,7 +1285,7 @@ static enum ndr_err_code process_columns(TALLOC_CTX *ctx, > is_64bit, > ndr_pull, > ndr_flags, >- offset, >+ baseaddress, > rows_buf, > len, > &cols[i]); >@@ -1316,13 +1317,20 @@ enum ndr_err_code extract_rowsarray( > talloc_zero_array(ctx, > struct wsp_cbasestoragevariant, > bindingsin->ccolumns); >+ uint64_t adjusted_address; > if (cols == NULL) { > return NDR_ERR_ALLOC; > } >+ >+ /* >+ * cater for paddingrows (see MS-WSP 2.2.3.12) >+ * Rows buffer starts cbreserved bytes into messages >+ */ >+ adjusted_address = baseaddress + cbreserved; >+ > err = process_columns(ctx, > is_64bit, >- cbreserved, >- baseaddress, >+ adjusted_address, > bindingsin, > rows_buf, > i, >-- >2.35.3 > > >From cff8f34d82839736bc62b2f99af3a96f4aa84848 Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Wed, 14 Feb 2024 12:01:28 +0000 >Subject: [PATCH 5/7] idl: Add constant for max rows buffer size > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15579 >Signed-off-by: Noel Power <noel.power@suse.com> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 01e901ef869a1a87fba0e67bce311dbeb199b717) >--- > librpc/idl/wsp_data.idl | 5 +++++ > 1 file changed, 5 insertions(+) > >diff --git a/librpc/idl/wsp_data.idl b/librpc/idl/wsp_data.idl >index 2a94355b0b0..fde754aef81 100644 >--- a/librpc/idl/wsp_data.idl >+++ b/librpc/idl/wsp_data.idl >@@ -11,6 +11,11 @@ interface constants > * for details of this and other language id(s) > */ > const uint32_t WSP_DEFAULT_LCID = 0x00000409; >+ /* >+ * Max size of rows buffer in getrowsout response >+ * see MS-WSP 2.2.3.11 >+ */ >+ const uint32_t MAX_ROW_BUFF_SIZE = 0x0004000; > > /* values for guidPropertySet */ > const char* DBPROPSET_FSCIFRMWRK_EXT = "A9BD1526-6A80-11D0-8C9D-0020AF1D740E"; >-- >2.35.3 > > >From 4218c85fb2de8b1546a042129d6fc7b4df48d663 Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Wed, 14 Feb 2024 11:19:39 +0000 >Subject: [PATCH 6/7] s3/rpc_client: Ensure max possible row buffer size is not > exceeded > >The max buf size of rows buffer should not exceed 0x00004000. >Ensuring this value is within limits means we can safely use >uint32_t offsets. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15579 >Signed-off-by: Noel Power <noel.power@suse.com> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit f487211706a74d516bf447ed393222b4c0dce7b0) >--- > source3/rpc_client/wsp_cli.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > >diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c >index d8a9aca46ff..39d1f7868d0 100644 >--- a/source3/rpc_client/wsp_cli.c >+++ b/source3/rpc_client/wsp_cli.c >@@ -1311,6 +1311,19 @@ enum ndr_err_code extract_rowsarray( > { > uint32_t i; > enum ndr_err_code err = NDR_ERR_SUCCESS; >+ /* >+ * limit check the size of rows_buf >+ * see MS-WSP 2.2.3.11 which describes the size >+ * of the rows buffer MUST not exceed 0x0004000 bytes. >+ * This limit will ensure we can safely check >+ * limits based on uint32_t offsets >+ */ >+ >+ if (rows_buf->length > MAX_ROW_BUFF_SIZE) { >+ DBG_ERR("Buffer size 0x%zx exceeds 0x%x max buffer size\n", >+ rows_buf->length, MAX_ROW_BUFF_SIZE); >+ return NDR_ERR_BUFSIZE; >+ } > > for (i = 0; i < rows; i++ ) { > struct wsp_cbasestoragevariant *cols = >-- >2.35.3 > > >From 4ff7950a898961354203f1d50f9693e60afdffaf Mon Sep 17 00:00:00 2001 >From: Noel Power <noel.power@suse.com> >Date: Thu, 8 Feb 2024 14:05:43 +0000 >Subject: [PATCH 7/7] s3/rpc_client: Fix array offset check > >Previous to this commit we were modifying the offset before >the array offset check. This was causing a spurious debug >message indicating the offset was out of bounds. An second >problem is that upon detecting the error we don't exit the loop. >A third problem was that when reading the offset the check >didn't cater for the size of the integer address about to be read. > >This commit moves the offset check to before the first read, >additionally when an error is detected now we actually exit the loop >and the offset have been corrected to include the size of the >integer to be read > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15579 >Signed-off-by: Noel Power <noel.power@suse.com> >Reviewed-by: Volker Lendecke <vl@samba.org> > >Autobuild-User(master): Volker Lendecke <vl@samba.org> >Autobuild-Date(master): Sat Feb 17 17:58:43 UTC 2024 on atb-devel-224 > >(cherry picked from commit 885850b6aaabf089f422b1b015481a0ccff4f90e) >--- > source3/rpc_client/wsp_cli.c | 61 ++++++++++++++++++++++++++++++------ > 1 file changed, 52 insertions(+), 9 deletions(-) > >diff --git a/source3/rpc_client/wsp_cli.c b/source3/rpc_client/wsp_cli.c >index 39d1f7868d0..15b6e36007e 100644 >--- a/source3/rpc_client/wsp_cli.c >+++ b/source3/rpc_client/wsp_cli.c >@@ -938,6 +938,15 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx, > count = 1; > } > >+ /* ensure count is at least within buffer range */ >+ if (count >= MAX_ROW_BUFF_SIZE || count >= rows_buf->length) { >+ DBG_ERR("count %"PRIu64" either exceeds max buffer size " >+ "or buffer size (%zu)", >+ count, rows_buf->length); >+ err = NDR_ERR_VALIDATE; >+ goto out; >+ } >+ > /* read address */ > if (is_64bit) { > err = ndr_pull_udlong(ndr_pull, >@@ -974,30 +983,64 @@ static enum ndr_err_code extract_variant_addresses(TALLOC_CTX *ctx, > goto out; > } > >+ /* >+ * non vector case addr points to value >+ * otherwise addr points to list of addresses >+ * for the values in vector >+ */ > if (is_vector == false) { > vec_address[0] = addr; > } else { > uint64_t array_offset = addr - baseaddress; > uint64_t i; >+ uint32_t intsize; >+ >+ if (is_64bit) { >+ intsize = 8; >+ } else { >+ intsize = 4; >+ } >+ >+ if (array_offset >= MAX_ROW_BUFF_SIZE >+ || array_offset >= rows_buf->length) { >+ DBG_ERR("offset %"PRIu64" either exceeds max buf size " >+ "or buffer size (%zu)", >+ array_offset, rows_buf->length); >+ err = NDR_ERR_VALIDATE; >+ goto out; >+ } >+ >+ /* addr points to a list of int32 or int64 addresses */ > for (i = 0; i < count; i++) { >+ /* >+ * read the addresses of the vector elements >+ * note: we can safely convert the uint64_t >+ * values here to uint32_t values as >+ * we are sure they are within range >+ * due to previous checks above. >+ */ >+ if (smb_buffer_oob((uint32_t)rows_buf->length, >+ (uint32_t)array_offset, >+ intsize)) { >+ DBG_ERR("offset %"PRIu64" will be outside " >+ "buffer range (buf len - %zu) after " >+ "reading %s address\n", >+ array_offset, >+ rows_buf->length, >+ is_64bit ? "64 bit" : "32 bit"); >+ err = NDR_ERR_VALIDATE; >+ goto out; >+ } > if (is_64bit) { > vec_address[i] = > PULL_LE_I64(rows_buf->data, > array_offset); >- array_offset = array_offset + 8; > } else { > vec_address[i] = > (uint32_t)PULL_LE_I32(rows_buf->data, > array_offset); >- array_offset = array_offset + 4; >- } >- if (array_offset >= rows_buf->length) { >- DBG_ERR("offset %"PRIu64" outside buffer range " >- "(buf len - %zu)\n", >- array_offset, >- rows_buf->length); >- err = NDR_ERR_VALIDATE; > } >+ array_offset += intsize; > } > } > err = NDR_ERR_SUCCESS; >-- >2.35.3 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=18257">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
vl
:
review+
Actions:
View
Attachments on
bug 15579
:
18256
| 18257