[PATCH] ldbndr: Include context for error strings in ndr_check_array_{size,length}() and ndr_pull_array_length()

Andrew Bartlett abartlet at samba.org
Thu May 3 00:13:13 UTC 2018


I'm trying to track down the new AD DC LSA failure.  The attached patch
may help us locate what is behind the failure and provide useful
debugging into the future.

Please review and push.

Andrew Bartlett
-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
From 0656b0edb2b680b0d26febb98d27b51cd0e319b8 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 3 May 2018 11:48:51 +1200
Subject: [PATCH] ldbndr: Include context for error strings in
 ndr_check_array_length()

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 librpc/ndr/libndr.h                      | 16 ++++++++++++---
 librpc/ndr/ndr.c                         | 35 ++++++++++++++++++++++++--------
 librpc/ndr/ndr_negoex.c                  | 24 ++++++++++++++++++----
 librpc/ndr/ndr_spoolss_buf.c             |  6 +++++-
 librpc/wscript_build                     |  2 +-
 pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm |  8 ++++----
 6 files changed, 70 insertions(+), 21 deletions(-)

diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h
index de93893be19..a33aaddc8b7 100644
--- a/librpc/ndr/libndr.h
+++ b/librpc/ndr/libndr.h
@@ -573,10 +573,20 @@ enum ndr_err_code ndr_token_retrieve(struct ndr_token_list *list, const void *ke
 uint32_t ndr_token_peek(struct ndr_token_list *list, const void *key);
 enum ndr_err_code ndr_pull_array_size(struct ndr_pull *ndr, const void *p);
 uint32_t ndr_get_array_size(struct ndr_pull *ndr, const void *p);
-enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr, void *p, uint32_t size);
-enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr, const void *p);
+enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr,
+				       void *p,
+				       uint32_t size,
+				       const char *token_name,
+				       const char *el_name);
+enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr,
+					const void *p,
+					const char *name);
 uint32_t ndr_get_array_length(struct ndr_pull *ndr, const void *p);
-enum ndr_err_code ndr_check_array_length(struct ndr_pull *ndr, void *p, uint32_t length);
+enum ndr_err_code ndr_check_array_length(struct ndr_pull *ndr,
+					 void *p,
+					 uint32_t length,
+					 const char *token_name,
+					 const char *el_name);
 enum ndr_err_code ndr_push_pipe_chunk_trailer(struct ndr_push *ndr, int ndr_flags, uint32_t count);
 enum ndr_err_code ndr_check_pipe_chunk_trailer(struct ndr_pull *ndr, int ndr_flags, uint32_t count);
 enum ndr_err_code ndr_push_set_switch_value(struct ndr_push *ndr, const void *p, uint32_t val);
diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c
index d478eb69c01..35769e43dd3 100644
--- a/librpc/ndr/ndr.c
+++ b/librpc/ndr/ndr.c
@@ -1063,14 +1063,22 @@ _PUBLIC_ uint32_t ndr_get_array_size(struct ndr_pull *ndr, const void *p)
 /*
   check the stored array size field
 */
-_PUBLIC_ enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr, void *p, uint32_t size)
+_PUBLIC_ enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr,
+						void *p,
+						uint32_t size,
+						const char *token_name,
+						const char *el_name)
 {
 	uint32_t stored;
 	stored = ndr_token_peek(&ndr->array_size_list, p);
 	if (stored != size) {
 		return ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE,
-				      "Bad array size - got %u expected %u\n",
-				      stored, size);
+				      "Bad array size in %s vs %s "
+				      "- got %u expected %u\n",
+				      token_name,
+				      el_name,
+				      stored,
+				      size);
 	}
 	return NDR_ERR_SUCCESS;
 }
