The Samba-Bugzilla – Attachment 15626 Details for
Bug 13876
NDR crash and memory allocation on large array values from untrustworthy servers
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch set for master
spoolss-master.patch (text/plain), 26.91 KB, created by
Andrew Bartlett
on 2019-11-21 05:04:55 UTC
(
hide
)
Description:
patch set for master
Filename:
MIME Type:
Creator:
Andrew Bartlett
Created:
2019-11-21 05:04:55 UTC
Size:
26.91 KB
patch
obsolete
>From 596b2b892bb6e1c45fbc0fe97dfd436a3737f3bf Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 18 Nov 2019 10:38:01 +1300 >Subject: [PATCH 1/7] libndr: Do not overwrite token list with NULL on > allocation failure > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > librpc/ndr/ndr.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > >diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c >index 53f9a816f94..93f47e9b1c6 100644 >--- a/librpc/ndr/ndr.c >+++ b/librpc/ndr/ndr.c >@@ -972,6 +972,7 @@ _PUBLIC_ enum ndr_err_code ndr_token_store(TALLOC_CTX *mem_ctx, > NDR_ERR_HAVE_NO_MEMORY(list->tokens); > } > } else { >+ struct ndr_token *new_tokens = NULL; > uint32_t alloc_count = talloc_array_length(list->tokens); > if (list->count == alloc_count) { > unsigned new_alloc; >@@ -980,11 +981,10 @@ _PUBLIC_ enum ndr_err_code ndr_token_store(TALLOC_CTX *mem_ctx, > if (new_alloc < alloc_count) { > return NDR_ERR_RANGE; > } >- list->tokens = talloc_realloc(mem_ctx, list->tokens, >- struct ndr_token, new_alloc); >- if (list->tokens == NULL) { >- NDR_ERR_HAVE_NO_MEMORY(list->tokens); >- } >+ new_tokens = talloc_realloc(mem_ctx, list->tokens, >+ struct ndr_token, new_alloc); >+ NDR_ERR_HAVE_NO_MEMORY(new_tokens); >+ list->tokens = new_tokens; > } > } > list->tokens[list->count].key = key; >-- >2.17.1 > > >From f98d040aedaf37e18c0f3a81ed9dfbc59586fd68 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Sat, 16 Nov 2019 07:59:58 +1300 >Subject: [PATCH 2/7] ndr: Restrict size of ndr_token lists to avoid memory > abuse by malicious clients > >This is designed to stop a very large number of tokens from being stored for >arrays of structures containing relative pointers in particular. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876 >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > librpc/ndr/ndr.c | 119 ++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 108 insertions(+), 11 deletions(-) > >diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c >index 93f47e9b1c6..0272509a01f 100644 >--- a/librpc/ndr/ndr.c >+++ b/librpc/ndr/ndr.c >@@ -36,6 +36,17 @@ > > #define NDR_BASE_MARSHALL_SIZE 1024 > >+/* >+ * This value is arbitary, but designed to reduce the memory a client >+ * can allocate and the work the client can force in processing a >+ * malicious packet. >+ * >+ * In an ideal world this would be controlled by range() restrictions >+ * on array sizes and careful IDL construction to avoid arbitary >+ * linked lists, but this is a backstop for now. >+ */ >+#define NDR_TOKEN_MAX_LIST_SIZE 65535 >+ > /* this guid indicates NDR encoding in a protocol tower */ > const struct ndr_syntax_id ndr_transfer_syntax_ndr = { > { 0x8a885d04, 0x1ceb, 0x11c9, {0x9f, 0xe8}, {0x08,0x00,0x2b,0x10,0x48,0x60} }, >@@ -974,8 +985,21 @@ _PUBLIC_ enum ndr_err_code ndr_token_store(TALLOC_CTX *mem_ctx, > } else { > struct ndr_token *new_tokens = NULL; > uint32_t alloc_count = talloc_array_length(list->tokens); >+ >+ /* >+ * Check every time we have not allocated too many >+ * tokens. This ensures developer sanity when >+ * debugging the boundary condition >+ */ >+ if (list->count >= NDR_TOKEN_MAX_LIST_SIZE) { >+ return NDR_ERR_RANGE; >+ } > if (list->count == alloc_count) { > unsigned new_alloc; >+ /* >+ * Double the list, until we start in chunks >+ * of 1000 >+ */ > unsigned increment = MIN(list->count, 1000); > new_alloc = alloc_count + increment; > if (new_alloc < alloc_count) { >@@ -1059,9 +1083,16 @@ _PUBLIC_ uint32_t ndr_token_peek(struct ndr_token_list *list, const void *key) > */ > _PUBLIC_ enum ndr_err_code ndr_pull_array_size(struct ndr_pull *ndr, const void *p) > { >+ enum ndr_err_code ret; > uint32_t size; > NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &size)); >- return ndr_token_store(ndr, &ndr->array_size_list, p, size); >+ ret = ndr_token_store(ndr, &ndr->array_size_list, p, size); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_pull_error(ndr, ret, >+ "More than %d NDR tokens stored for array_size", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ return ret; > } > > /* >@@ -1092,6 +1123,7 @@ _PUBLIC_ enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr, void *p, u > */ > _PUBLIC_ enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr, const void *p) > { >+ enum ndr_err_code ret; > uint32_t length, offset; > NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &offset)); > if (offset != 0) { >@@ -1099,7 +1131,13 @@ _PUBLIC_ enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr, const voi > "non-zero array offset %u\n", offset); > } > NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &length)); >- return ndr_token_store(ndr, &ndr->array_length_list, p, length); >+ ret = ndr_token_store(ndr, &ndr->array_length_list, p, length); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_pull_error(ndr, ret, >+ "More than %d NDR tokens stored for array_length_list", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ return ret; > } > > /* >@@ -1164,12 +1202,27 @@ _PUBLIC_ enum ndr_err_code ndr_check_pipe_chunk_trailer(struct ndr_pull *ndr, in > */ > _PUBLIC_ enum ndr_err_code ndr_push_set_switch_value(struct ndr_push *ndr, const void *p, uint32_t val) > { >- return ndr_token_store(ndr, &ndr->switch_list, p, val); >+ enum ndr_err_code ret = >+ ndr_token_store(ndr, &ndr->switch_list, p, val); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_push_error(ndr, ret, >+ "More than %d NDR tokens stored for switch_list", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ return ret; > } > > _PUBLIC_ enum ndr_err_code ndr_pull_set_switch_value(struct ndr_pull *ndr, const void *p, uint32_t val) > { >- return ndr_token_store(ndr, &ndr->switch_list, p, val); >+ >+ enum ndr_err_code ret = >+ ndr_token_store(ndr, &ndr->switch_list, p, val); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_pull_error(ndr, ret, >+ "More than %d NDR tokens stored for switch_list", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ return ret; > } > > _PUBLIC_ enum ndr_err_code ndr_print_set_switch_value(struct ndr_print *ndr, const void *p, uint32_t val) >@@ -1482,8 +1535,15 @@ _PUBLIC_ void ndr_push_restore_relative_base_offset(struct ndr_push *ndr, uint32 > */ > _PUBLIC_ enum ndr_err_code ndr_push_setup_relative_base_offset1(struct ndr_push *ndr, const void *p, uint32_t offset) > { >+ enum ndr_err_code ret; > ndr->relative_base_offset = offset; >- return ndr_token_store(ndr, &ndr->relative_base_list, p, offset); >+ ret = ndr_token_store(ndr, &ndr->relative_base_list, p, offset); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_push_error(ndr, ret, >+ "More than %d NDR tokens stored for relative_base_list", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ return ret; > } > > /* >@@ -1501,12 +1561,19 @@ _PUBLIC_ enum ndr_err_code ndr_push_setup_relative_base_offset2(struct ndr_push > */ > _PUBLIC_ enum ndr_err_code ndr_push_relative_ptr1(struct ndr_push *ndr, const void *p) > { >+ enum ndr_err_code ret; > if (p == NULL) { > NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, 0)); > return NDR_ERR_SUCCESS; > } > NDR_CHECK(ndr_push_align(ndr, 4)); >- NDR_CHECK(ndr_token_store(ndr, &ndr->relative_list, p, ndr->offset)); >+ ret = ndr_token_store(ndr, &ndr->relative_list, p, ndr->offset); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_push_error(ndr, ret, >+ "More than %d NDR tokens stored for relative_list", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ NDR_CHECK(ret); > return ndr_push_uint32(ndr, NDR_SCALARS, 0xFFFFFFFF); > } > >@@ -1516,12 +1583,19 @@ _PUBLIC_ enum ndr_err_code ndr_push_relative_ptr1(struct ndr_push *ndr, const vo > */ > _PUBLIC_ enum ndr_err_code ndr_push_short_relative_ptr1(struct ndr_push *ndr, const void *p) > { >+ enum ndr_err_code ret; > if (p == NULL) { > NDR_CHECK(ndr_push_uint16(ndr, NDR_SCALARS, 0)); > return NDR_ERR_SUCCESS; > } > NDR_CHECK(ndr_push_align(ndr, 2)); >- NDR_CHECK(ndr_token_store(ndr, &ndr->relative_list, p, ndr->offset)); >+ ret = ndr_token_store(ndr, &ndr->relative_list, p, ndr->offset); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_push_error(ndr, ret, >+ "More than %d NDR tokens stored for relative_list", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ NDR_CHECK(ret); > return ndr_push_uint16(ndr, NDR_SCALARS, 0xFFFF); > } > /* >@@ -1617,6 +1691,7 @@ _PUBLIC_ enum ndr_err_code ndr_push_short_relative_ptr2(struct ndr_push *ndr, co > */ > _PUBLIC_ enum ndr_err_code ndr_push_relative_ptr2_start(struct ndr_push *ndr, const void *p) > { >+ enum ndr_err_code ret; > if (p == NULL) { > return NDR_ERR_SUCCESS; > } >@@ -1655,8 +1730,16 @@ _PUBLIC_ enum ndr_err_code ndr_push_relative_ptr2_start(struct ndr_push *ndr, co > "ndr_push_relative_ptr2_start RELATIVE_REVERSE flag set and relative_end_offset %d", > ndr->relative_end_offset); > } >- NDR_CHECK(ndr_token_store(ndr, &ndr->relative_begin_list, p, ndr->offset)); >- return NDR_ERR_SUCCESS; >+ ret = ndr_token_store(ndr, >+ &ndr->relative_begin_list, >+ p, >+ ndr->offset); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_push_error(ndr, ret, >+ "More than %d NDR tokens stored for array_size", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ return ret; > } > > /* >@@ -1786,8 +1869,15 @@ _PUBLIC_ void ndr_pull_restore_relative_base_offset(struct ndr_pull *ndr, uint32 > */ > _PUBLIC_ enum ndr_err_code ndr_pull_setup_relative_base_offset1(struct ndr_pull *ndr, const void *p, uint32_t offset) > { >+ enum ndr_err_code ret; > ndr->relative_base_offset = offset; >- return ndr_token_store(ndr, &ndr->relative_base_list, p, offset); >+ ret = ndr_token_store(ndr, &ndr->relative_base_list, p, offset); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_pull_error(ndr, ret, >+ "More than %d NDR tokens stored for relative_base_list", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ return ret; > } > > /* >@@ -1805,13 +1895,20 @@ _PUBLIC_ enum ndr_err_code ndr_pull_setup_relative_base_offset2(struct ndr_pull > */ > _PUBLIC_ enum ndr_err_code ndr_pull_relative_ptr1(struct ndr_pull *ndr, const void *p, uint32_t rel_offset) > { >+ enum ndr_err_code ret; > rel_offset += ndr->relative_base_offset; > if (rel_offset > ndr->data_size) { > return ndr_pull_error(ndr, NDR_ERR_BUFSIZE, > "ndr_pull_relative_ptr1 rel_offset(%u) > ndr->data_size(%u)", > rel_offset, ndr->data_size); > } >- return ndr_token_store(ndr, &ndr->relative_list, p, rel_offset); >+ ret = ndr_token_store(ndr, &ndr->relative_list, p, rel_offset); >+ if (ret == NDR_ERR_RANGE) { >+ return ndr_pull_error(ndr, ret, >+ "More than %d NDR tokens stored for relative_list", >+ NDR_TOKEN_MAX_LIST_SIZE); >+ } >+ return ret; > } > > /* >-- >2.17.1 > > >From fe9e9f3f8896d937e32205481c8f8487ff57a163 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Sat, 16 Nov 2019 09:37:30 +1300 >Subject: [PATCH 3/7] s4-libcli/rap: Set the switch_value before NDR_BUFFERS to > prepare for new libndr behaviour > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > source4/libcli/rap/rap.c | 3 +++ > 1 file changed, 3 insertions(+) > >diff --git a/source4/libcli/rap/rap.c b/source4/libcli/rap/rap.c >index 9f41e17d278..1b2cadb753d 100644 >--- a/source4/libcli/rap/rap.c >+++ b/source4/libcli/rap/rap.c >@@ -476,6 +476,7 @@ static enum ndr_err_code ndr_pull_rap_NetPrintQEnum_data(struct ndr_pull *ndr, s > NDR_CHECK(ndr_pull_rap_printq_info(ndr, NDR_SCALARS, &r->out.info[cntr_info_0])); > } > for (cntr_info_0 = 0; cntr_info_0 < r->out.count; cntr_info_0++) { >+ NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->out.info[cntr_info_0], r->in.level)); > NDR_CHECK(ndr_pull_rap_printq_info(ndr, NDR_BUFFERS, &r->out.info[cntr_info_0])); > } > NDR_PULL_SET_MEM_CTX(ndr, _mem_save_info_0, 0); >@@ -864,6 +865,7 @@ static enum ndr_err_code ndr_pull_rap_NetPrintJobEnum_data(struct ndr_pull *ndr, > NDR_CHECK(ndr_pull_rap_printj_info(ndr, NDR_SCALARS, &r->out.info[cntr_info_0])); > } > for (cntr_info_0 = 0; cntr_info_0 < r->out.count; cntr_info_0++) { >+ NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->out.info[cntr_info_0], r->in.level)); > NDR_CHECK(ndr_pull_rap_printj_info(ndr, NDR_BUFFERS, &r->out.info[cntr_info_0])); > } > NDR_PULL_SET_MEM_CTX(ndr, _mem_save_info_0, 0); >@@ -1413,6 +1415,7 @@ static enum ndr_err_code ndr_pull_rap_NetSessionEnum_data(struct ndr_pull *ndr, > NDR_CHECK(ndr_pull_rap_session_info(ndr, NDR_SCALARS, &r->out.info[cntr_info_0])); > } > for (cntr_info_0 = 0; cntr_info_0 < r->out.count; cntr_info_0++) { >+ NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->out.info[cntr_info_0], r->in.level)); > NDR_CHECK(ndr_pull_rap_session_info(ndr, NDR_BUFFERS, &r->out.info[cntr_info_0])); > } > NDR_PULL_SET_MEM_CTX(ndr, _mem_save_info_0, 0); >-- >2.17.1 > > >From 683799794fbaf5861fa9c29db346d9b9d244e741 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Sun, 17 Nov 2019 19:32:50 +1300 >Subject: [PATCH 4/7] negoex: Set the switch_value before NDR_BUFFERS to > prepare for new libndr behaviour > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > librpc/ndr/ndr_negoex.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/librpc/ndr/ndr_negoex.c b/librpc/ndr/ndr_negoex.c >index b5cb5bc8bcf..95adce5a7e3 100644 >--- a/librpc/ndr/ndr_negoex.c >+++ b/librpc/ndr/ndr_negoex.c >@@ -439,6 +439,7 @@ enum ndr_err_code ndr_pull_negoex_MESSAGE(struct ndr_pull *ndr, int ndr_flags, s > NDR_PULL_NEED_BYTES(ndr, r->message_length); > ndr->data_size = ndr->offset + r->message_length; > ndr->offset = saved_offset; >+ NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->p, r->type)); > NDR_CHECK(ndr_pull_negoex_PAYLOAD(ndr, NDR_BUFFERS, &r->p)); > ndr->offset = ndr->data_size; > ndr->data_size = start_data_size; >-- >2.17.1 > > >From 9d44443bc299a47c7afd82f5ec4273618264f5e4 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Fri, 15 Nov 2019 16:59:12 +1300 >Subject: [PATCH 5/7] pidl: Generate and consume the switch level token for > both NDR_SCALARS and NDR_BUFFERS > >This means what was previously a list becomes a single variable that >could be passed as a function paraemter, but this is avoided for now >because it would change the ABI and be more intrusive. > >Before this, a client could cause a NDR token containing the swith level >to be allocated for each and every element in the array that they >promised they were sending (without having to actually send them). > >Found by Michael Hanselmann using Honggfuzz and an fuzzer for Samba's >NDR layer. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 37 ++++++++++++++++-------- > 1 file changed, 25 insertions(+), 12 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index c1a2cc99cb7..636bfb05b32 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -647,8 +647,6 @@ sub ParseElementPushLevel > $self->pidl("NDR_CHECK(ndr_push_array_$nl->{DATA_TYPE}($ndr, $ndr_flags, $var_name, $length));"); > return; > } >- } elsif ($l->{TYPE} eq "SWITCH") { >- $self->ParseSwitchPush($e, $l, $ndr, $var_name, $env); > } elsif ($l->{TYPE} eq "DATA") { > $self->ParseDataPush($e, $l, $ndr, $var_name, $primitives, $deferred); > } elsif ($l->{TYPE} eq "TYPEDEF") { >@@ -709,6 +707,14 @@ sub ParseElementPushLevel > $self->pidl("}"); > } > } elsif ($l->{TYPE} eq "SWITCH") { >+ my $nl = GetNextLevel($e,$l); >+ my $needs_deferred_switch = is_deferred_switch_non_empty($nl); >+ >+ # Avoid setting a switch value if it will not be >+ # consumed again in the NDR_BUFFERS pull >+ if ($needs_deferred_switch or !$deferred) { >+ $self->ParseSwitchPush($e, $l, $ndr, $var_name, $env); >+ } > $self->ParseElementPushLevel($e, GetNextLevel($e, $l), $ndr, $var_name, $env, $primitives, $deferred); > } > } >@@ -1174,8 +1180,6 @@ sub ParseElementPullLevel > } > } elsif ($l->{TYPE} eq "POINTER") { > $self->ParsePtrPull($e, $l, $ndr, $var_name); >- } elsif ($l->{TYPE} eq "SWITCH") { >- $self->ParseSwitchPull($e, $l, $ndr, $var_name, $env); > } elsif ($l->{TYPE} eq "DATA") { > $self->ParseDataPull($e, $l, $ndr, $var_name, $primitives, $deferred); > } elsif ($l->{TYPE} eq "TYPEDEF") { >@@ -1260,7 +1264,15 @@ sub ParseElementPullLevel > $self->ParseMemCtxPullEnd($e, $l, $ndr); > > } elsif ($l->{TYPE} eq "SWITCH") { >- $self->ParseElementPullLevel($e, GetNextLevel($e,$l), $ndr, $var_name, $env, $primitives, $deferred); >+ my $nl = GetNextLevel($e,$l); >+ my $needs_deferred_switch = is_deferred_switch_non_empty($nl); >+ >+ # Avoid setting a switch value if it will not be >+ # consumed again in the NDR_BUFFERS pull >+ if ($needs_deferred_switch or !$deferred) { >+ $self->ParseSwitchPull($e, $l, $ndr, $var_name, $env); >+ } >+ $self->ParseElementPullLevel($e, $nl, $ndr, $var_name, $env, $primitives, $deferred); > } > } > >@@ -2156,20 +2168,21 @@ sub ParseUnionPull($$$$) > $self->pidl("NDR_PULL_CHECK_FLAGS(ndr, ndr_flags);"); > $self->pidl("if (ndr_flags & NDR_SCALARS) {"); > $self->indent; >- if (! $needs_deferred_switch) { >- $self->pidl("/* This token is not used again */"); >- $self->pidl("level = ndr_pull_steal_switch_value($ndr, $varname);"); >- } else { >- $self->pidl("level = ndr_pull_get_switch_value($ndr, $varname);"); >- } >+ $self->pidl("/* This token is not used again (except perhaps below in the NDR_BUFFERS case) */"); >+ $self->pidl("level = ndr_pull_steal_switch_value($ndr, $varname);"); > $self->ParseUnionPullPrimitives($e,$ndr,$varname,$switch_type); > $self->deindent; > $self->pidl("}"); > if ($needs_deferred_switch) { > $self->pidl("if (ndr_flags & NDR_BUFFERS) {"); > $self->indent; >- $self->pidl("/* The token is not needed after this. */"); >+ # In case we had ndr_flags of NDR_SCALERS|NDR_BUFFERS >+ $self->pidl("if (!(ndr_flags & NDR_SCALARS)) {"); >+ $self->indent; >+ $self->pidl("/* We didn't get it above, and the token is not needed after this. */"); > $self->pidl("level = ndr_pull_steal_switch_value($ndr, $varname);"); >+ $self->deindent; >+ $self->pidl("}"); > $self->ParseUnionPullDeferred($e,$ndr,$varname); > $self->deindent; > $self->pidl("}"); >-- >2.17.1 > > >From cfdbd4041623a7accd3f8f6e9894b6748551c498 Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Mon, 18 Nov 2019 12:02:03 +1300 >Subject: [PATCH 6/7] pidl: Add and use ndr_print_steal_switch_value(), > removing ndr_print_get_switch_value() > >This avoids really long token lists for switch values >that will not be needed past this point. > >The function name is changed to clarify what exactly is being >done here, and the old function is removed to ensure it is >not being used anywhere else. > >Merge the removal of ndr_print_get_switch_value into >just-tagged librpc/ABI/ndr-1.0.0.sigs as this >has not been put into any release yet. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13876 > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > librpc/ABI/ndr-1.0.0.sigs | 2 +- > librpc/ndr/libndr.h | 2 +- > librpc/ndr/ndr.c | 13 +++++++++++-- > librpc/ndr/ndr_drsuapi.c | 2 +- > librpc/ndr/ndr_ntlmssp.c | 2 +- > librpc/ndr/ndr_schannel.c | 4 ++-- > librpc/ndr/ndr_spoolss_buf.c | 2 +- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 2 +- > 8 files changed, 19 insertions(+), 10 deletions(-) > >diff --git a/librpc/ABI/ndr-1.0.0.sigs b/librpc/ABI/ndr-1.0.0.sigs >index ffb3d0763fc..7546f63ed8e 100644 >--- a/librpc/ABI/ndr-1.0.0.sigs >+++ b/librpc/ABI/ndr-1.0.0.sigs >@@ -49,7 +49,6 @@ ndr_print_double: void (struct ndr_print *, const char *, double) > ndr_print_enum: void (struct ndr_print *, const char *, const char *, const char *, uint32_t) > ndr_print_function_debug: void (ndr_print_function_t, const char *, int, void *) > ndr_print_function_string: char *(TALLOC_CTX *, ndr_print_function_t, const char *, int, void *) >-ndr_print_get_switch_value: uint32_t (struct ndr_print *, const void *) > ndr_print_gid_t: void (struct ndr_print *, const char *, gid_t) > ndr_print_hyper: void (struct ndr_print *, const char *, uint64_t) > ndr_print_int16: void (struct ndr_print *, const char *, int16_t) >@@ -68,6 +67,7 @@ ndr_print_printf_helper: void (struct ndr_print *, const char *, ...) > ndr_print_ptr: void (struct ndr_print *, const char *, const void *) > ndr_print_set_switch_value: enum ndr_err_code (struct ndr_print *, const void *, uint32_t) > ndr_print_sockaddr_storage: void (struct ndr_print *, const char *, const struct sockaddr_storage *) >+ndr_print_steal_switch_value: uint32_t (struct ndr_print *, const void *) > ndr_print_string: void (struct ndr_print *, const char *, const char *) > ndr_print_string_array: void (struct ndr_print *, const char *, const char **) > ndr_print_string_helper: void (struct ndr_print *, const char *, ...) >diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h >index 392ab76cc94..128b363ddf3 100644 >--- a/librpc/ndr/libndr.h >+++ b/librpc/ndr/libndr.h >@@ -618,7 +618,7 @@ enum ndr_err_code ndr_pull_set_switch_value(struct ndr_pull *ndr, const void *p, > enum ndr_err_code ndr_print_set_switch_value(struct ndr_print *ndr, const void *p, uint32_t val); > uint32_t ndr_push_get_switch_value(struct ndr_push *ndr, const void *p); > uint32_t ndr_pull_get_switch_value(struct ndr_pull *ndr, const void *p); >-uint32_t ndr_print_get_switch_value(struct ndr_print *ndr, const void *p); >+uint32_t ndr_print_steal_switch_value(struct ndr_print *ndr, const void *p); > uint32_t ndr_pull_steal_switch_value(struct ndr_pull *ndr, const void *p); > enum ndr_err_code ndr_pull_struct_blob(const DATA_BLOB *blob, TALLOC_CTX *mem_ctx, void *p, ndr_pull_flags_fn_t fn); > enum ndr_err_code ndr_pull_struct_blob_all(const DATA_BLOB *blob, TALLOC_CTX *mem_ctx, void *p, ndr_pull_flags_fn_t fn); >diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c >index 0272509a01f..1f5df4e2b27 100644 >--- a/librpc/ndr/ndr.c >+++ b/librpc/ndr/ndr.c >@@ -1243,9 +1243,18 @@ _PUBLIC_ uint32_t ndr_pull_get_switch_value(struct ndr_pull *ndr, const void *p) > return ndr_token_peek(&ndr->switch_list, p); > } > >-_PUBLIC_ uint32_t ndr_print_get_switch_value(struct ndr_print *ndr, const void *p) >+/* retrieve a switch value and remove it from the list */ >+_PUBLIC_ uint32_t ndr_print_steal_switch_value(struct ndr_print *ndr, const void *p) > { >- return ndr_token_peek(&ndr->switch_list, p); >+ enum ndr_err_code status; >+ uint32_t v; >+ >+ status = ndr_token_retrieve(&ndr->switch_list, p, &v); >+ if (!NDR_ERR_CODE_IS_SUCCESS(status)) { >+ return 0; >+ } >+ >+ return v; > } > > /* retrieve a switch value and remove it from the list */ >diff --git a/librpc/ndr/ndr_drsuapi.c b/librpc/ndr/ndr_drsuapi.c >index 45d3ac095c3..bcfd5145ba2 100644 >--- a/librpc/ndr/ndr_drsuapi.c >+++ b/librpc/ndr/ndr_drsuapi.c >@@ -571,7 +571,7 @@ enum ndr_err_code ndr_pull_drsuapi_DsBindInfo(struct ndr_pull *ndr, int ndr_flag > _PUBLIC_ void ndr_print_drsuapi_DsBindInfo(struct ndr_print *ndr, const char *name, const union drsuapi_DsBindInfo *r) > { > uint32_t level; >- level = ndr_print_get_switch_value(ndr, r); >+ level = ndr_print_steal_switch_value(ndr, r); > ndr_print_union(ndr, name, level, "drsuapi_DsBindInfo"); > switch (level) { > case 24: >diff --git a/librpc/ndr/ndr_ntlmssp.c b/librpc/ndr/ndr_ntlmssp.c >index 7027ac0b13d..021bc402a80 100644 >--- a/librpc/ndr/ndr_ntlmssp.c >+++ b/librpc/ndr/ndr_ntlmssp.c >@@ -164,7 +164,7 @@ _PUBLIC_ void ndr_print_ntlmssp_lm_response(TALLOC_CTX *mem_ctx, > _PUBLIC_ void ndr_print_ntlmssp_Version(struct ndr_print *ndr, const char *name, const union ntlmssp_Version *r) > { > int level; >- level = ndr_print_get_switch_value(ndr, r); >+ level = ndr_print_steal_switch_value(ndr, r); > switch (level) { > case NTLMSSP_NEGOTIATE_VERSION: > ndr_print_ntlmssp_VERSION(ndr, name, &r->version); >diff --git a/librpc/ndr/ndr_schannel.c b/librpc/ndr/ndr_schannel.c >index 9bbc628a294..6b08a79cab2 100644 >--- a/librpc/ndr/ndr_schannel.c >+++ b/librpc/ndr/ndr_schannel.c >@@ -27,7 +27,7 @@ > _PUBLIC_ void ndr_print_NL_AUTH_MESSAGE_BUFFER(struct ndr_print *ndr, const char *name, const union NL_AUTH_MESSAGE_BUFFER *r) > { > int level; >- level = ndr_print_get_switch_value(ndr, r); >+ level = ndr_print_steal_switch_value(ndr, r); > switch (level) { > case NL_FLAG_OEM_NETBIOS_DOMAIN_NAME: > ndr_print_string(ndr, name, r->a); >@@ -58,7 +58,7 @@ _PUBLIC_ void ndr_print_NL_AUTH_MESSAGE_BUFFER(struct ndr_print *ndr, const char > _PUBLIC_ void ndr_print_NL_AUTH_MESSAGE_BUFFER_REPLY(struct ndr_print *ndr, const char *name, const union NL_AUTH_MESSAGE_BUFFER_REPLY *r) > { > int level; >- level = ndr_print_get_switch_value(ndr, r); >+ level = ndr_print_steal_switch_value(ndr, r); > switch (level) { > case NL_NEGOTIATE_RESPONSE: > ndr_print_uint32(ndr, name, r->dummy); >diff --git a/librpc/ndr/ndr_spoolss_buf.c b/librpc/ndr/ndr_spoolss_buf.c >index 244d692dca8..a57f578fd45 100644 >--- a/librpc/ndr/ndr_spoolss_buf.c >+++ b/librpc/ndr/ndr_spoolss_buf.c >@@ -1096,7 +1096,7 @@ _PUBLIC_ enum ndr_err_code ndr_pull_spoolss_DriverInfo101(struct ndr_pull *ndr, > void ndr_print_spoolss_Field(struct ndr_print *ndr, const char *name, const union spoolss_Field *r) > { > int level; >- level = ndr_print_get_switch_value(ndr, r); >+ level = ndr_print_steal_switch_value(ndr, r); > ndr_print_union(ndr, name, level, "spoolss_Field"); > switch (level) { > case PRINTER_NOTIFY_TYPE: >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index 636bfb05b32..fd872cf077f 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -2017,7 +2017,7 @@ sub ParseUnionPrint($$$$$) > > $self->start_flags($e, $ndr); > >- $self->pidl("level = ndr_print_get_switch_value($ndr, $varname);"); >+ $self->pidl("level = ndr_print_steal_switch_value($ndr, $varname);"); > > $self->pidl("ndr_print_union($ndr, name, level, \"$name\");"); > >-- >2.17.1 > > >From 0da2e06b90af269bef809e20d4956fc6b8c0075b Mon Sep 17 00:00:00 2001 >From: Andrew Bartlett <abartlet@samba.org> >Date: Fri, 15 Nov 2019 20:04:41 +1300 >Subject: [PATCH 7/7] pidl: Mismatch between set and get of relative base > pointers > >The set was within the switch, the get was before the switch. > >The difference is shown when there is an empty default element. > >Signed-off-by: Andrew Bartlett <abartlet@samba.org> >--- > pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > >diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >index fd872cf077f..d2d3d674903 100644 >--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm >@@ -2107,11 +2107,6 @@ sub ParseUnionPullDeferred($$$$) > my ($self,$e,$ndr,$varname) = @_; > my $have_default = 0; > >- if (defined($e->{PROPERTIES}{relative_base})) { >- # retrieve the current offset as base for relative pointers >- # based on the toplevel struct/union >- $self->pidl("NDR_CHECK(ndr_pull_setup_relative_base_offset2($ndr, $varname));"); >- } > $self->pidl("switch (level) {"); > $self->indent; > foreach my $el (@{$e->{ELEMENTS}}) { >@@ -2122,6 +2117,11 @@ sub ParseUnionPullDeferred($$$$) > $self->pidl("$el->{CASE}:"); > if ($el->{TYPE} ne "EMPTY") { > $self->indent; >+ if (defined($e->{PROPERTIES}{relative_base})) { >+ # retrieve the current offset as base for relative pointers >+ # based on the toplevel struct/union >+ $self->pidl("NDR_CHECK(ndr_pull_setup_relative_base_offset2($ndr, $varname));"); >+ } > $self->ParseElementPull($el, $ndr, {$el->{NAME} => "$varname->$el->{NAME}"}, 0, 1); > $self->deindent; > } >-- >2.17.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=15626">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 13876
:
15621
|
15622
|
15623
|
15626