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

fix: Tighten Stack Usage #553

Merged
merged 2 commits into from
Jun 23, 2021
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
2 changes: 1 addition & 1 deletion src/modulefinder/sentry_modulefinder_apple.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ remove_image(const struct mach_header *mh, intptr_t UNUSED(vmaddr_slide))
goto done;
}

char ref_addr[100];
char ref_addr[32];
snprintf(ref_addr, sizeof(ref_addr), "0x%llx", (long long)info.dli_fbase);
sentry_value_t new_modules = sentry_value_new_list();

Expand Down
21 changes: 14 additions & 7 deletions src/modulefinder/sentry_modulefinder_linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,25 +435,32 @@ load_modules(sentry_value_t modules)
return;
}

// just read the whole map at once, maybe do it line-by-line as a followup…
char buf[4096];
// Read the whole map at once. Doing it line-by-line would be a good
// followup.
sentry_stringbuilder_t sb;
sentry__stringbuilder_init(&sb);
while (true) {
char *buf = sentry__stringbuilder_reserve(&sb, 4096);
if (!buf) {
sentry__stringbuilder_cleanup(&sb);
close(fd);
return;
}
ssize_t n = read(fd, buf, 4096);
if (n < 0 && (errno == EAGAIN || errno == EINTR)) {
continue;
} else if (n <= 0) {
break;
}
if (sentry__stringbuilder_append_buf(&sb, buf, n)) {
sentry__stringbuilder_cleanup(&sb);
close(fd);
return;
}
sentry__stringbuilder_set_len(&sb, sentry__stringbuilder_len(&sb) + n);
}
close(fd);

// ensure the buffer is zero terminated
if (sentry__stringbuilder_append(&sb, "")) {
sentry__stringbuilder_cleanup(&sb);
return;
}
char *contents = sentry__stringbuilder_into_string(&sb);
if (!contents) {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/sentry_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ void
sentry__jsonwriter_write_double(sentry_jsonwriter_t *jw, double val)
{
if (can_write_item(jw)) {
char buf[50];
char buf[24];
// The MAX_SAFE_INTEGER is 9007199254740991, which has 16 digits
int written = sentry__snprintf_c(buf, sizeof(buf), "%.16g", val);
// print `null` if we have printf issues or a non-finite double, which
Expand Down
4 changes: 2 additions & 2 deletions src/sentry_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ sentry__get_os_context(void)
}
ffi->dwFileFlags &= ffi->dwFileFlagsMask;

char buf[100];
char buf[32];
snprintf(buf, sizeof(buf), "%u.%u.%u", ffi->dwFileVersionMS >> 16,
ffi->dwFileVersionMS & 0xffff, ffi->dwFileVersionLS >> 16);

Expand Down Expand Up @@ -71,7 +71,7 @@ sentry__get_os_context(void)

sentry_value_set_by_key(os, "name", sentry_value_new_string("macOS"));

char buf[100];
char buf[32];
size_t buf_len = sizeof(buf);

if (sysctlbyname("kern.osproductversion", buf, &buf_len, NULL, 0) != 0) {
Expand Down
26 changes: 21 additions & 5 deletions src/sentry_string.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ sentry__stringbuilder_init(sentry_stringbuilder_t *sb)
sb->len = 0;
}

static int
append(sentry_stringbuilder_t *sb, const char *s, size_t len)
char *
sentry__stringbuilder_reserve(sentry_stringbuilder_t *sb, size_t len)
{
size_t needed = sb->len + len + 1;
size_t needed = sb->len + len;
if (!sb->buf || needed > sb->allocated) {
size_t new_alloc_size = sb->allocated;
if (new_alloc_size == 0) {
Expand All @@ -27,7 +27,7 @@ append(sentry_stringbuilder_t *sb, const char *s, size_t len)
}
char *new_buf = sentry_malloc(new_alloc_size);
if (!new_buf) {
return 1;
return NULL;
}
if (sb->buf) {
memcpy(new_buf, sb->buf, sb->allocated);
Expand All @@ -36,7 +36,17 @@ append(sentry_stringbuilder_t *sb, const char *s, size_t len)
sb->buf = new_buf;
sb->allocated = new_alloc_size;
}
memcpy(sb->buf + sb->len, s, len);
return &sb->buf[sb->len];
}

static int
append(sentry_stringbuilder_t *sb, const char *s, size_t len)
{
char *buf = sentry__stringbuilder_reserve(sb, len + 1);
if (!buf) {
return 1;
}
memcpy(buf, s, len);
sb->len += len;

// make sure we're always zero terminated
Expand Down Expand Up @@ -105,6 +115,12 @@ sentry__stringbuilder_len(const sentry_stringbuilder_t *sb)
return sb->len;
}

void
sentry__stringbuilder_set_len(sentry_stringbuilder_t *sb, size_t len)
{
sb->len = len;
}

char *
sentry__string_clone(const char *str)
{
Expand Down
12 changes: 12 additions & 0 deletions src/sentry_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ void sentry__stringbuilder_cleanup(sentry_stringbuilder_t *sb);
*/
size_t sentry__stringbuilder_len(const sentry_stringbuilder_t *sb);

/**
* Resizes the stringbuilder buffer to make sure there is at least `len` bytes
* available at the end, and returns a pointer *to the reservation*.
*/
char *sentry__stringbuilder_reserve(sentry_stringbuilder_t *sb, size_t len);

/**
* Sets the number of used bytes in the string builder, to be used together with
* `sentry__stringbuilder_reserve` to avoid copying from an intermediate buffer.
*/
void sentry__stringbuilder_set_len(sentry_stringbuilder_t *sb, size_t len);

/**
* Duplicates a zero terminated string.
*/
Expand Down
2 changes: 1 addition & 1 deletion src/sentry_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ sentry__dsn_get_minidump_url(const sentry_dsn_t *dsn)
char *
sentry__msec_time_to_iso8601(uint64_t time)
{
char buf[255];
char buf[64];
size_t buf_len = sizeof(buf);
time_t secs = time / 1000;
struct tm *tm;
Expand Down
4 changes: 2 additions & 2 deletions src/sentry_value.c
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ sentry__value_stringify(sentry_value_t value)
case SENTRY_VALUE_TYPE_STRING:
return sentry__string_clone(sentry_value_as_string(value));
default: {
char buf[50];
char buf[24];
size_t written = (size_t)sentry__snprintf_c(
buf, sizeof(buf), "%g", sentry_value_as_double(value));
if (written >= sizeof(buf)) {
Expand Down Expand Up @@ -934,7 +934,7 @@ sentry__value_new_string_from_wstr(const wchar_t *s)
sentry_value_t
sentry__value_new_addr(uint64_t addr)
{
char buf[100];
char buf[32];
size_t written = (size_t)snprintf(
buf, sizeof(buf), "0x%llx", (unsigned long long)addr);
if (written >= sizeof(buf)) {
Expand Down