This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.orgReview at https://gerrit.osmocom.org/1013 fix gsn_restart file buffer overflow and missing path sep Fix errors during gsn_restart file path composition: - possible buffer overflow because the wrong remaining length was fed to strncat(). - missing path separator: put restart file in dir/gsn_restart instead of ../dirgsn_restart. This assumes that the path separator is '/'. Use talloc_asprintf() to fix all filename length problems and shorten the code. In order to free the allocated path, add a free_filename label, and jump there instead of returning from the fopen("w") failure branch. Also don't return from "fclose failed" branch in order to free the path, remove the if {} braces. Remove the NAMESIZE constant that is now unused. Change-Id: Idf0a64ff45720aa818f2f9de1e8ba2fe2c82631b --- M gtp/gtp.c M gtp/gtp.h 2 files changed, 8 insertions(+), 11 deletions(-) git pull ssh://gerrit.osmocom.org:29418/openggsn refs/changes/13/1013/1 diff --git a/gtp/gtp.c b/gtp/gtp.c index 12cb492..161a6f0 100644 --- a/gtp/gtp.c +++ b/gtp/gtp.c @@ -646,13 +646,11 @@ FILE *f; int i, rc; int counter = 0; - char filename[NAMESIZE]; - - filename[NAMESIZE - 1] = 0; /* No null term. guarantee by strncpy */ - strncpy(filename, gsn->statedir, NAMESIZE - 1); - strncat(filename, RESTART_FILE, NAMESIZE - 1 - sizeof(RESTART_FILE)); - + char *filename; + i = umask(022); + filename = talloc_asprintf(NULL, "%s/%s", gsn->statedir, RESTART_FILE); + OSMO_ASSERT(filename); /* We try to open file. On failure we will later try to create file */ if (!(f = fopen(filename, "r"))) { @@ -680,17 +678,17 @@ LOGP(DLGTP, LOGL_ERROR, "fopen(path=%s, mode=%s) failed: Error = %s\n", filename, "w", strerror(errno)); - return; + goto free_filename; } umask(i); fprintf(f, "%d\n", gsn->restart_counter); close_file: - if (fclose(f)) { + if (fclose(f)) LOGP(DLGTP, LOGL_ERROR, "fclose failed: Error = %s\n", strerror(errno)); - return; - } +free_filename: + talloc_free(filename); } int gtp_new(struct gsn_t **gsn, char *statedir, struct in_addr *listen, diff --git a/gtp/gtp.h b/gtp/gtp.h index 539e255..1434e1e 100644 --- a/gtp/gtp.h +++ b/gtp/gtp.h @@ -29,7 +29,6 @@ #define ERRMSG_SIZE 255 #define RESTART_FILE "gsn_restart" -#define NAMESIZE 1024 /* GTP version 1 extension header type definitions. */ #define GTP_EXT_PDCP_PDU 0xC0 /* PDCP PDU Number */ -- To view, visit https://gerrit.osmocom.org/1013 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idf0a64ff45720aa818f2f9de1e8ba2fe2c82631b Gerrit-PatchSet: 1 Gerrit-Project: openggsn Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>