Neels Hofmeyr (2): fix gsn_restart file buffer overflow and missing path sep gsn_restart file: set umask back to original after write failure
gtp/gtp.c | 20 +++++++++----------- gtp/gtp.h | 1 - 2 files changed, 9 insertions(+), 12 deletions(-)
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. --- gtp/gtp.c | 18 ++++++++---------- gtp/gtp.h | 1 - 2 files changed, 8 insertions(+), 11 deletions(-)
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 @@ static void log_restart(struct gsn_t *gsn) 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 @@ static void log_restart(struct gsn_t *gsn) 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 */
An fopen("w") error used to omit the umask() call to reinstate the previous umask. Move the final umask() call to the bottom so that it is called in all code paths. --- gtp/gtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gtp/gtp.c b/gtp/gtp.c index 161a6f0..a46a76f 100644 --- a/gtp/gtp.c +++ b/gtp/gtp.c @@ -681,7 +681,6 @@ static void log_restart(struct gsn_t *gsn) goto free_filename; }
- umask(i); fprintf(f, "%d\n", gsn->restart_counter); close_file: if (fclose(f)) @@ -689,6 +688,7 @@ close_file: "fclose failed: Error = %s\n", strerror(errno)); free_filename: talloc_free(filename); + umask(i); }
int gtp_new(struct gsn_t **gsn, char *statedir, struct in_addr *listen,
Unrelated to the patch content, what would it take to incorporate OpenGGSN into the list of projects managed with gerrit?
01.10.2016 01:10, Neels Hofmeyr пишет:
Neels Hofmeyr (2): fix gsn_restart file buffer overflow and missing path sep gsn_restart file: set umask back to original after write failure
gtp/gtp.c | 20 +++++++++----------- gtp/gtp.h | 1 - 2 files changed, 9 insertions(+), 12 deletions(-)
On Sat, Oct 01, 2016 at 05:10:39PM +0200, Max wrote:
Unrelated to the patch content, what would it take to incorporate OpenGGSN into the list of projects managed with gerrit?
I would +1 to add all remaining osmocom gits to gerrit now. I just recently by accident pushed a refs/for/master branch into a repos that isn't on gerit :P
The repos replication to git.osmocom.org is the part that I don't know anything about; Holger? Harald?
~Neels
On 03 Oct 2016, at 15:51, Neels Hofmeyr nhofmeyr@sysmocom.de wrote:
On Sat, Oct 01, 2016 at 05:10:39PM +0200, Max wrote:
Unrelated to the patch content, what would it take to incorporate OpenGGSN into the list of projects managed with gerrit?
I would +1 to add all remaining osmocom gits to gerrit now. I just recently by accident pushed a refs/for/master branch into a repos that isn't on gerit :P
The repos replication to git.osmocom.org is the part that I don't know anything about; Holger? Harald?
1.) Disable push access for everyone and then allow jenkins 2.) git clone openggsn as gerrit user in the directory 3.) reload gerrit (flush caches or restart so it sees it)
should be close to the exact steps.
holger
We should probably not discuss further details on-list; this just to indicate that we'll probably move some repositories as soon as we've sorted out the details.
~Neels
osmocom-net-gprs@lists.osmocom.org
-
Holger Freyther -
Max -
Neels Hofmeyr