criblio / appscope

Gain observability into any Linux command or application with no code modification
https://appscope.dev
Apache License 2.0
266 stars 33 forks source link

Spike: Go Signal Handling #508

Closed iapaddler closed 2 years ago

iapaddler commented 3 years ago

Reproduce: docker run --rm -it --entrypoint /bin/sh -u 0:0 hashicorp/terraform:1.0.3 --or on a dev instance -- docker run --rm -it --entrypoint /bin/sh -u 0:0 -v /<fullpath>/appscope/:/appscope hashicorp/terraform:1.0.3 where fullpath points to an appscope repo

ldscope terraform help you get the error: /tmp/libscope-0.7.2/ldscopedyn could not find or execute commandhelp. Exiting.

The terraform executable is executing ldscopedyn with the command help. Of course, help is not an executable.

The terraform executable uses a package panicwrap (https://github.com/mitchellh/panicwrap) which re-executes a Go binary and monitors stderr output from the binary for a panic.

The panicwrap package defines a command to be executed. This instantiates a Command struct. The path used in the Command struct is sourced from the Go function os.executablePath. The function os.executablePath calls syscall.Readlink with the path /proc/self/exe. When a static app is started with ldscope the link in /proc/self/exe points to ldscopedyn. As a result, ldscopedyn is started with the argument argv[1], the original argv[2], which in this case is help. Boom.

At this point, without testing, we think the approach to resolve this is an interposition of syscall.Readlink. If /proc/self/exe is used as path and the result contains the substring ldscope then we will return argv[1], the original intended executable. In this case terraform.

iapaddler commented 3 years ago

After much thinking and experimenting we are at a point where we can run terraform in a loop and it seems to not crash. Moreover, we can safely modify the argv array and also not change memory in the parent.

However, we've had to also disable other interposed functions. It seems that part of what's needed is likely good.

What could explain what we are currently seeing is a change in the fs register value, the Go g value, in the parent when a fork/exec is executed. Have not verified that. If this is the case, could it imply that Go runtime is changing the m or the p out from under the parent on a fork?

iapaddler commented 3 years ago

FWIW. What's been done at this point is to directly hook the raw syscall functions used by the Go runtime to do an exec and a vfork. These are assy functions. They do not utilize a Go preamble that is able to manage the stack.

A direct hook implies not using the entry point asm_cgocall() in the runtime. Therefore, looking back a stack frame to get variables and using that previous stack frame to jump to the intended raw syscall code. It works. Just requires more assy code.

This does allow us to safely manipulate the argv array and the flags used on vfork.

iapaddler commented 3 years ago

The terraform exec has been running without issue for a while at this point.

Go runtime refresher:

In the terraform executable the use of a panic wrapper package causes the current process to do a vfork/exec of itself. In the terraform case, in the exit handler, very occasionally, not every time, we are not able to parse the the fs value from the'g'. In this case, the value we get from the syscall is not a valid Go fs and the Go runtime crashes.

We updated the code in the case where the fs value can not be parsed from the 'g' to return without calling the interposed function. This has enabled terraform to run for several hours thus far without issue.

Interposing the exec syscall used by the Go runtime has enabled us to manipulate the argv array such that when the panic wrapper does an exec of itself it behaves as expected.

Next, we need to validate specific fork behavior. The question here relates to the use of CLONE_VM in a vfork which causes memory in the parent to be modified when we update the argv array. It may be harmless. We need to validate.

iapaddler commented 3 years ago

We have terraform running. It runs in a test looping for hours: ldscope terraform -help.

We do not need to modify clone flags in the vfork operation. So far. We are allocing dynamic memory and modifying the argv array of the calling Go function when an Go static makes an execve syscall and yet it doesn't appear to have an adverse affect on the parent Go routine.

Due to the readlink of /proc/self/exe and an execve with this value, we lose the original value of argv[1]. Therefore, we have saved a pointer to the original argv array in order build the necessary argv for an execve syscall.

iapaddler commented 3 years ago

We have been able to update the argv array and enable terraform to execute as expected.

We are now faced with supporting behavior that we've been putting off for a while. Can't put it off any longer; support for a static Go exec to fork/exec another static Go exec.

With the fork/exec behavior, the SIGCHLD signal is able to be delivered while the thread executing the signal handler is in an interposed handler. This means that the signal handling in the Go runtime can't find a 'g' and it bails out.

At this point, it seems that we need to restore the fs register when signal code is run in the Go runtime. This implies interposing the base signal receiver, detecting that the current thread is in an interposed handler, getting a valid fs register content for a viable 'g', restore the fs register, do the signal handler, restore the fs register to the libc TLS value. Then get a cup of coffee.

iapaddler commented 3 years ago

The panic wrapper package, being used with terraform, supports a few signal handlers. In response, we are adding a whole new feature; supporting Go static execs that create signal handlers. This means that we must survive the cases where a signal is delivered while we are interposing a function.

iapaddler commented 3 years ago

The interposed signal handler appears to be working as needed. But then, the stack checker finds us: runtime: unexpected return pc for runtime.sigtrampgo called from 0x7fcc796b06df

iapaddler commented 3 years ago

Having all the requisite plumbing in place to support signal handlers, we encounter an occasional case where the 'g' can not be located when a signal handler is attempting to execute.

Found a/the scenario where this occurs. In the switch function, when a new 'g' is encountered we create a corresponding thread so that libc calls have a unique TLS for each 'g'. In order to execute the thread related functions in libc, before updating the fs register with a new value, we use a common thread TLS as an interim. That case was not identified when the signal handler runs. That is, the corresponding 'g' was not exported for the signal handler. Works much better.

Initial support for this, in order to prove that this is the missing 'g', causes serialization issues. Will need to explore a long term solution.

ghost commented 3 years ago

moving out of 0.7.5 milestone

iapaddler commented 3 years ago

Problem Statement

We do not support the ability for a static Go application to configure and utilize signal handlers. The Terraform application is the first use case where we’ve encountered a Go static executable that employs signal handlers.

Behavior

When a static Go executable that employs signal handlers, such as Terraform, is scoped, we will occasionally encounter an error detected by the Go runtime; fatal: bad g in signal handler. This occurs when a signal handler is executed at the time the fs register is switched for use with a libc function. Note that the error doesn’t occur on every execution. It requires running the application numerous times in a loop from a script in order to encounter the error.

Blocking signals when the fs register is switched from 'g' to TLS is not possible. Blocking all signals all the time is not reasonable as the Go runtime uses SIGURG internally. Suffice to say that the Go routine is interrupted by a signal and subsequently the fs register switch code is executed. At that point the signal has already been delivered, therefore, blocking the signal does not help.

Approaches

There are a few options as we consider support for signal handlers in Go static executables. (1) Don’t support signal handlers in Go static execs.

In ldscope we could examine symbols to determine if signal functions are included in the executable. If signal code appears to be present we would execute, but would not scope the application. The application would not fail. No data would be extracted from the application. We would report this case, likely in the form of a log entry and an event intended to notify that the application is not scoped.

(2) Continue with the current design. A fair amount of time has been applied to this option without an obvious solution. We have been able to interpose signal handler functions in the Go runtime such that we have an opportunity to update the ‘g' and/or the fs register before the runtime references a ‘g’ in signal handling. We can save the ‘g' in use and apply it as needed. A significant impediment results, however, as we’ve seen multiple issues with applying a ‘g’ during signal handling. In some cases the memory pointed to a ‘g’ appears to have been garbage collected. In other cases, the ‘m’ associated with a saved ‘g’ is no longer valid. It is possible for the runtime to update an ‘m’. In still other cases, ‘g.m’ appear to be be valid while the ‘p’ associated with the 'm’ is not expected to be present and the runtime panics.

(3) Update the design approach for static Go execs such that we don’t switch the fs register. This design approach is not fully worked out. It will require research effort to determine feasibility. The fact remains that libc functions require the ability to locate a TLS object. The basic thinking here is to use a modified libc that, when executing a static Go application, retrieves a TLS object without reference to the fs register. Should this turn out to work, it will reduce complexity of libscope code and improve performance of scoped static Go executables.

Proposal

It is proposed that we validate the efficacy of option 3 above.

It appears that continuing with option 2 will require time, without any assurance that it will result in an acceptable solution.

Option 1 will allow us to ignore the signal use case, if that is deemed acceptable at this point.

Should we choose to proceed with option 3, we will need to verify that a local build of a libc library linked to libscope will work as expected. Given the use of relatively involved contrib sources, such as openSSL and pcre2, use of a local libc could require some work. It’s not expected to be a blocker.

The biggest validation of option 3 involves the ability to provide access to the appropriate TLS object without fs register reference in a Go static exec case.

Very similar TLS functionality is present in gnu libc and musl libc. Using musl libc as a reference, we can see that the function pthread_self() results in this code: __asm__ ("mov %%fs:0,%0" : "=r" (tp) );. Example of the use of a TLS reference: CURRENT_LOCALE (__pthread_self()->locale). We can see that the fs register content is the TLS.

One way to approach this, just by way of example, to illustrate the intent, is a change in the way TLS is retrieved when a Go static exec is being used with libscope. It should be possible to use the current ‘g' as an identifier to locate the associated TLS. In this case, the ‘g’ just needs to be unique. The requirement is to ensure that no two 'g' routines reference the same TLS at the the same time. Even in the case where ‘g’ might be garbage collected, it should not matter as long as the 'g’ is unique. We could start with a modification to the function that retrieves fs register content. In musl libc that is __get_tp().

static inline uintptr_t __get_tp()
{

   uintptr_t tp;
   asm ("mov %%fs:0,%0" : "=r" (tp) );
   return tp;

} 

Supporting a Go static might look something like this:

extern int g_go_static;

extern uint64_t go_g;

static inline uintptr_t __get_tp()
{

   uintptr_t tp;

   if (g_go_static == 0) {

      asm ("mov %%fs:0,%0" : "=r" (tp) );

   } else {

      /* we'd want to wrap the specific list function. left for illustration purposes */

      tp = lstFind(g_tlslist, go_g)

      /* error handling here */

   }

   return tp;

}

The point of the above illustration is to suggest that the current 'g' can be used as an identifier to locate the corresponding TLS in libc code, without the need to reference the fs register. But, only for Go static execs.

Conclusion

Supporting signal handlers in Go static applications in the presence of an fs register switch is difficult. It may require that we take a different approach to executing libc functions from within the context of a Go static exec.

iapaddler commented 2 years ago

Some progress has been made in determining if the design approach described will work. The end goal is the removal of a dependency on the fs register pointing to the thread local base, the TCB, for the current thread. The Go runtime uses the fs register to point to the current 'g'. In order to call glibc functions the fs register must be saved and then reloaded with the current TCB. That requires a syscall.

The concept is based on the using a static library that supports libc functions. The internal libc would be converted to provide a TCB in all cases. With a Go static executable a TCB needs to be made available without the use of the fs register. The fs register can continue to be used in dynamic apps, including Go dynamic apps.

The first step is to get a static local libc that can be linked with libscope.so during build. The end result implies changes to all libc functions used in libscope. However, all we want to accomplish initially is validation that 1) we can execute a Go static app properly without an fs register update and 2) will signal handlers execute without issue if there is no fs register update.

