Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#3315: avoid conflicts with memset and memcpy #3374

Merged
merged 1 commit into from
Feb 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion core/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# **********************************************************
# Copyright (c) 2010-2018 Google, Inc. All rights reserved.
# Copyright (c) 2010-2019 Google, Inc. All rights reserved.
# Copyright (c) 2009-2010 VMware, Inc. All rights reserved.
# **********************************************************

Expand Down Expand Up @@ -80,6 +80,17 @@ endif ()
add_asm_target(arch/pre_inject_asm.asm preinject_asm_src preinject_asm_tgt ""
"-DNOT_DYNAMORIO_CORE_PROPER" "${asm_deps}")

if (UNIX AND X86)
# i#3315: We want our own memcpy and memset for the shared-lib DR core and
# for drinjectlib + drfrontendlib to avoid glibc-versioned symbols in
# our auxiliary tools (i#1504), but we do *not* want our own memcpy and
# memset for static-lib DR core. Thus we separate them out. The i#1504
# glibc versioning is only an issue on x86.
add_asm_target(arch/x86/memfuncs.asm memfuncs_asm_src memfuncs_asm_tgt
"_memfuncs" "" "${asm_deps}")
add_library(drmemfuncs STATIC ${memfuncs_asm_src})
endif ()

# i#1409: to share core code with non-core, we use the "drhelper" library.
# XXX: we're using _shared here, and for inject_shared, etc., to mean
# "shared between multiple libraries", while we've also started using
Expand Down Expand Up @@ -559,6 +570,11 @@ add_library(dynamorio SHARED
)

configure_core_lib(dynamorio)
if (UNIX AND X86)
# We need our own memcpy + memset for isolation.
# They're separated out for sharing for i#1504.
target_link_libraries(dynamorio drmemfuncs)
endif ()

if (UNIX)
# i#47: set the ELF header entry point to _start for early injection.
Expand Down Expand Up @@ -843,6 +859,10 @@ if ("${CMAKE_GENERATOR}" MATCHES "Visual Studio")
add_dependencies(${PRELOAD_NAME} ${preinject_asm_tgt})
endif ()
target_link_libraries(${PRELOAD_NAME} drhelper)
if (UNIX AND X86)
# We need our own memcpy + memset for isolation.
target_link_libraries(${PRELOAD_NAME} drmemfuncs)
endif ()
copy_target_to_device(${PRELOAD_NAME} "${location_suffix}")

