From 17b6b066cf8cebe40ce9b6f219d3b8fb74fe8ad1 Mon Sep 17 00:00:00 2001 From: William Godbe Date: Wed, 30 Dec 2015 14:16:10 -0800 Subject: [PATCH] Audit usage of memcpy in PAL for Debug --- src/inc/sbuffer.inl | 4 +-- src/pal/inc/pal.h | 66 +++++++++++++++++++++++------------ src/pal/src/cruntime/misc.cpp | 24 +++++++++++++ src/pal/src/include/pal/palinternal.h | 4 +++ src/pal/src/safecrt/memcpy_s.c | 3 ++ src/strongname/api/common.h | 23 +++++++++--- src/vm/common.h | 23 +++++++++--- 7 files changed, 113 insertions(+), 34 deletions(-) diff --git a/src/inc/sbuffer.inl b/src/inc/sbuffer.inl index 70893b1..4d888ed 100644 --- a/src/inc/sbuffer.inl +++ b/src/inc/sbuffer.inl @@ -229,7 +229,7 @@ inline void SBuffer::Set(const SBuffer &buffer) // From the code for Resize and EnsureMutable, this is clearly impossible. PREFIX_ASSUME( (this->m_buffer != NULL) || (buffer.m_size == 0) ); - CopyMemory(m_buffer, buffer.m_buffer, buffer.m_size); + MoveMemory(m_buffer, buffer.m_buffer, buffer.m_size); } RETURN; @@ -255,7 +255,7 @@ inline void SBuffer::Set(const BYTE *buffer, COUNT_T size) // From the code for Resize, this is clearly impossible. PREFIX_ASSUME( (this->m_buffer != NULL) || (size == 0) ); - CopyMemory(m_buffer, buffer, size); + MoveMemory(m_buffer, buffer, size); RETURN; } diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h index 7eaba46..b937917 100644 --- a/src/pal/inc/pal.h +++ b/src/pal/inc/pal.h @@ -3411,7 +3411,7 @@ GetThreadTimes( OUT LPFILETIME lpExitTime, OUT LPFILETIME lpKernelTime, OUT LPFILETIME lpUserTime); - + #define TLS_OUT_OF_INDEXES ((DWORD)0xFFFFFFFF) PALIMPORT @@ -3908,9 +3908,9 @@ PALIMPORT HANDLE PALAPI HeapCreate( - IN DWORD flOptions, - IN SIZE_T dwInitialSize, - IN SIZE_T dwMaximumSize); + IN DWORD flOptions, + IN SIZE_T dwInitialSize, + IN SIZE_T dwMaximumSize); PALIMPORT LPVOID @@ -4442,10 +4442,10 @@ int PALAPI CompareStringOrdinal( IN LPCWSTR lpString1, - IN int cchCount1, - IN LPCWSTR lpString2, - IN int cchCount2, - IN BOOL bIgnoreCase); + IN int cchCount1, + IN LPCWSTR lpString2, + IN int cchCount2, + IN BOOL bIgnoreCase); typedef struct _nlsversioninfoex { DWORD dwNLSVersionInfoSize; @@ -4460,15 +4460,15 @@ int PALAPI FindNLSStringEx( IN LPCWSTR lpLocaleName, - IN DWORD dwFindNLSStringFlags, - IN LPCWSTR lpStringSource, - IN int cchSource, + IN DWORD dwFindNLSStringFlags, + IN LPCWSTR lpStringSource, + IN int cchSource, IN LPCWSTR lpStringValue, - IN int cchValue, - OUT LPINT pcchFound, - IN LPNLSVERSIONINFOEX lpVersionInformation, - IN LPVOID lpReserved, - IN LPARAM lParam ); + IN int cchValue, + OUT LPINT pcchFound, + IN LPNLSVERSIONINFOEX lpVersionInformation, + IN LPVOID lpReserved, + IN LPARAM lParam ); typedef enum { COMPARE_STRING = 0x0001, @@ -4479,10 +4479,10 @@ BOOL PALAPI IsNLSDefinedString( IN NLS_FUNCTION Function, - IN DWORD dwFlags, - IN LPNLSVERSIONINFOEX lpVersionInfo, - IN LPCWSTR lpString, - IN int cchStr ); + IN DWORD dwFlags, + IN LPNLSVERSIONINFOEX lpVersionInfo, + IN LPCWSTR lpString, + IN int cchStr ); PALIMPORT @@ -4508,7 +4508,7 @@ int PALAPI GetSystemDefaultLocaleName( OUT LPWSTR lpLocaleName, - IN int cchLocaleName); + IN int cchLocaleName); #ifdef UNICODE #define GetLocaleInfo GetLocaleInfoW @@ -6086,12 +6086,33 @@ typedef struct { PALIMPORT div_t div(int numer, int denom); +#if defined(_DEBUG) + +/*++ +Function: +PAL_memcpy + +Overlapping buffer-safe version of memcpy. +See MSDN doc for memcpy +--*/ +EXTERN_C +PALIMPORT +void *PAL_memcpy (void *dest, const void *src, size_t count); + +PALIMPORT void * __cdecl memcpy(void *, const void *, size_t); + +#define memcpy PAL_memcpy +#define IS_PAL_memcpy 1 +#define TEST_PAL_DEFERRED(def) IS_##def +#define IS_REDEFINED_IN_PAL(def) TEST_PAL_DEFERRED(def) +#else //defined(_DEBUG) PALIMPORT void * __cdecl memcpy(void *, const void *, size_t); +#endif //defined(_DEBUG) PALIMPORT int __cdecl memcmp(const void *, const void *, size_t); PALIMPORT void * __cdecl memset(void *, int, size_t); PALIMPORT void * __cdecl memmove(void *, const void *, size_t); PALIMPORT void * __cdecl memchr(const void *, int, size_t); - +PALIMPORT long long int __cdecl atoll(const char *); PALIMPORT size_t __cdecl strlen(const char *); PALIMPORT int __cdecl strcmp(const char*, const char *); PALIMPORT int __cdecl strncmp(const char*, const char *, size_t); @@ -6112,7 +6133,6 @@ PALIMPORT int __cdecl vsprintf(char *, const char *, va_list); PALIMPORT int __cdecl sscanf(const char *, const char *, ...); PALIMPORT int __cdecl atoi(const char *); PALIMPORT LONG __cdecl atol(const char *); -PALIMPORT long long int __cdecl atoll(const char *); PALIMPORT ULONG __cdecl strtoul(const char *, char **, int); PALIMPORT double __cdecl atof(const char *); PALIMPORT double __cdecl strtod(const char *, char **); diff --git a/src/pal/src/cruntime/misc.cpp b/src/pal/src/cruntime/misc.cpp index 9910d7e..e1749b9 100644 --- a/src/pal/src/cruntime/misc.cpp +++ b/src/pal/src/cruntime/misc.cpp @@ -38,6 +38,9 @@ Abstract: #if defined(_AMD64_) || defined(_x86_) #include #endif // defined(_AMD64_) || defined(_x86_) +#if defined(_DEBUG) +#include +#endif //defined(_DEBUG) SET_DEFAULT_DEBUG_CHANNEL(CRT); @@ -634,3 +637,24 @@ void MiscUnsetenv(const char *name) } InternalLeaveCriticalSection(pthrCurrent, &gcsEnvironment); } + +#if defined(_DEBUG) + +/*++ +Function: +PAL_memcpy + +Overlapping buffer-safe version of memcpy. +See MSDN doc for memcpy +--*/ +void *PAL_memcpy (void *dest, const void *src, size_t count) +{ + UINT_PTR x = (UINT_PTR)dest, y = (UINT_PTR)src; + assert((x + count <= y) || (y + count <= x)); + + void *ret; + #undef memcpy + ret = memcpy(dest, src, count); + return ret; +} +#endif //DEBUG \ No newline at end of file diff --git a/src/pal/src/include/pal/palinternal.h b/src/pal/src/include/pal/palinternal.h index 4ea8c3b..c059db5 100644 --- a/src/pal/src/include/pal/palinternal.h +++ b/src/pal/src/include/pal/palinternal.h @@ -162,7 +162,9 @@ function_name() to call the system's implementation of those functions when including standard C header files */ #define div DUMMY_div #define div_t DUMMY_div_t +#if !defined(_DEBUG) #define memcpy DUMMY_memcpy +#endif //!defined(_DEBUG) #define memcmp DUMMY_memcmp #define memset DUMMY_memset #define memmove DUMMY_memmove @@ -350,7 +352,9 @@ function_name() to call the system's implementation #undef atexit #undef div #undef div_t +#if !defined(_DEBUG) #undef memcpy +#endif //!defined(_DEBUG) #undef memcmp #undef memset #undef memmove diff --git a/src/pal/src/safecrt/memcpy_s.c b/src/pal/src/safecrt/memcpy_s.c index b61ddb3..1462650 100644 --- a/src/pal/src/safecrt/memcpy_s.c +++ b/src/pal/src/safecrt/memcpy_s.c @@ -21,6 +21,7 @@ #include #include +#include #include "internal_securecrt.h" #include "mbusafecrt_internal.h" @@ -75,6 +76,8 @@ errno_t __cdecl memcpy_s( return EINVAL; } + UINT_PTR x = (UINT_PTR)dst, y = (UINT_PTR)src; + assert((x + count <= y) || (y + count <= x)); memcpy(dst, src, count); return 0; } diff --git a/src/strongname/api/common.h b/src/strongname/api/common.h index ea315e4..43920e5 100644 --- a/src/strongname/api/common.h +++ b/src/strongname/api/common.h @@ -10,7 +10,7 @@ #define _common_h_ #if defined(_MSC_VER) && defined(_X86_) && !defined(FPO_ON) -#pragma optimize("y", on) // Small critical routines, don't put in EBP frame +#pragma optimize("y", on) // Small critical routines, don't put in EBP frame #define FPO_ON 1 #define COMMON_TURNED_FPO_ON 1 #endif @@ -227,25 +227,38 @@ inline void* memcpyUnsafe(void *dest, const void *src, size_t len) // #if defined(_DEBUG) && !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE) + //If memcpy has been defined to PAL_memcpy, we undefine it so that this case + //can be covered by the if !defined(memcpy) block below + #ifdef FEATURE_PAL + #if IS_REDEFINED_IN_PAL(memcpy) + #undef memcpy + #endif //IS_REDEFINED_IN_PAL + #endif //FEATURE_PAL + // You should be using CopyValueClass if you are doing an memcpy // in the CG heap. - #if !defined(memcpy) + #if !defined(memcpy) inline void* memcpyNoGCRefs(void * dest, const void * src, size_t len) { WRAPPER_NO_CONTRACT; + + #ifndef FEATURE_PAL + return memcpy(dest, src, len); + #else //FEATURE_PAL + return PAL_memcpy(dest, src, len); + #endif //FEATURE_PAL - return memcpy(dest, src, len); } extern "C" void * __cdecl GCSafeMemCpy(void *, const void *, size_t); #define memcpy(dest, src, len) GCSafeMemCpy(dest, src, len) #endif // !defined(memcpy) + #if !defined(CHECK_APP_DOMAIN_LEAKS) #define CHECK_APP_DOMAIN_LEAKS 1 #endif #else // !_DEBUG && !DACCESS_COMPILE && !CROSSGEN_COMPILE inline void* memcpyNoGCRefs(void * dest, const void * src, size_t len) { WRAPPER_NO_CONTRACT; - return memcpy(dest, src, len); } #endif // !_DEBUG && !DACCESS_COMPILE && !CROSSGEN_COMPILE @@ -399,7 +412,7 @@ typedef VOID (__stdcall *WAITORTIMERCALLBACK)(PVOID, BOOL); #include "pedecoder.h" #if defined(COMMON_TURNED_FPO_ON) -#pragma optimize("", on) // Go back to command line default optimizations +#pragma optimize("", on) // Go back to command line default optimizations #undef COMMON_TURNED_FPO_ON #undef FPO_ON #endif diff --git a/src/vm/common.h b/src/vm/common.h index 2d23f8c..2fe1e01 100644 --- a/src/vm/common.h +++ b/src/vm/common.h @@ -22,7 +22,7 @@ #if defined(_MSC_VER) && defined(_X86_) && !defined(FPO_ON) -#pragma optimize("y", on) // Small critical routines, don't put in EBP frame +#pragma optimize("y", on) // Small critical routines, don't put in EBP frame #define FPO_ON 1 #define COMMON_TURNED_FPO_ON 1 #endif @@ -89,6 +89,7 @@ #include #include #include +#include #include @@ -256,12 +257,26 @@ FORCEINLINE void* memcpyUnsafe(void *dest, const void *src, size_t len) // #if defined(_DEBUG) && !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE) + //If memcpy has been defined to PAL_memcpy, we undefine it so that this case + //can be covered by the if !defined(memcpy) block below + #ifdef FEATURE_PAL + #if IS_REDEFINED_IN_PAL(memcpy) + #undef memcpy + #endif //IS_REDEFINED_IN_PAL + #endif //FEATURE_PAL + // You should be using CopyValueClass if you are doing an memcpy // in the CG heap. - #if !defined(memcpy) + #if !defined(memcpy) FORCEINLINE void* memcpyNoGCRefs(void * dest, const void * src, size_t len) { WRAPPER_NO_CONTRACT; - return memcpy(dest, src, len); + + #ifndef FEATURE_PAL + return memcpy(dest, src, len); + #else //FEATURE_PAL + return PAL_memcpy(dest, src, len); + #endif //FEATURE_PAL + } extern "C" void * __cdecl GCSafeMemCpy(void *, const void *, size_t); #define memcpy(dest, src, len) GCSafeMemCpy(dest, src, len) @@ -517,7 +532,7 @@ inline HRESULT CreateConfigStreamHelper(LPCWSTR filename, IStream** pOutStream) #if defined(COMMON_TURNED_FPO_ON) -#pragma optimize("", on) // Go back to command line default optimizations +#pragma optimize("", on) // Go back to command line default optimizations #undef COMMON_TURNED_FPO_ON #undef FPO_ON #endif -- 2.7.4