Instead of making sweeping changes, changing only what's needed in order to prove or disprove this approach, results in making changes to the internal libc utilized for test. This is not what we want to do in a viable solution. However, it does get us to a proof point much more directly. We started with musl libc and removed dependencies on TCB. This included hard coded values for errno and removal of thread area values, disable creation of stdin, stdout and stderr. Next, we needed to create a memory allocation capability that does not rely on a TCB nor does it manipulate the heap pointer. This turns out to be quite difficult with default musl libc memory allocation. Getting a viable free capability without the use of meta data is a non-starter. Getting correct meta data without use of the heap is problematic. Therefore, a really horrible hack was employed. We used the example provided with musl libc for an alternate memory allocator. Removing dependencies on TCB and the heap is reasonable in this starting point. However, there is no free or realloc functionality. So, we just do nothing on free and we created a simple realloc function.

The next step is to manage the namespace. Use of an internal libc implies multiple definitions of all libc functions. Obviously, not going to work. So, objcopy to the rescue. A single command line allows us to add a prefix to all symbols. For example: objcopy --prefix-symbols=scope_ ~/appscope/contrib/libmusl/libc.a. At this point we have an internal libc that is utilized by calling functions with a scope_ prefix; scope_malloc, etc. We then redefine the libc functions that we want to apply in libscope. Example; extern int scope_isprint(int); #undef isprint #define isprint scope_isprint. This approach is applied to specific code, not a wholesale change.