# drpreinject.dll doesn't link in instr_shared.c so we can't include our inline
Expand Down Expand Up @@ -930,6 +950,10 @@ endif ()
add_library(drinjectlib ${inject_lib_type} ${INJECTOR_SRCS})
add_gen_events_deps(drinjectlib)
target_link_libraries(drinjectlib drdecode drhelper)
if (UNIX AND X86)
# We need our own memcpy + memset to avoid glibc versioning (i#1504).
target_link_libraries(drinjectlib drmemfuncs)
endif ()
set_target_properties(drinjectlib PROPERTIES
# COMPILE_DEFINITONS isn't working for me: cmake bug?
# Set define parameters for resources.rc
Expand Down Expand Up @@ -1066,3 +1090,7 @@ install_exported_target(drdecode ${INSTALL_LIB})
# We have to export drhelper as static libs we're exporting depend on it.
DR_export_target(drhelper)
install_exported_target(drhelper ${INSTALL_LIB_BASE})
if (UNIX AND X86)
DR_export_target(drmemfuncs)
install_exported_target(drmemfuncs ${INSTALL_LIB_BASE})
endif ()
132 changes: 132 additions & 0 deletions core/arch/x86/memfuncs.asm
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/* **********************************************************
* Copyright (c) 2011-2019 Google, Inc. All rights reserved.
* Copyright (c) 2001-2010 VMware, Inc. All rights reserved.
* ********************************************************** */

/*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* * Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* * Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* * Neither the name of VMware, Inc. nor the names of its contributors may be
* used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL VMWARE, INC. OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
* DAMAGE.
*/

/* Copyright (c) 2003-2007 Determina Corp. */
/* Copyright (c) 2001-2003 Massachusetts Institute of Technology */
/* Copyright (c) 2001 Hewlett-Packard Company */

/*
* memfuncs.asm: Contains our custom memcpy and memset routines.
*
* The core certainly needs its own memcpy and memset for libc isolation as
* described below, when a shared library and used for early injection.
* We also use these to avoid glibc-versioned imports in our auxiliary tools
* such as drrun (i#1504). However, when DR is built as a static library, these
* functions can conflict with the application's own versions (even when
* marking as weak): i#3348. Since a static-library DR will not be taking control
* early, and since these functions should always be re-entrant, we simply
* do not include them in libdynamorio_static and it imports from libc.
* We accomplish that by separating these into their own library.
*/

#include "../asm_defines.asm"
#include "x86_asm_defines.asm" /* PUSHGPR, POPGPR, etc. */
START_FILE

/***************************************************************************/
#ifdef UNIX

/* i#46: Implement private memcpy and memset for libc isolation. If we import
* memcpy and memset from libc in the normal way, the application can override
* those definitions and intercept them. In particular, this occurs when
* running an app that links in the Address Sanitizer runtime. Since we already
* need a reasonably efficient assembly memcpy implementation for safe_read, we
* go ahead and reuse the code for private memcpy and memset.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see an explicit reference to safe_read in here, but im guessing the comment is referring to these macros which might be shared? Do you mind clearing up exactly what parts or shared, or fixing this comment up if the following functions don't actually reuse stuff which is specifically for safe_read?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safe_read_asm shares the REP_STRING_OP macro which is also used here.

This comment does bring up something I did not make clear in the issue: there is a downside to having static DR use glibc memcpy. It lets sanitizers or other interceptors override
DR's memcpy. However, we already have interoperability concerns with sanitizers, so I was assuming we could live with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to also add the sanitizer concern to the code comments and not just the issue so I can send you a comment update PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #3376

*
* XXX: See comment on REP_STRING_OP about maybe using SSE instrs. It's more
* viable for memcpy and memset than for safe_read_asm.
*/

/* Private memcpy.
*/
DECLARE_FUNC(memcpy)
WEAK(memcpy)
GLOBAL_LABEL(memcpy:)
ARGS_TO_XDI_XSI_XDX() /* dst=xdi, src=xsi, n=xdx */
mov REG_XAX, REG_XDI /* Save dst for return. */
/* Copy xdx bytes, align on src. */
REP_STRING_OP(memcpy, REG_XSI, movs)
RESTORE_XDI_XSI()
ret /* Return original dst. */
END_FUNC(memcpy)

/* Private memset.
*/
DECLARE_FUNC(memset)
WEAK(memset)
GLOBAL_LABEL(memset:)
ARGS_TO_XDI_XSI_XDX() /* dst=xdi, val=xsi, n=xdx */
push REG_XDI /* Save dst for return. */
test esi, esi /* Usually val is zero. */
jnz make_val_word_size
xor eax, eax
do_memset:
/* Set xdx bytes, align on dst. */
REP_STRING_OP(memset, REG_XDI, stos)
pop REG_XAX /* Return original dst. */
RESTORE_XDI_XSI()
ret

/* Create pointer-sized value in XAX using multiply. */
make_val_word_size:
and esi, HEX(ff)
# ifdef X64
mov rax, HEX(0101010101010101)
# else
mov eax, HEX(01010101)
# endif
/* Use two-operand imul to avoid clobbering XDX. */
imul REG_XAX, REG_XSI
jmp do_memset
END_FUNC(memset)


# ifndef MACOS /* XXX: attribute alias issue, plus using nasm */
/* gcc emits calls to these *_chk variants in release builds when the size of
* dst is known at compile time. In C, the caller is responsible for cleaning
* up arguments on the stack, so we alias these *_chk routines to the non-chk
* routines and rely on the caller to clean up the extra dst_len arg.
*/
.global __memcpy_chk
.hidden __memcpy_chk
.set __memcpy_chk,memcpy

.global __memset_chk
.hidden __memset_chk
.set __memset_chk,memset
# endif

#endif /* UNIX */


END_FILE
73 changes: 1 addition & 72 deletions core/arch/x86/x86_shared.asm
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2011-2015 Google, Inc. All rights reserved.
* Copyright (c) 2011-2019 Google, Inc. All rights reserved.
* Copyright (c) 2001-2010 VMware, Inc. All rights reserved.
* ********************************************************** */

Expand Down Expand Up @@ -447,77 +447,6 @@ GLOBAL_LABEL(get_stack_ptr:)
#endif /* WINDOWS */


#ifdef UNIX
/* i#46: Implement private memcpy and memset for libc isolation. If we import
* memcpy and memset from libc in the normal way, the application can override
* those definitions and intercept them. In particular, this occurs when
* running an app that links in the Address Sanitizer runtime. Since we already
* need a reasonably efficient assembly memcpy implementation for safe_read, we
* go ahead and reuse the code for private memcpy and memset.
*
* XXX: See comment on REP_STRING_OP about maybe using SSE instrs. It's more
* viable for memcpy and memset than for safe_read_asm.
*/

/* Private memcpy.
*/
DECLARE_FUNC(memcpy)
GLOBAL_LABEL(memcpy:)
ARGS_TO_XDI_XSI_XDX() /* dst=xdi, src=xsi, n=xdx */
mov REG_XAX, REG_XDI /* Save dst for return. */
/* Copy xdx bytes, align on src. */
REP_STRING_OP(memcpy, REG_XSI, movs)
RESTORE_XDI_XSI()
ret /* Return original dst. */
END_FUNC(memcpy)

/* Private memset.
*/
DECLARE_FUNC(memset)
GLOBAL_LABEL(memset:)
ARGS_TO_XDI_XSI_XDX() /* dst=xdi, val=xsi, n=xdx */
push REG_XDI /* Save dst for return. */
test esi, esi /* Usually val is zero. */
jnz make_val_word_size
xor eax, eax
do_memset:
/* Set xdx bytes, align on dst. */
REP_STRING_OP(memset, REG_XDI, stos)
pop REG_XAX /* Return original dst. */
RESTORE_XDI_XSI()
ret

/* Create pointer-sized value in XAX using multiply. */
make_val_word_size:
and esi, HEX(ff)
# ifdef X64
mov rax, HEX(0101010101010101)
# else
mov eax, HEX(01010101)
# endif
/* Use two-operand imul to avoid clobbering XDX. */
imul REG_XAX, REG_XSI
jmp do_memset
END_FUNC(memset)


# ifndef MACOS /* XXX: attribute alias issue, plus using nasm */
/* gcc emits calls to these *_chk variants in release builds when the size of
* dst is known at compile time. In C, the caller is responsible for cleaning
* up arguments on the stack, so we alias these *_chk routines to the non-chk
* routines and rely on the caller to clean up the extra dst_len arg.
*/
.global __memcpy_chk
.hidden __memcpy_chk
.set __memcpy_chk,memcpy

.global __memset_chk
.hidden __memset_chk
.set __memset_chk,memset
# endif
#endif /* UNIX */


/***************************************************************************/
#if defined(WINDOWS) && !defined(X64)

Expand Down
6 changes: 5 additions & 1 deletion libutil/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# **********************************************************
# Copyright (c) 2010-2018 Google, Inc. All rights reserved.
# Copyright (c) 2010-2019 Google, Inc. All rights reserved.
# Copyright (c) 2009-2010 VMware, Inc. All rights reserved.
# **********************************************************

Expand Down Expand Up @@ -207,6 +207,10 @@ if (AARCH64)
endif()
# We need drhelper for dynamorio_syscall for core/unix/os.c.
target_link_libraries(drfrontendlib drhelper)
if (UNIX AND X86)
# We need our own memcpy + memset to avoid glibc versioning (i#1504).
target_link_libraries(drfrontendlib drmemfuncs)
endif ()
add_static_lib_debug_info(drfrontendlib "${INSTALL_BIN}")
DR_export_target(drfrontendlib)
install_exported_target(drfrontendlib ${INSTALL_BIN})