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

Mono.Unix.Native.Syscall.sys_strerror_r SEGFAULT on musl libc #9532

Closed
frebib opened this issue Jul 13, 2018 · 8 comments · Fixed by #10240

Comments

@frebib
Copy link
Contributor

@frebib frebib commented Jul 13, 2018

Steps to Reproduce

  1. On any Alpine Linux/musl based OS (docker run -ti --rm spritsail/alpine-sdk)
  2. apk add mono (from edge/testing repository)
  3. csc <the file below>
using System;
using Mono.Unix;

namespace Mono.Unix {
    public class UnixTest {
        public static void Main() {
            Console.WriteLine("EINVAL: " + UnixMarshal.GetErrorDescription(Native.Errno.EINVAL));
        }
    }
}
  1. mono <the file above>.exe
  2. Crash!

Cause of the issue

The Mono installation I am running is built from source. Observing the compilation of the support/ libraries, namely libMonoPosixHelper.so with cd support; make V=1.
The offending source errno.c which contains the definition for the native half of Mono.Unix.Native.Syscall.sys_strerror_r compiles as such:

gcc -DHAVE_CONFIG_H -I. -I..  -I../mono/eglib -I../mono/eglib -I.. -Os -fomit-frame-pointer -DGC_LINUX_THREADS -D_GNU_SOURCE -D_REENTRANT -DUSE_MMAP -DUSE_MUNMAP -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes  -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wno-cast-qual -Wwrite-strings -Wno-switch -Wno-switch-enum -Wno-unused-value -Wno-attributes -Wno-format-zero-length  -DUSE_COMPILER_TLS  -Os -fomit-frame-pointer -fno-strict-aliasing -std=gnu99 -fno-strict-aliasing -fwrapv -DMONO_DLL_EXPORT -Wno-unused-but-set-variable -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes  -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wno-cast-qual -Wwrite-strings -Wno-switch -Wno-switch-enum -Wno-unused-value -Wno-attributes -Wno-format-zero-length -mno-tls-direct-seg-refs -Werror-implicit-function-declaration -c -o errno.lo errno.c

The issue here is that _GNU_SOURCE is defined even though musl provides no such definition in any of the headers (AFAIK) because it is not GNU compatible. The strerror_r implementation in musl is of the XPG variant, so should follow the else branch of the ifdef.

Because of this _GNU_SOURCE definition, presumably from somewhere within the build files, this library is built against the wrong implementation for the platforb and therefore causing the crash.

Interestingly the _GNU_SOURCE definition isn't supplied during autogen/configure so I'm confused as to why it's defined at compilation time.

configure:22997: checking whether strerror_r is declared                                                                                                                                                                                                                        
configure:22997: gcc -c -Os -fomit-frame-pointer -fno-strict-aliasing -std=gnu99 -fno-strict-aliasing -fwrapv -DMONO_DLL_EXPORT -Wno-unused-but-set-variable -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes  -Wmissing-prototypes -Wnested-ex
conftest.c:143:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]                                                                                                                                                                                         
 main ()                                                                                                                                                                                                                                                                        
 ^~~~                                                                                                                                                                                                                                                                           
configure:22997: $? = 0                                                                                                                                                                                                                                                         
configure:22997: result: yes                                                                                                                                                                                                                                                    
configure:23010: checking for strerror_r                                                                                                                                                                                                                                        
configure:23010: gcc -o conftest -Os -fomit-frame-pointer -fno-strict-aliasing -std=gnu99 -fno-strict-aliasing -fwrapv -DMONO_DLL_EXPORT -Wno-unused-but-set-variable -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes  -Wmissing-prototypes -W
conftest.c:133:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]                                                                                                                                                                                         
 char strerror_r ();                                                                                                                                                                                                                                                            
 ^~~~                                                                                                                                                                                                                                                                           
conftest.c:142:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]                                                                                                                                                                                         
 main ()                                                                                                                                                                                                                                                                        
 ^~~~                                                                                                                                                                                                                                                                           
