From 591b03bd3cfd52b33a5e8512fef466494cf329f6 Mon Sep 17 00:00:00 2001 From: David Härdeman Date: Fri, 26 Jun 2020 16:09:04 +0200 Subject: Properly cleanup and free struct cfg and servers --- minecctl/minecctl-rcon.c | 19 +----- minecctl/minecctl.c | 168 +++++++++++++++++++++++++++++------------------ minecctl/minecctl.h | 10 +-- minecctl/misc.c | 73 +++++++++++--------- minecctl/misc.h | 2 + 5 files changed, 154 insertions(+), 118 deletions(-) (limited to 'minecctl') diff --git a/minecctl/minecctl-rcon.c b/minecctl/minecctl-rcon.c index 27c586c..96d3522 100644 --- a/minecctl/minecctl-rcon.c +++ b/minecctl/minecctl-rcon.c @@ -133,11 +133,8 @@ rcon_login(struct cfg *cfg, struct server *server) send_msg(fd, buf, sizeof(buf), RCON_PACKET_LOGIN, server->rcon_password, &rtype, &reply); - /* An rcon password isn't exactly super-secret, but can't hurt */ explicit_bzero(buf, sizeof(buf)); - explicit_bzero(server->rcon_password, strlen(server->rcon_password)); - xfree(server->rcon_password); - server->rcon_password = NULL; + free_password(&server->rcon_password); if (rtype == RCON_PACKET_LOGIN_OK) verbose("%s: login ok", server->name); @@ -236,20 +233,6 @@ get_one_status(int fd, char *buf, size_t len, const char *cmd, return false; } -static struct server * -get_default_server(struct cfg *cfg) -{ - struct server *server; - - server = list_first_entry_or_null(&cfg->servers, struct server, list); - if (!server) - die("No servers defined"); - - read_server_config(server); - - return server; -} - void do_status(struct cfg *cfg) { char buf[4096]; diff --git a/minecctl/minecctl.c b/minecctl/minecctl.c index 19bcc1e..90939ee 100644 --- a/minecctl/minecctl.c +++ b/minecctl/minecctl.c @@ -32,19 +32,20 @@ dump_config() info("Configuration"); info("============="); - info("password : %s", cfg->password); - info("cfgdir : %s", cfg->cfgdir); - info("addrstr : %s", cfg->addrstr); - info("cmdstr : %s", cfg->cmdstr); - info("cmd : %p", cfg->cmd); - info("force stop : %s", cfg->force_stop ? "yes" : "no"); + info("cfgdir : %s", cfg->cfgdir); + info("rcon_password : %s", cfg->rcon_password); + info("rcon_addrstr : %s", cfg->rcon_addrstr); + info("mc_addrstr : %s", cfg->mc_addrstr); + info("cmdstr : %s", cfg->cmdstr); + info("cmd : %p", cfg->cmd); + info("force stop : %s", cfg->force_stop ? "yes" : "no"); list_for_each_entry(server, &cfg->servers, list) { info(" * server"); - info(" name : %s", server->name); - info(" filename : %s", server->filename); - info(" rcon_password : %s", server->rcon_password); - info(" file_read : %s", server->file_read ? "yes" : "no"); + info(" name : %s", server->name); + info(" filename : %s", server->filename); + info(" rcon_password : %s", server->rcon_password); + info(" file_read : %s", server->file_read ? "yes" : "no"); list_for_each_entry(saddr, &server->rcon_addrs, list) info(" * rcon addr : %s", saddr->addrstr); list_for_each_entry(saddr, &server->mc_addrs, list) @@ -147,9 +148,9 @@ usage(bool no_error) info("Usage: %s [OPTION...] COMMAND\n" "\n" "Valid options:\n" - " -p, --password=PWD use PWD as the rcon password\n" + " -p, --rcon-password=PWD use PWD when connecting via rcon\n" " (or use environment variable RCON_PASSWORD)\n" - " -a, --rcon-address=ADDR connect to rcon at ADDR\n" + " -r, --rcon-address=ADDR connect to rcon at ADDR\n" " (or use environment variable RCON_ADDRESS)\n" " -m, --mc-address=ADDR connect to Minecraft server at ADDR\n" " (only relevant for some commands, can also\n" @@ -180,7 +181,7 @@ usage(bool no_error) } static struct server * -allocate_server() +server_new() { struct server *server; @@ -192,6 +193,28 @@ allocate_server() return server; } +static void +server_free(struct server *server) +{ + struct saddr *saddr, *tmp; + + xfree(server->name); + xfree(server->filename); + free_password(&server->rcon_password); + + list_for_each_entry_safe(saddr, tmp, &server->rcon_addrs, list) { + list_del(&saddr->list); + xfree(saddr); + } + + list_for_each_entry_safe(saddr, tmp, &server->mc_addrs, list) { + list_del(&saddr->list); + xfree(saddr); + } + + xfree(server); +} + static void get_servers() { @@ -211,7 +234,7 @@ get_servers() if (!is_valid_server_config_filename(dent, NULL)) continue; - server = allocate_server(); + server = server_new(); server->filename = xstrdup(dent->d_name); suffix = strrchr(dent->d_name, '.'); @@ -262,43 +285,41 @@ create_server_from_cmdline_args() { struct server *server; - if (!cfg->addrstr && !cfg->mcaddrstr) + if (!cfg->rcon_addrstr && !cfg->mc_addrstr) return false; - server = allocate_server(); + server = server_new(); - if (cfg->addrstr) { - if (!str_to_addrs(cfg->addrstr, &server->rcon_addrs)) + if (cfg->rcon_addrstr) { + if (!str_to_addrs(cfg->rcon_addrstr, &server->rcon_addrs)) goto error; - server->name = cfg->addrstr; - cfg->addrstr = NULL; + server->name = cfg->rcon_addrstr; + cfg->rcon_addrstr = NULL; } - if (cfg->mcaddrstr) { - if (!str_to_addrs(cfg->mcaddrstr, &server->mc_addrs)) + if (cfg->mc_addrstr) { + if (!str_to_addrs(cfg->mc_addrstr, &server->mc_addrs)) goto error; if (!server->name) - server->name = cfg->mcaddrstr; + server->name = cfg->mc_addrstr; else - xfree(cfg->mcaddrstr); + xfree(cfg->mc_addrstr); - cfg->mcaddrstr = NULL; + cfg->mc_addrstr = NULL; } - if (cfg->password) { - server->rcon_password = cfg->password; - cfg->password = NULL; + if (cfg->rcon_password) { + server->rcon_password = cfg->rcon_password; + cfg->rcon_password = NULL; } list_add(&server->list, &cfg->servers); return true; error: - /* FIXME: free addrs */ - xfree(server->name); - xfree(server); + server_free(server); return false; } @@ -307,12 +328,28 @@ do_list(struct cfg *cfg) { struct server *server; + /* server->filename check excludes servers created from cmdline */ list_for_each_entry(server, &cfg->servers, list) - info("%s", server->name); + if (server->filename) + info("%s", server->name); +} + +struct server * +get_default_server(struct cfg *cfg) +{ + struct server *server; + + server = list_first_entry_or_null(&cfg->servers, struct server, list); + if (!server) + die("No servers defined"); + + read_server_config(server); + + return server; } static void -set_server(const char *name) +set_default_server(const char *name) { struct server *server; @@ -332,12 +369,12 @@ set_server(const char *name) static inline void get_optional_server_arg(char * const **argv, bool more) { - if (!cfg->addrstr) { + if (!cfg->rcon_addrstr) { if (!**argv) { error("Missing arguments"); usage(false); } - set_server(**argv); + set_default_server(**argv); (*argv)++; } @@ -356,7 +393,7 @@ parse_command(char * const *argv) /* Shorthand for console if address is set */ if (!*argv) { - if (!cfg->addrstr) { + if (!cfg->rcon_addrstr) { error("Missing arguments"); usage(false); } @@ -457,17 +494,17 @@ parse_cmdline(int argc, char * const *argv) while (true) { int option_index = 0; static struct option long_options[] = { - { "password", required_argument, 0, 'p' }, - { "rcon-address", required_argument, 0, 'a' }, - { "mc-address", required_argument, 0, 'A' }, - { "cfgdir", required_argument, 0, 'c' }, - { "verbose", no_argument, 0, 'v' }, - { "force", no_argument, 0, 'f' }, - { "help", no_argument, 0, 'h' }, - { 0, 0, 0, 0 } + { "rcon-password", required_argument, 0, 'p' }, + { "rcon-address", required_argument, 0, 'r' }, + { "mc-address", required_argument, 0, 'm' }, + { "cfgdir", required_argument, 0, 'c' }, + { "verbose", no_argument, 0, 'v' }, + { "force", no_argument, 0, 'f' }, + { "help", no_argument, 0, 'h' }, + { 0, 0, 0, 0 } }; - c = getopt_long(argc, argv, ":p:a:m:c:fvh", + c = getopt_long(argc, argv, ":p:r:m:c:vfh", long_options, &option_index); if (c == -1) @@ -475,23 +512,23 @@ parse_cmdline(int argc, char * const *argv) switch (c) { case 'p': - cfg->password = xstrdup(optarg); + cfg->rcon_password = xstrdup(optarg); break; - case 'a': - cfg->addrstr = xstrdup(optarg); + case 'r': + cfg->rcon_addrstr = xstrdup(optarg); break; case 'm': - cfg->mcaddrstr = xstrdup(optarg); + cfg->mc_addrstr = xstrdup(optarg); break; case 'c': cfg->cfgdir = optarg; break; - case 'f': - cfg->force_stop = true; - break; case 'v': debug_mask |= DBG_VERBOSE; break; + case 'f': + cfg->force_stop = true; + break; case 'h': usage(true); default: @@ -500,22 +537,22 @@ parse_cmdline(int argc, char * const *argv) } } - if (!cfg->password) { + if (!cfg->rcon_password) { e = getenv("RCON_PASSWORD"); if (e) - cfg->password = xstrdup(e); + cfg->rcon_password = xstrdup(e); } - if (!cfg->addrstr) { + if (!cfg->rcon_addrstr) { e = getenv("RCON_ADDRESS"); if (e) - cfg->addrstr = xstrdup(e); + cfg->rcon_addrstr = xstrdup(e); } - if (!cfg->mcaddrstr) { + if (!cfg->mc_addrstr) { e = getenv("MC_ADDRESS"); if (e) - cfg->mcaddrstr = xstrdup(e); + cfg->mc_addrstr = xstrdup(e); } } @@ -523,6 +560,7 @@ int main(int argc, char **argv) { int rv = EXIT_FAILURE; + struct server *server, *tmp; debug_mask = DBG_ERROR | DBG_INFO; @@ -542,7 +580,7 @@ main(int argc, char **argv) goto out; } - if (cfg->addrstr || cfg->mcaddrstr) + if (cfg->rcon_addrstr || cfg->mc_addrstr) if (!create_server_from_cmdline_args()) goto out; dump_config(); @@ -553,12 +591,12 @@ main(int argc, char **argv) rv = EXIT_SUCCESS; out: - /* FIXME: Not enough cleanup */ - if (cfg->password) - explicit_bzero(cfg->password, strlen(cfg->password)); - xfree(cfg->password); - xfree(cfg->addrstr); - xfree(cfg->mcaddrstr); + list_for_each_entry_safe(server, tmp, &cfg->servers, list) + server_free(server); + free_password(&cfg->rcon_password); + xfree(cfg->rcon_addrstr); + xfree(cfg->mc_addrstr); + xfree(cfg->cmdstr); xfree(cfg); exit(rv); } diff --git a/minecctl/minecctl.h b/minecctl/minecctl.h index 604225d..9e3d79b 100644 --- a/minecctl/minecctl.h +++ b/minecctl/minecctl.h @@ -13,10 +13,10 @@ struct server { struct cfg { /* command line arguments */ - char *cfgdir; - char *password; - char *addrstr; - char *mcaddrstr; + const char *cfgdir; + char *rcon_password; + char *rcon_addrstr; + char *mc_addrstr; char *cmdstr; bool force_stop; @@ -27,5 +27,7 @@ struct cfg { void read_server_config(struct server *server); +struct server *get_default_server(struct cfg *cfg); + #endif diff --git a/minecctl/misc.c b/minecctl/misc.c index 13b4377..bb33161 100644 --- a/minecctl/misc.c +++ b/minecctl/misc.c @@ -61,6 +61,41 @@ strv_join(char * const *strv) return r; } +int +connect_any(struct list_head *addrs, bool may_fail) +{ + struct saddr *saddr; + bool connected = false; + int sfd; + + if (list_empty(addrs)) + die("No address to connect to"); + + list_for_each_entry(saddr, addrs, list) { + verbose("Attempting connection to %s", saddr->addrstr); + sfd = socket(saddr->storage.ss_family, SOCK_STREAM | SOCK_CLOEXEC, 0); + if (sfd < 0) + die("socket: %m"); + + socket_set_low_latency(sfd, true, true, true); + + if (connect(sfd, (struct sockaddr *)&saddr->storage, saddr->addrlen) < 0) { + close(sfd); + continue; + } + + connected = true; + break; + } + + if (!connected && may_fail) + return -1; + else if (!connected) + die("Failed to connect to remote host"); + + return sfd; +} + char * ask_password() { @@ -99,39 +134,15 @@ ask_password() return password; } -int -connect_any(struct list_head *addrs, bool may_fail) +void +free_password(char **password) { - struct saddr *saddr; - bool connected = false; - int sfd; - - if (list_empty(addrs)) - die("No address to connect to"); - - list_for_each_entry(saddr, addrs, list) { - verbose("Attempting connection to %s", saddr->addrstr); - sfd = socket(saddr->storage.ss_family, SOCK_STREAM | SOCK_CLOEXEC, 0); - if (sfd < 0) - die("socket: %m"); - - socket_set_low_latency(sfd, true, true, true); - - if (connect(sfd, (struct sockaddr *)&saddr->storage, saddr->addrlen) < 0) { - close(sfd); - continue; - } - - connected = true; - break; - } - - if (!connected && may_fail) - return -1; - else if (!connected) - die("Failed to connect to remote host"); + if (!password || !*password) + return; - return sfd; + explicit_bzero(*password, strlen(*password)); + xfree(*password); + *password = NULL; } void diff --git a/minecctl/misc.h b/minecctl/misc.h index ca36dab..155681e 100644 --- a/minecctl/misc.h +++ b/minecctl/misc.h @@ -11,4 +11,6 @@ int connect_any(struct list_head *addrs, bool may_fail); char *ask_password(); +void free_password(char **password); + #endif -- cgit v1.2.3