s3-lib Remove the clobber_region() code.

This code wrote to the full buffer in fstrcpy(), pstrcpy() and other
fixed-length string manipulation functions.

The hope of this code was to find out at run time if we were mixing up
pstring and fstring etc, and to record where this came from.  It has a
runtime performance impact (particularly if compiled with
--enable-developer).

It is being removed because of the complexity it adds, and the
distinct lack of bugs that this complexity has been credited in
finding.

The macro-based compile-time checking of string sizes remains.

Andrew Bartlett

2 files changed, 38 insertions, 88 deletions

diff --git a/source3/include/proto.h b/source3/include/proto.h
index bb9241c893..d806b36dd2 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -449,25 +449,19 @@ bool pull_ucs2_talloc(TALLOC_CTX *ctx, char **dest, const smb_ucs2_t *src,
 		      size_t *converted_size);
 bool pull_ascii_talloc(TALLOC_CTX *ctx, char **dest, const char *src,
 		       size_t *converted_size);
-size_t push_string_check_fn(const char *function, unsigned int line,
-			    void *dest, const char *src,
+size_t push_string_check_fn(void *dest, const char *src,
 			    size_t dest_len, int flags);
-size_t push_string_base(const char *function, unsigned int line,
-			const char *base, uint16 flags2, 
+size_t push_string_base(const char *base, uint16 flags2,
 			void *dest, const char *src,
 			size_t dest_len, int flags);
-size_t pull_string_fn(const char *function,
-			unsigned int line,
-			const void *base_ptr,
+size_t pull_string_fn(const void *base_ptr,
 			uint16 smb_flags2,
 			char *dest,
 			const void *src,
 			size_t dest_len,
 			size_t src_len,
 			int flags);
-size_t pull_string_talloc_fn(const char *function,
-			unsigned int line,
-			TALLOC_CTX *ctx,
+size_t pull_string_talloc_fn(TALLOC_CTX *ctx,
 			const void *base_ptr,
 			uint16 smb_flags2,
 			char **ppdest,
@@ -479,10 +473,6 @@ codepoint_t next_codepoint_ext(const char *str, charset_t src_charset,
 			       size_t *bytes_consumed);
 codepoint_t next_codepoint(const char *str, size_t *size);
 
-/* The following definitions come from lib/clobber.c  */
-
-void clobber_region(const char *fn, unsigned int line, char *dest, size_t len);
-
 /* The following definitions come from lib/conn_tdb.c  */
 
 struct db_record *connections_fetch_entry(TALLOC_CTX *mem_ctx,
@@ -1356,23 +1346,17 @@ size_t str_ascii_charnum(const char *s);
 bool trim_char(char *s,char cfront,char cback);
 bool strhasupper(const char *s);
 bool strhaslower(const char *s);
-char *safe_strcpy_fn(const char *fn,
-		int line,
-		char *dest,
+char *safe_strcpy_fn(char *dest,
 		const char *src,
 		size_t maxlength);
-char *safe_strcat_fn(const char *fn,
-		int line,
-		char *dest,
+char *safe_strcat_fn(char *dest,
 		const char *src,
 		size_t maxlength);
-char *alpha_strcpy_fn(const char *fn,
-		int line,
-		char *dest,
+char *alpha_strcpy_fn(char *dest,
 		const char *src,
 		const char *other_safe_chars,
 		size_t maxlength);
-char *StrnCpy_fn(const char *fn, int line,char *dest,const char *src,size_t n);
+char *StrnCpy_fn(char *dest,const char *src,size_t n);
 bool in_list(const char *s, const char *list, bool casesensitive);
 void string_free(char **s);
 bool string_set(char **dest,const char *src);
@@ -2333,24 +2317,18 @@ bool spnego_mech_list_blob(TALLOC_CTX *mem_ctx,
 
 /* The following definitions come from libsmb/clistr.c  */
 
-size_t clistr_push_fn(const char *function,
-			unsigned int line,
-			struct cli_state *cli,
+size_t clistr_push_fn(struct cli_state *cli,
 			void *dest,
 			const char *src,
 			int dest_len,
 			int flags);
-size_t clistr_pull_fn(const char *function,
-			unsigned int line,
-			const char *inbuf,
+size_t clistr_pull_fn(const char *inbuf,
 			char *dest,
 			const void *src,
 			int dest_len,
 			int src_len,
 			int flags);
-size_t clistr_pull_talloc_fn(const char *function,
-				unsigned int line,
-				TALLOC_CTX *ctx,
+size_t clistr_pull_talloc_fn(TALLOC_CTX *ctx,
 				const char *base,
 				uint16_t flags2,
 				char **pp_dest,
@@ -5024,8 +5002,7 @@ bool is_share_read_only_for_token(const char *username,
 
 /* The following definitions come from smbd/srvstr.c  */
 
-size_t srvstr_push_fn(const char *function, unsigned int line,
-		      const char *base_ptr, uint16 smb_flags2, void *dest,
+size_t srvstr_push_fn(const char *base_ptr, uint16 smb_flags2, void *dest,
 		      const char *src, int dest_len, int flags);
 ssize_t message_push_string(uint8 **outbuf, const char *str, int flags);
 
diff --git a/source3/include/safe_string.h b/source3/include/safe_string.h
index 56e3b67aa6..f54464e9c0 100644
--- a/source3/include/safe_string.h
+++ b/source3/include/safe_string.h
@@ -62,14 +62,6 @@
 
 #endif /* !_SPLINT_ */
 
-#ifdef DEVELOPER
-#define SAFE_STRING_FUNCTION_NAME __FUNCTION__
-#define SAFE_STRING_LINE __LINE__
-#else
-#define SAFE_STRING_FUNCTION_NAME ("")
-#define SAFE_STRING_LINE (0)
-#endif
-
 /* We need a number of different prototypes for our 
    non-existant fuctions */
 char * __unsafe_string_function_usage_here__(void);
@@ -94,8 +86,7 @@ size_t __unsafe_string_function_usage_here_char__(void);
     safe_strcpy(dest, src, size-PTR_DIFF(dest,base)-1)
 
 /* String copy functions - macro hell below adds 'type checking' (limited,
-   but the best we can do in C) and may tag with function name/number to
-   record the last 'clobber region' on that string */
+   but the best we can do in C) */
 
 #define fstrcpy(d,s) safe_strcpy((d),(s),sizeof(fstring)-1)
 #define fstrcat(d,s) safe_strcat((d),(s),sizeof(fstring)-1)
@@ -106,61 +97,43 @@ size_t __unsafe_string_function_usage_here_char__(void);
  * update a lot of code. To make this a little easier here are some
  * functions that provide the lengths with less pain */
 
-/* Inside the _fn variants of these is a call to clobber_region(), -
- * which might destroy the stack on a buggy function.  We help the
- * debugging process by putting the function and line who last caused
- * a clobbering into a static buffer.  If the program crashes at
- * address 0xf1f1f1f1 then this function is probably, but not
- * necessarily, to blame. */
-
 /* overmalloc_safe_strcpy: DEPRECATED!  Used when you know the
  * destination buffer is longer than maxlength, but you don't know how
  * long.  This is not a good situation, because we can't do the normal
  * sanity checks. Don't use in new code! */
 
 #define overmalloc_safe_strcpy(dest,src,maxlength) \
-	safe_strcpy_fn(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			dest,src,maxlength)
+	safe_strcpy_fn(dest,src,maxlength)
 
 #define safe_strcpy(dest,src,maxlength) \
-	safe_strcpy_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			dest,src,maxlength)
+	safe_strcpy_fn2(dest,src,maxlength)
 
 #define safe_strcat(dest,src,maxlength) \
-	safe_strcat_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			dest,src,maxlength)
+	safe_strcat_fn2(dest,src,maxlength)
 
 #define push_string_check(dest, src, dest_len, flags) \
-	push_string_check_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			dest, src, dest_len, flags)
+	push_string_check_fn2(dest, src, dest_len, flags)
 
 #define pull_string_talloc(ctx, base_ptr, smb_flags2, dest, src, src_len, flags) \
-	pull_string_talloc_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			ctx, base_ptr, smb_flags2, dest, src, src_len, flags)
+	pull_string_talloc_fn2(ctx, base_ptr, smb_flags2, dest, src, src_len, flags)
 
 #define clistr_push(cli, dest, src, dest_len, flags) \
-	clistr_push_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			cli, dest, src, dest_len, flags)
+	clistr_push_fn2(cli, dest, src, dest_len, flags)
 
 #define clistr_pull(inbuf, dest, src, dest_len, src_len, flags) \
-	clistr_pull_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			inbuf, dest, src, dest_len, src_len, flags)
+	clistr_pull_fn2(inbuf, dest, src, dest_len, src_len, flags)
 
 #define clistr_pull_talloc(ctx, base, flags2, pp_dest, src, src_len, flags) \
-	clistr_pull_talloc_fn(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			      ctx, base, flags2, pp_dest, src, src_len, flags)
+	clistr_pull_talloc_fn(ctx, base, flags2, pp_dest, src, src_len, flags)
 
 #define srvstr_push(base_ptr, smb_flags2, dest, src, dest_len, flags) \
-	srvstr_push_fn2(SAFE_STRING_FUNCTION_NAME, SAFE_STRING_LINE, \
-			base_ptr, smb_flags2, dest, src, dest_len, flags)
+	srvstr_push_fn2(base_ptr, smb_flags2, dest, src, dest_len, flags)
 
 #define alpha_strcpy(dest,src,other_safe_chars,maxlength) \
-	alpha_strcpy_fn(SAFE_STRING_FUNCTION_NAME,SAFE_STRING_LINE, \
-			dest,src,other_safe_chars,maxlength)
+	alpha_strcpy_fn(dest,src,other_safe_chars,maxlength)
 
 #define StrnCpy(dest,src,n) \
-	StrnCpy_fn(SAFE_STRING_FUNCTION_NAME,SAFE_STRING_LINE, \
-			dest,src,n)
+	StrnCpy_fn(dest,src,n)
 
 #ifdef HAVE_COMPILER_WILL_OPTIMIZE_OUT_FNS
 
@@ -168,38 +141,38 @@ size_t __unsafe_string_function_usage_here_char__(void);
    have the correct types (this works only where sizeof() returns the size of the buffer, not
    the size of the pointer). */
 
-#define safe_strcpy_fn2(fn_name, fn_line, d, s, max_len) \
+#define safe_strcpy_fn2(d, s, max_len) \
     (CHECK_STRING_SIZE(d, max_len+1) \
     ? __unsafe_string_function_usage_here__() \
-    : safe_strcpy_fn(fn_name, fn_line, (d), (s), (max_len)))
+    : safe_strcpy_fn((d), (s), (max_len)))
 
-#define safe_strcat_fn2(fn_name, fn_line, d, s, max_len) \
+#define safe_strcat_fn2(d, s, max_len) \
     (CHECK_STRING_SIZE(d, max_len+1) \
     ? __unsafe_string_function_usage_here__() \
-    : safe_strcat_fn(fn_name, fn_line, (d), (s), (max_len)))
+    : safe_strcat_fn((d), (s), (max_len)))
 
-#define push_string_check_fn2(fn_name, fn_line, dest, src, dest_len, flags) \
+#define push_string_check_fn2(dest, src, dest_len, flags) \
     (CHECK_STRING_SIZE(dest, dest_len) \
     ? __unsafe_string_function_usage_here_size_t__() \
-    : push_string_check_fn(fn_name, fn_line, dest, src, dest_len, flags))
+    : push_string_check_fn(dest, src, dest_len, flags))
 
-#define pull_string_talloc_fn2(fn_name, fn_line, ctx, base_ptr, smb_flags2, dest, src, src_len, flags) \
-    pull_string_talloc_fn(fn_name, fn_line, ctx, base_ptr, smb_flags2, dest, src, src_len, flags)
+#define pull_string_talloc_fn2(ctx, base_ptr, smb_flags2, dest, src, src_len, flags) \
+    pull_string_talloc_fn(ctx, base_ptr, smb_flags2, dest, src, src_len, flags)
 
-#define clistr_push_fn2(fn_name, fn_line, cli, dest, src, dest_len, flags) \
+#define clistr_push_fn2(cli, dest, src, dest_len, flags) \
     (CHECK_STRING_SIZE(dest, dest_len) \
     ? __unsafe_string_function_usage_here_size_t__() \
-    : clistr_push_fn(fn_name, fn_line, cli, dest, src, dest_len, flags))
+    : clistr_push_fn(cli, dest, src, dest_len, flags))
 
-#define clistr_pull_fn2(fn_name, fn_line, inbuf, dest, src, dest_len, srclen, flags) \
+#define clistr_pull_fn2(inbuf, dest, src, dest_len, srclen, flags) \
     (CHECK_STRING_SIZE(dest, dest_len) \
     ? __unsafe_string_function_usage_here_size_t__() \
-    : clistr_pull_fn(fn_name, fn_line, inbuf, dest, src, dest_len, srclen, flags))
+    : clistr_pull_fn(inbuf, dest, src, dest_len, srclen, flags))
 
-#define srvstr_push_fn2(fn_name, fn_line, base_ptr, smb_flags2, dest, src, dest_len, flags) \
+#define srvstr_push_fn2(base_ptr, smb_flags2, dest, src, dest_len, flags) \
     (CHECK_STRING_SIZE(dest, dest_len) \
     ? __unsafe_string_function_usage_here_size_t__() \
-    : srvstr_push_fn(fn_name, fn_line, base_ptr, smb_flags2, dest, src, dest_len, flags))
+    : srvstr_push_fn(base_ptr, smb_flags2, dest, src, dest_len, flags))
 
 #else