configure:23010: $? = 0                                                                                                                                                                                                                                                         
configure:23010: result: yes                                                                                                                                                                                                                                                    
configure:23019: checking whether strerror_r returns char *                                                                                                                                                                                                                     
configure:23043: gcc -c -Os -fomit-frame-pointer -fno-strict-aliasing -std=gnu99 -fno-strict-aliasing -fwrapv -DMONO_DLL_EXPORT -Wno-unused-but-set-variable -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes  -Wmissing-prototypes -Wnested-ex
conftest.c:145:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]                                                                                                                                                                                         
 main ()                                                                                                                                                                                                                                                                        
 ^~~~                                                                                                                                                                                                                                                                           
conftest.c: In function 'main':                                                                                                                                                                                                                                                 
conftest.c:149:13: error: invalid type argument of unary '*' (have 'int')                                                                                                                                                                                                       
    char x = *strerror_r (0, buf, sizeof buf);                                                                                                                                                                                                                                  
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                                                                                                                   
conftest.c:150:14: warning: initialization makes pointer from integer without a cast [-Wint-conversion]                                                                                                                                                                         
    char *p = strerror_r (0, buf, sizeof buf);                                                                                                                                                                                                                                  
              ^~~~~~~~~~                                                                                                                                                                                                                                                        
configure:23043: $? = 1                                                                                                                                                                                                                                                         
configure: failed program was: 
<snip>
| int                                                                                                                                                                                                                                                                           
| main ()                                                                                                                                                                                                                                                                       
| {                                                                                                                                                                                                                                                                             
|                                                                                                                                                                                                                                                                               
|         char buf[100];                                                                                                                                                                                                                                                        
|         char x = *strerror_r (0, buf, sizeof buf);                                                                                                                                                                                                                            
|         char *p = strerror_r (0, buf, sizeof buf);                                                                                                                                                                                                                            
|         return !p || x;                                                                                                                                                                                                                                                       
|                                                                                                                                                                                                                                                                               
|   ;                                                                                                                                                                                                                                                                           
|   return 0;                                                                                                                                                                                                                                                                   
| }                                                                                                                                                                                                                                                                             
configure:23081: result: no 

Current Behavior

SIGSEGV 😢

Expected Behavior

No crash 😄

On which platforms did you notice this

  • macOS
  • Linux
  • Windows

Versions Used:
5.10.0.179
5.12.0.226

Stacktrace

  at <unknown> <0xffffffff>
  at (wrapper managed-to-native) Mono.Unix.Native.Syscall.sys_strerror_r (int,System.Text.StringBuilder,ulong) [0x0000b] in <5a0e8d31304747319302afc0a1a098f1>:0
  at Mono.Unix.Native.Syscall.strerror_r (Mono.Unix.Native.Errno,System.Text.StringBuilder,ulong) [0x00006] in <5a0e8d31304747319302afc0a1a098f1>:0
  at Mono.Unix.Native.Syscall.strerror_r (Mono.Unix.Native.Errno,System.Text.StringBuilder) [0x00008] in <5a0e8d31304747319302afc0a1a098f1>:0
  at Mono.Unix.ErrorMarshal.strerror_r (Mono.Unix.Native.Errno) [0x00018] in <5a0e8d31304747319302afc0a1a098f1>:0
  at Mono.Unix.ErrorMarshal..cctor () [0x00011] in <5a0e8d31304747319302afc0a1a098f1>:0
  at (wrapper runtime-invoke) object.runtime_invoke_void (object,intptr,intptr,intptr) [0x0001e] in <49fde7938a9b47b480042d844e818c7e>:0
  at <unknown> <0xffffffff>
  at (wrapper managed-to-native) object.__icall_wrapper_mono_generic_class_init (intptr) [0x00000] in <49fde7938a9b47b480042d844e818c7e>:0
  at Mono.Unix.UnixMarshal.GetErrorDescription (Mono.Unix.Native.Errno) [0x00000] in <5a0e8d31304747319302afc0a1a098f1>:0
  at Mono.Unix.UnixTest.Main () [0x00001] in <62c493552c5b408d9336108f22958808>:0
  at (wrapper runtime-invoke) object.runtime_invoke_void (object,intptr,intptr,intptr) [0x0004c] in <49fde7938a9b47b480042d844e818c7e>:0
/proc/self/maps:
...
@frebib

This comment has been minimized.

Copy link
Contributor Author

@frebib frebib commented Jul 13, 2018

I have a feeling this could be caused by this unconditional definition:

mono/configure.ac

Lines 238 to 239 in 9ba3fa0

*-*-linux*)
CPPFLAGS="$CPPFLAGS -DGC_LINUX_THREADS -D_GNU_SOURCE -D_REENTRANT -DUSE_MMAP"

Shouldn't the build test that this is actually supported by the system instead of just assuming it is?

@jaykrell

This comment has been minimized.

Copy link
Collaborator

@jaykrell jaykrell commented Jul 13, 2018

Imho we should icall into the runtime, and there compile/link against the C headers/libraries.
That is a little inefficient but guards against functions existing or not.

@jaykrell

This comment has been minimized.

Copy link
Collaborator

@jaykrell jaykrell commented Jul 13, 2018

I really don't like p/invoke against code/headers/libraries not closely associated with the p/invoker.
Another project used to p/invoke to stat -- a mess.

@frebib

This comment has been minimized.

Copy link
Contributor Author

@frebib frebib commented Jul 13, 2018

static ErrorMarshal ()
{
try {
Translate = new ErrorTranslator (strerror_r);
Translate (Native.Errno.ERANGE);
}
catch (EntryPointNotFoundException) {
Translate = new ErrorTranslator (strerror);
}

The runtime does actually check for this but the native source is the broken thing here

@frebib

This comment has been minimized.

Copy link
Contributor Author

@frebib frebib commented Jul 13, 2018

According to this (likely unreliable) post, using a more specific check macro is better.

--- support/errno.c
+++ support/errno.c
@@ -41,7 +41,7 @@
  * we assume that the XPG version is present.
  */
 
-#ifdef _GNU_SOURCE
+#if (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE
 #define mph_min(x,y) ((x) <= (y) ? (x) : (y))
 
 /* If you pass an invalid errno value to glibc 2.3.2's strerror_r, you get

I have just tried this patch and it seems to fix the issue, at least on this system. Thoughts?

@jaykrell

This comment has been minimized.

Copy link
Collaborator

@jaykrell jaykrell commented Jul 13, 2018

Would using strerror be so terrible? All these not-guaranteed thread-safe functions, are always thread-safe really. But not reentrant, granted.

@frebib

This comment has been minimized.

Copy link
Contributor Author

@frebib frebib commented Jul 13, 2018

No, I don't honestly think it really matters either way. But regardless of whichever it chooses to use, it should do it gracefully

@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Jul 23, 2018

@frebib please submit your patch as PR

monojenkins added a commit that referenced this issue Aug 31, 2018
[runtime] Use AC_FUNC_STRERROR_R() in Mono.Posix

Hopefully this is less of a disaster than #9691 

Inspired by 3b3b48d
Fixes #9532

Signed-off-by: Joe Groocock <me@frebib.net>



<!--
Thank you for your Pull Request!

If you are new to contributing to Mono, please try to do your best at conforming to our coding guidelines http://www.mono-project.com/community/contributing/coding-guidelines/ but don't worry if you get something wrong. One of the project members will help you to get things landed.

Does your pull request fix any of the existing issues? Please use the following format: Fixes #issue-number
-->
EgorBo added a commit to EgorBo/mono that referenced this issue Sep 10, 2018
Inspired by mono@3b3b48d
Fixes mono#9532

Signed-off-by: Joe Groocock <me@frebib.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.