@@ -1078,13 +1086,16 @@ _PUBLIC_ enum ndr_err_code ndr_check_array_size(struct ndr_pull *ndr, void *p, u
 /*
   pull an array length field and add it to the array_length_list token list
 */
-_PUBLIC_ enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr, const void *p)
+_PUBLIC_ enum ndr_err_code ndr_pull_array_length(struct ndr_pull *ndr,
+						 const void *p,
+						 const char *name)
 {
 	uint32_t length, offset;
 	NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &offset));
 	if (offset != 0) {
 		return ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE,
-				      "non-zero array offset %u\n", offset);
+				      "non-zero array offset in %s "
+				      "%u\n", name, offset);
 	}
 	NDR_CHECK(ndr_pull_uint3264(ndr, NDR_SCALARS, &length));
 	return ndr_token_store(ndr, &ndr->array_length_list, p, length);
@@ -1101,14 +1112,22 @@ _PUBLIC_ uint32_t ndr_get_array_length(struct ndr_pull *ndr, const void *p)
 /*
   check the stored array length field
 */
-_PUBLIC_ enum ndr_err_code ndr_check_array_length(struct ndr_pull *ndr, void *p, uint32_t length)
+_PUBLIC_ enum ndr_err_code ndr_check_array_length(struct ndr_pull *ndr,
+						  void *p,
+						  uint32_t length,
+						  const char *token_name,
+						  const char *el_name)
 {
 	uint32_t stored;
 	stored = ndr_token_peek(&ndr->array_length_list, p);
 	if (stored != length) {
 		return ndr_pull_error(ndr, NDR_ERR_ARRAY_SIZE,
-				      "Bad array length - got %u expected %u\n",
-				      stored, length);
+				      "Bad array length in %s vs %s"
+				      "- got %u expected %u\n",
+				      token_name,
+				      el_name,
+				      stored,
+				      length);
 	}
 	return NDR_ERR_SUCCESS;
 }
diff --git a/librpc/ndr/ndr_negoex.c b/librpc/ndr/ndr_negoex.c
index b5cb5bc8bcf..e471451c1bd 100644
--- a/librpc/ndr/ndr_negoex.c
+++ b/librpc/ndr/ndr_negoex.c
@@ -99,7 +99,11 @@ enum ndr_err_code ndr_pull_negoex_BYTE_VECTOR(struct ndr_pull *ndr, int ndr_flag
 		}
 #if 0
 		if (r->blob.data) {
-			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->blob.data, r->blob.length));
+			NDR_CHECK(ndr_check_array_size(ndr,
+						       (void*)&r->blob.data,
+						       r->blob.length,
+						       "r->blob.data",
+						       "r->blob.length")));
 		}
 #endif
 	}
@@ -179,7 +183,11 @@ enum ndr_err_code ndr_pull_negoex_AUTH_SCHEME_VECTOR(struct ndr_pull *ndr, int n
 		}
 #if 0
 		if (r->array) {
-			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->array, r->count));
+			NDR_CHECK(ndr_check_array_size(ndr,
+						       (void*)&r->array,
+						       r->count,
+						       "r->array",
+						       "r->count"));
 		}
 #endif
 	}
@@ -265,7 +273,11 @@ enum ndr_err_code ndr_pull_negoex_EXTENSION_VECTOR(struct ndr_pull *ndr, int ndr
 		}
 #if 0
 		if (r->array) {
-			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->array, r->count));
+			NDR_CHECK(ndr_check_array_size(ndr,
+						       (void*)&r->array,
+						       r->count,
+						       "r->array",
+						       "r->count"));
 		}
 #endif
 	}
@@ -351,7 +363,11 @@ enum ndr_err_code ndr_pull_negoex_ALERT_VECTOR(struct ndr_pull *ndr, int ndr_fla
 		}
 #if 0
 		if (r->array) {
-			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->array, r->count));
+			NDR_CHECK(ndr_check_array_size(ndr,
+						       (void*)&r->array,
+						       r->count,
+						       "r->array",
+						       "r->count"));
 		}
 #endif
 	}
diff --git a/librpc/ndr/ndr_spoolss_buf.c b/librpc/ndr/ndr_spoolss_buf.c
index c1d175fcbe5..f0c1a1c88d6 100644
--- a/librpc/ndr/ndr_spoolss_buf.c
+++ b/librpc/ndr/ndr_spoolss_buf.c
@@ -1087,7 +1087,11 @@ _PUBLIC_ enum ndr_err_code ndr_pull_spoolss_DriverInfo101(struct ndr_pull *ndr,
 			ndr->flags = _flags_save_string;
 		}
 		if (r->file_info) {
-			NDR_CHECK(ndr_check_array_size(ndr, (void*)&r->file_info, r->file_count));
+			NDR_CHECK(ndr_check_array_size(ndr,
+						       (void*)&r->file_info,
+						       r->file_count,
+						       "file_info",
+						       "r->file_count"));
 		}
 	}
 	return NDR_ERR_SUCCESS;
diff --git a/librpc/wscript_build b/librpc/wscript_build
index 36414fbddf4..3226c571ade 100644
--- a/librpc/wscript_build
+++ b/librpc/wscript_build
@@ -720,7 +720,7 @@ bld.SAMBA_LIBRARY('ndr',
     public_deps='samba-errors talloc samba-util util_str_hex',
     public_headers='gen_ndr/misc.h gen_ndr/ndr_misc.h ndr/libndr.h:ndr.h',
     header_path= [('*gen_ndr*', 'gen_ndr')],
-    vnum='0.1.0',
+    vnum='0.2.0',
     abi_directory='ABI',
     abi_match='ndr_* GUID_*',
     )
diff --git a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm
index cfcd29e25a7..be3041ed85b 100644
--- a/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm
+++ b/pidl/lib/Parse/Pidl/Samba4/NDR/Parser.pm
@@ -423,7 +423,7 @@ sub ParseArrayPullHeader($$$$$$)
 	}
 
 	if ($l->{IS_VARYING}) {
-		$self->pidl("NDR_CHECK(ndr_pull_array_length($ndr, " . get_pointer_to($var_name) . "));");
+		$self->pidl("NDR_CHECK(ndr_pull_array_length($ndr, " . get_pointer_to($var_name) . ", \"$var_name\"));");
 	}
 
 	my $array_size = $self->ParseArrayPullGetSize($e, $l, $ndr, $var_name, $env);
@@ -432,7 +432,7 @@ sub ParseArrayPullHeader($$$$$$)
 	if ($array_length ne $array_size) {
 		$self->pidl("if ($array_length > $array_size) {");
 		$self->indent;
-		$self->pidl("return ndr_pull_error($ndr, NDR_ERR_ARRAY_SIZE, \"Bad array size %u should exceed array length %u\", $array_size, $array_length);");
+		$self->pidl("return ndr_pull_error($ndr, NDR_ERR_ARRAY_SIZE, \"Bad array size in $var_name %u should exceed array length %u\", $array_size, $array_length);");
 		$self->deindent;
 		$self->pidl("}");
 	}
@@ -444,7 +444,7 @@ sub ParseArrayPullHeader($$$$$$)
 			check_null_pointer($e, $env, sub { $self->defer(shift); },
 					   "return ndr_pull_error($ndr, NDR_ERR_INVALID_POINTER, \"NULL Pointer for size_is()\");"),
 			check_fully_dereferenced($e, $env));
-		$self->defer("NDR_CHECK(ndr_check_array_size($ndr, (void*)" . get_pointer_to($var_name) . ", $size));");
+		$self->defer("NDR_CHECK(ndr_check_array_size($ndr, (void*)" . get_pointer_to($var_name) . ", $size, \"$var_name\", \"$size\"));");
 		$self->defer_deindent;
 		$self->defer("}");
 	}
@@ -456,7 +456,7 @@ sub ParseArrayPullHeader($$$$$$)
 			check_null_pointer($e, $env, sub { $self->defer(shift); },
 					   "return ndr_pull_error($ndr, NDR_ERR_INVALID_POINTER, \"NULL Pointer for length_is()\");"),
 			check_fully_dereferenced($e, $env));
-		$self->defer("NDR_CHECK(ndr_check_array_length($ndr, (void*)" . get_pointer_to($var_name) . ", $length));");
+		$self->defer("NDR_CHECK(ndr_check_array_length($ndr, (void*)" . get_pointer_to($var_name) . ", $length, \"$var_name\", \"$length\"));");
 		$self->defer_deindent;
 		$self->defer("}");
 	}
-- 
2.11.0



More information about the samba-technical mailing list