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

jq segfaults when built with uclibc #2003

Open
nkulikov opened this issue Oct 29, 2019 · 6 comments
Open

jq segfaults when built with uclibc #2003

nkulikov opened this issue Oct 29, 2019 · 6 comments
Labels

Comments

@nkulikov
Copy link

Describe the bug
jq built for MIPS by using gcc+uclibc toolchain segfaults on any input.

To Reproduce

$ echo '{"api_url": "https://test.com"}' | jq .api_url
Segmentation fault (core dumped)

Expected behavior
jq should not segfault on valid input.

Environment (please complete the following information):

  • custom built linux distro (gcc-4.9.4, uclibc 0.9.33.2)
  • jq version: master 37b2d21

Additional context
gdb backtrace:

(gdb) bt
#0  0x00000000 in ?? ()
#1  0x0041b7d0 in memory_exhausted () at src/jv_alloc.c:96
#2  0x0041b8c8 in jv_mem_calloc (nemb=12, sz=0) at src/jv_alloc.c:136
#3  0x0046af18 in block_compile (b=..., out=0x4f82b8, lf=0x4f9ce0, args=...)
    at src/compile.c:1380
#4  0x0040da20 in jq_compile_args (jq=0x4f82b0, str=0x7f810e0f ".api_url", 
    args=...) at src/execute.c:1176
#5  0x00406e84 in main (argc=2, argv=0x7f810324) at src/main.c:634

It seems uclibc returns null pointer when num or size argument is zero.

glibc in such situation returns not null pointer:

Even a request for zero bytes (i.e., malloc(0)) returns a pointer to something of the minimum allocatable size.

https://github.com/bminor/glibc/blob/master/malloc/malloc.c#L116

If size is zero, the behavior is implementation defined (null pointer may be returned, or some non-null pointer may be returned that may not be used to access storage)

https://en.cppreference.com/w/c/memory/calloc

@pkoppstein
Copy link
Contributor

Have you tried https://uclibc-ng.org/ ?

@nkulikov
Copy link
Author

Nope. We stick with old version where malloc behaviour in such case defined as:

void *malloc (size_t size)
{
...
#ifdef __MALLOC_GLIBC_COMPAT__
  if (unlikely (size == 0))
    size++;
#else
  /* Some programs will call malloc (0).  Lets be strict and return NULL */
  if (unlikely (size == 0))
    goto oom;
#endif
...

uclibc-ng upstream removed __MALLOC_GLIBC_COMPAT__ conditional branch and fallback to glibc compatible version:

...
  if (unlikely (size == 0))
    size++;
...

https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/libc/stdlib/malloc/malloc.c#n211

So uclibc-ng should work here.

Anyway bc->globals->cfunctions = jv_mem_calloc(sizeof(struct cfunction), ncfunc); code looks fragile because C standard (ISO/IEC 9899:201x) clearly states in 7.22.3.1:

If the size of the space requested is zero, the behaviour is implementation-defined: either a null pointer is returned, or the behavior is as if the size were some nonzero value, except that the returned pointer shall not be used to access an object.

Even in glibc compatible case returned "fake" pointer should not be used later. I didn't have time to deeply investigate jq library code, so I hope it conforms and does not use returned pointer later. :)

@nicowilliams
Copy link
Contributor

@nkulikov Can you confirm that the following patch fixes this:

diff --git a/src/jv_alloc.c b/src/jv_alloc.c
index 5fa6768..9fd7463 100644
--- a/src/jv_alloc.c
+++ b/src/jv_alloc.c
@@ -120,7 +120,7 @@ static void memory_exhausted() {

 void* jv_mem_alloc(size_t sz) {
   void* p = malloc(sz);
-  if (!p) {
+  if (!p && sz) {
     memory_exhausted();
   }
   return p;
@@ -132,7 +132,7 @@ void* jv_mem_alloc_unguarded(size_t sz) {

 void* jv_mem_calloc(size_t nemb, size_t sz) {
   void* p = calloc(nemb, sz);
-  if (!p) {
+  if (!p && sz) {
     memory_exhausted();
   }
   return p;
@@ -160,7 +160,7 @@ void jv_mem_free(void* p) {

 void* jv_mem_realloc(void* p, size_t sz) {
   p = realloc(p, sz);
-  if (!p) {
+  if (!p && sz) {
     memory_exhausted();
   }
   return p;

?

@nkulikov
Copy link
Author

nkulikov commented Jan 1, 2020

I can not check it right now, but will do it soon as I get access to hardware and environment. Thanks for your support!

@nicowilliams
Copy link
Contributor

@nkulikov it'd sure be nice if we could setup some Travis builds using musl and uclibc or uclibc-ng. Any ideas how to do that? For musl it seems easy enough, but for uclibc or uclibc-ng it looks like we have to use tarballs.

nicowilliams added a commit to nicowilliams/jq that referenced this issue Jan 2, 2020
nicowilliams added a commit to nicowilliams/jq that referenced this issue Jan 2, 2020
nicowilliams added a commit to nicowilliams/jq that referenced this issue Jan 3, 2020
nicowilliams added a commit to nicowilliams/jq that referenced this issue Jan 3, 2020
nicowilliams added a commit to nicowilliams/jq that referenced this issue Jan 3, 2020
nicowilliams added a commit to nicowilliams/jq that referenced this issue Jan 3, 2020
nicowilliams added a commit to nicowilliams/jq that referenced this issue Jan 3, 2020
ljfranklin added a commit to ljfranklin/buildroot that referenced this issue Jul 6, 2020
- Bumps jq package to latest to fix seg fault errors
- jqlang/jq#2003
- I saw this on ARM64 compiled with uclibc, issue mentions MIPS, so
  might only be an issue for non-x86

Signed-off-by: Lyle Franklin <lylejfranklin@gmail.com>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Jul 11, 2020
Bump jq package to latest to fix seg fault errors reported at
jqlang/jq#2003

Signed-off-by: Lyle Franklin <lylejfranklin@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
woodsts pushed a commit to woodsts/buildroot that referenced this issue Jul 21, 2020
Bump jq package to latest to fix seg fault errors reported at
jqlang/jq#2003

Signed-off-by: Lyle Franklin <lylejfranklin@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
(cherry picked from commit c947941)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
woodsts pushed a commit to woodsts/buildroot that referenced this issue Jul 21, 2020
Bump jq package to latest to fix seg fault errors reported at
jqlang/jq#2003

Signed-off-by: Lyle Franklin <lylejfranklin@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
(cherry picked from commit c947941)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
jezze pushed a commit to jezze/buildroot that referenced this issue Aug 7, 2020
Bump jq package to latest to fix seg fault errors reported at
jqlang/jq#2003

Signed-off-by: Lyle Franklin <lylejfranklin@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
awanga pushed a commit to awanga/alt-f-next that referenced this issue Oct 7, 2020
Bump jq package to latest to fix seg fault errors reported at
jqlang/jq#2003

Signed-off-by: Lyle Franklin <lylejfranklin@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
@nkulikov
Copy link
Author

Sorry for so looooong response :) I found a time to replace our hack with more recent version of jq from upstream. a17dd32 still had this issue.
As I see multiple projects switched to a17dd32 to workaround this(?) issue. Now, can anybody confirm that it helps with uclibc 0.9.33.2 (or maybe more recent version of uclibc)?

I didn't catch idea with proposed patch because these changes already applied to upstream a long time ago (8dca3ef?). Or maybe I miss something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants