Skip to content

Commit

Permalink
avoid the stack-smasher strcpy-alloca and minor cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@vtjnash
vtjnash committed Dec 19, 2014
1 parent 79d473c commit 605c363
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/dump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ const char * jl_get_system_image_cpu_target(const char *fname)
return NULL;

// First, get "sys" from "sys.ji"
char *fname_shlib = (char*)alloca(strlen(fname));
char *fname_shlib = (char*)alloca(strlen(fname)+1);
strcpy(fname_shlib, fname);
char *fname_shlib_dot = strrchr(fname_shlib, '.');
if (fname_shlib_dot != NULL)
Expand Down
10 changes: 4 additions & 6 deletions src/task.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ save_stack(jl_task_t *t)
{
if (t->state == done_sym || t->state == failed_sym)
return;
volatile int _x;
volatile char *_x;
size_t nb = (char*)jl_stackbase - (char*)&_x;
char *buf;
if (t->stkbuf == NULL || t->bufsz < nb) {
Expand Down Expand Up @@ -206,10 +206,8 @@ restore_stack(jl_task_t *t, jl_jmp_buf *where, char *p)
restore_stack(t, where, p);
}
jl_jmp_target = where;

if (t->stkbuf != NULL) {
memcpy(_x, t->stkbuf, t->ssize);
}
assert(t->stkbuf != NULL);
memcpy(_x, t->stkbuf, t->ssize);
jl_longjmp(*jl_jmp_target, 1);
}
#endif
Expand Down Expand Up @@ -757,7 +755,7 @@ void NORETURN throw_internal(jl_value_t *e)
JL_PRINTF(JL_STDERR, "fatal: error thrown and no exception handler available.\n");
jl_static_show(JL_STDERR, e);
JL_PRINTF(JL_STDERR, "\n");
exit(1);
jl_exit(1);
}
jl_current_task->exception = e;
finish_task(jl_current_task, e);
Expand Down

5 comments on commit 605c363

@staticfloat
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this, Jameson! I couldn't figure out where the crashes in #9380 were coming from. How did you track this one down?

@ivarne
Copy link
Member

@ivarne ivarne commented on 605c363 Dec 19, 2014

Choose a reason for hiding this comment

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

So if strlen(fname) % 8 == 0 you would write some zeros outside the reserved buffer?

I actually looked at that line, and thought it looked funny, and control checked to see that it was big enough, but ended up with the wrong result.

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

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

The OS X compilers have stack smashing protection builtin. I don't know why Debian doesn't enable them. It couldn't start and lldb directly blamed this function

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

So should this be cherry-picked into #9376? We might need several other marked-for-backporting commits to get it to apply cleanly?

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check that f129236 is sufficient for this same fix on release-0.3.

Please sign in to comment.