Next, contrib code. We utilize third party libs for JSON, yaml, SSL, regex. These libraries make libc calls. The first ones that become problematic, without a TCB and the required fs register switch, are memory allocation functions. Once again, objcopy. We change specific symbols in contrib libs. Example; objcopy --redefine-sym malloc=scope_malloc build/pcre2/libpcre2-posix.a. This creates the ability for a contrib lib to call the internal libc function as needed without modification to source.

When the appropriate updates are applied we are able to realize Go static applications that execute effectively without fs register updates. The next step is to validate that signal handlers run asynchronously and without issue in Go statics. We used the terraform container in order to validate signals since that is where the issue first emerged. We are able to have terraform run several hundred times from a shell script without apparent issue. This has not been possible to date.

A new issue will be created to track projects needed in order create a viable solution based on this design approach. An epoch in agile terms.

iapaddler commented 2 years ago

The changes made to musl from branch master in order to get an internal libc.a to work: musl_go_libc_diff.txt

iapaddler commented 2 years ago

Changes made to appscope in order to get an internal libc.a to work. With this Go static execs work without the need for an fs register switch. The terraform app works, which implies that signal handlers are running.

This was pushed to branch spike/508-go-signals

appscope_libc_go.txt

iapaddler commented 2 years ago

This is a script used to build a libc.a and a resulting libscope.


#! /bin/bash

cd ~/musl
echo "Build musl libc"
make
echo "Copy libc"
cp ./lib/libc.a ~/appscope/contrib/libmusl/.
echo "Add prefix to symbols"
objcopy --prefix-symbols=scope_ ~/appscope/contrib/libmusl/libc.a
cd ~/appscope
echo "Build core"
make coreclean
make coreall
gdb --args ldscope ./test/testContainers/go/net/tlsClientStatic