From c914000971063a4500cfb34f4141dfc3c943368d Mon Sep 17 00:00:00 2001 From: "[anp/hsw]" Date: Sat, 13 Mar 2021 18:43:15 +0700 Subject: Fix some errors and warnings found by cppcheck [accel-pppd/ctrl/ipoe/ipoe.c:4054]: (style) A pointer can not be negative so it is either pointless or an error to check if it is not. [accel-pppd/logs/log_syslog.c:148]: (error) Array 'facility_name[9]' accessed at index 35, which is out of bounds. [accel-pppd/lua/session.c:274]: (error) Common realloc mistake: 'mods' nulled but not freed upon failure [accel-pppd/extra/ippool.c:114]: (warning) %u in format string (no. 1) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:114]: (warning) %u in format string (no. 2) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:114]: (warning) %u in format string (no. 3) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:114]: (warning) %u in format string (no. 4) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:114]: (warning) %u in format string (no. 5) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:141]: (warning) %u in format string (no. 1) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:141]: (warning) %u in format string (no. 2) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:141]: (warning) %u in format string (no. 3) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:141]: (warning) %u in format string (no. 4) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/extra/ippool.c:141]: (warning) %u in format string (no. 5) requires 'unsigned int *' but the argument type is 'int *'. [accel-pppd/main.c:97]: (warning) %d in format string (no. 1) requires 'int *' but the argument type is 'unsigned int *'. [accel-pppd/radius/radius.c:687] -> [accel-pppd/radius/radius.c:690]: (warning) Possible null pointer dereference: rpd - otherwise it is redundant to check it against null. [accel-pppd/radius/serv.c:805] -> [accel-pppd/radius/serv.c:829]: (warning) Possible null pointer dereference: ptr2 - otherwise it is redundant to check it against null. [accel-pppd/radius/serv.c:813] -> [accel-pppd/radius/serv.c:829]: (warning) Possible null pointer dereference: ptr2 - otherwise it is redundant to check it against null. [accel-pppd/radius/serv.c:823] -> [accel-pppd/radius/serv.c:829]: (warning) Possible null pointer dereference: ptr2 - otherwise it is redundant to check it against null. --- accel-pppd/ctrl/ipoe/ipoe.c | 2 +- accel-pppd/extra/ippool.c | 6 ++++-- accel-pppd/logs/log_syslog.c | 8 +++++--- accel-pppd/lua/session.c | 12 +++++++++--- accel-pppd/main.c | 2 +- accel-pppd/radius/radius.c | 3 ++- accel-pppd/radius/serv.c | 5 +++-- 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/accel-pppd/ctrl/ipoe/ipoe.c b/accel-pppd/ctrl/ipoe/ipoe.c index d91596ae..0649e9c8 100644 --- a/accel-pppd/ctrl/ipoe/ipoe.c +++ b/accel-pppd/ctrl/ipoe/ipoe.c @@ -4051,7 +4051,7 @@ static void load_config(void) opt = conf_get_opt("ipoe", "check-ip"); if (!opt) opt = conf_get_opt("common", "check-ip"); - if (opt && opt >= 0) + if (opt) conf_check_exists = atoi(opt) > 0; #ifdef RADIUS diff --git a/accel-pppd/extra/ippool.c b/accel-pppd/extra/ippool.c index 5e0098ac..2ca8b6f6 100644 --- a/accel-pppd/extra/ippool.c +++ b/accel-pppd/extra/ippool.c @@ -109,7 +109,8 @@ static void parse_gw_ip_address(const char *val) //parses ranges like x.x.x.x/mask static int parse1(const char *str, uint32_t *begin, uint32_t *end) { - int n, f1, f2, f3, f4, m; + int n; + unsigned int f1, f2, f3, f4, m; n = sscanf(str, "%u.%u.%u.%u/%u",&f1, &f2, &f3, &f4, &m); if (n != 5) @@ -136,7 +137,8 @@ static int parse1(const char *str, uint32_t *begin, uint32_t *end) //parses ranges like x.x.x.x-y static int parse2(const char *str, uint32_t *begin, uint32_t *end) { - int n, f1, f2, f3, f4, f5; + int n; + unsigned int f1, f2, f3, f4, f5; n = sscanf(str, "%u.%u.%u.%u-%u",&f1, &f2, &f3, &f4, &f5); if (n != 5) diff --git a/accel-pppd/logs/log_syslog.c b/accel-pppd/logs/log_syslog.c index 562d895f..f06c1cc8 100644 --- a/accel-pppd/logs/log_syslog.c +++ b/accel-pppd/logs/log_syslog.c @@ -138,19 +138,21 @@ static void parse_opt(const char *opt, char **ident, int *facility) int n; const char *facility_name[] = {"daemon", "local0", "local1", "local2", "local3", "local4", "local5", "local6", "local7"}; const int facility_num[] = {LOG_DAEMON, LOG_LOCAL0, LOG_LOCAL1, LOG_LOCAL2, LOG_LOCAL3, LOG_LOCAL4, LOG_LOCAL5, LOG_LOCAL6, LOG_LOCAL7}; + const int facility_total = 9; ptr = strchr(str, ','); if (ptr) { *ptr = 0; n = strtol(ptr + 1, &endptr, 10); if (*endptr) { - for (n = 0; n < sizeof(facility_name); n++) { + for (n = 0; n < facility_total; n++) { if (!strcasecmp(ptr + 1, facility_name[n])) break; } - if (n == sizeof(facility_name)) + if (n == facility_total) { log_emerg("log_syslog: unknown facility name '%s'\n", ptr + 1); - else + *facility = LOG_DAEMON; + } else *facility = facility_num[n]; } else *facility = n; diff --git a/accel-pppd/lua/session.c b/accel-pppd/lua/session.c index bd98911b..131f02f5 100644 --- a/accel-pppd/lua/session.c +++ b/accel-pppd/lua/session.c @@ -268,10 +268,16 @@ static int session_module(lua_State *L) void __export lua_session_module_register(const struct lua_session_module *mod) { + char *mods_new; if (!mods) - mods = malloc(sizeof(void *)); + mods_new = malloc(sizeof(void *)); else - mods = realloc(mods, (mod_cnt + 1) * sizeof(void *)); + mods_new = realloc(mods, (mod_cnt + 1) * sizeof(void *)); - mods[mod_cnt++] = mod; + if (mods_new) { + mods = mods_new; + mods[mod_cnt++] = mod; + } else { + log_emerg("lua: out of memory\n"); + } } diff --git a/accel-pppd/main.c b/accel-pppd/main.c index cb7e1cda..4b52dfcc 100644 --- a/accel-pppd/main.c +++ b/accel-pppd/main.c @@ -94,7 +94,7 @@ static void change_limits(void) f = fopen("/proc/sys/fs/nr_open", "r"); if (f) { - fscanf(f, "%d", &nr_open); + fscanf(f, "%u", &nr_open); fclose(f); } diff --git a/accel-pppd/radius/radius.c b/accel-pppd/radius/radius.c index fd640913..5563c945 100644 --- a/accel-pppd/radius/radius.c +++ b/accel-pppd/radius/radius.c @@ -684,7 +684,6 @@ static void ses_finished(struct ap_session *ses) { struct radius_pd_t *rpd = find_pd(ses); struct ipv6db_addr_t *a; - struct framed_route *fr = rpd->fr; struct framed_ip6_route *fr6; if (!rpd) { @@ -692,6 +691,8 @@ static void ses_finished(struct ap_session *ses) abort(); } + struct framed_route *fr = rpd->fr; + pthread_rwlock_wrlock(&sessions_lock); pthread_mutex_lock(&rpd->lock); list_del(&rpd->entry); diff --git a/accel-pppd/radius/serv.c b/accel-pppd/radius/serv.c index d27d04f8..65af74d3 100644 --- a/accel-pppd/radius/serv.c +++ b/accel-pppd/radius/serv.c @@ -758,6 +758,8 @@ static int parse_server2(const char *_opt, struct rad_server_t *s) goto out; ptr2 = strchr(ptr1 + 1, ','); + if (!ptr2) + goto out; *ptr1 = 0; @@ -826,8 +828,7 @@ static int parse_server2(const char *_opt, struct rad_server_t *s) else s->backup = 0; - if (ptr2) - *ptr2 = 0; + *ptr2 = 0; s->secret = _strdup(ptr1 + 1); -- cgit v1.2.3 From 6c2fb7f07692ec8d9aad971b177dbb8718bf391a Mon Sep 17 00:00:00 2001 From: "[anp/hsw]" Date: Sat, 13 Mar 2021 20:22:05 +0700 Subject: Fix another errors found by cppcheck [accel-pppd/cli/tcp.c:305]: (error) Uninitialized variable: cln [accel-pppd/cli/telnet.c:642]: (error) Uninitialized variable: cln [accel-pppd/ctrl/l2tp/l2tp.c:4302]: (error) Uninitialized variable: msg_attr [accel-pppd/ctrl/l2tp/l2tp.c:4484]: (error) Uninitialized variable: msg_type [accel-pppd/ctrl/pppoe/disc.c:169]: (error) Uninitialized variable: n [accel-pppd/ctrl/pppoe/pppoe.c:1588]: (error) Uninitialized variable: pado --- accel-pppd/cli/tcp.c | 2 +- accel-pppd/cli/telnet.c | 2 +- accel-pppd/ctrl/l2tp/l2tp.c | 4 ++-- accel-pppd/ctrl/pppoe/disc.c | 2 +- accel-pppd/ctrl/pppoe/pppoe.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/accel-pppd/cli/tcp.c b/accel-pppd/cli/tcp.c index 270d8cbf..87633cbe 100644 --- a/accel-pppd/cli/tcp.c +++ b/accel-pppd/cli/tcp.c @@ -298,7 +298,7 @@ static int serv_read(struct triton_md_handler_t *h) static void serv_close(struct triton_context_t *ctx) { - struct tcp_client_t *cln; + struct tcp_client_t *cln = NULL; while (!list_empty(&clients)) { cln = list_entry(clients.next, typeof(*cln), entry); diff --git a/accel-pppd/cli/telnet.c b/accel-pppd/cli/telnet.c index 4b5f63ba..4ea18391 100644 --- a/accel-pppd/cli/telnet.c +++ b/accel-pppd/cli/telnet.c @@ -635,7 +635,7 @@ static int serv_read(struct triton_md_handler_t *h) } static void serv_close(struct triton_context_t *ctx) { - struct telnet_client_t *cln; + struct telnet_client_t *cln = NULL; while (!list_empty(&clients)) { cln = list_entry(clients.next, typeof(*cln), entry); diff --git a/accel-pppd/ctrl/l2tp/l2tp.c b/accel-pppd/ctrl/l2tp/l2tp.c index 2467189f..8567027d 100644 --- a/accel-pppd/ctrl/l2tp/l2tp.c +++ b/accel-pppd/ctrl/l2tp/l2tp.c @@ -4224,7 +4224,7 @@ static int l2tp_tunnel_store_msg(struct l2tp_conn_t *conn, static int l2tp_tunnel_reply(struct l2tp_conn_t *conn, int need_ack) { - const struct l2tp_attr_t *msg_attr; + const struct l2tp_attr_t *msg_attr = NULL; struct l2tp_packet_t *pack; struct l2tp_sess_t *sess; uint16_t msg_sid; @@ -4430,7 +4430,7 @@ static int l2tp_udp_read(struct triton_md_handler_t *h) { struct l2tp_serv_t *serv = container_of(h, typeof(*serv), hnd); struct l2tp_packet_t *pack; - const struct l2tp_attr_t *msg_type; + const struct l2tp_attr_t *msg_type = NULL; struct in_pktinfo pkt_info; char src_addr[17]; diff --git a/accel-pppd/ctrl/pppoe/disc.c b/accel-pppd/ctrl/pppoe/disc.c index a0a21751..b1119eb1 100644 --- a/accel-pppd/ctrl/pppoe/disc.c +++ b/accel-pppd/ctrl/pppoe/disc.c @@ -139,7 +139,7 @@ int pppoe_disc_start(struct pppoe_serv_t *serv) struct rb_node **p, *parent = NULL; struct tree *t; int ifindex = serv->ifindex, i; - struct pppoe_serv_t *n; + struct pppoe_serv_t *n = NULL; if (!net) { pthread_mutex_lock(&nets_lock); diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c index 045a638e..6d876495 100644 --- a/accel-pppd/ctrl/pppoe/pppoe.c +++ b/accel-pppd/ctrl/pppoe/pppoe.c @@ -1577,7 +1577,7 @@ void _server_stop(struct pppoe_serv_t *serv) void pppoe_server_free(struct pppoe_serv_t *serv) { - struct delayed_pado_t *pado; + struct delayed_pado_t *pado = NULL; pthread_rwlock_wrlock(&serv_lock); list_del(&serv->entry); -- cgit v1.2.3 From 39981480b61f4712cbd5ec415e851f1d52fb04bf Mon Sep 17 00:00:00 2001 From: "[anp/hsw]" Date: Sat, 13 Mar 2021 21:06:58 +0700 Subject: Fix segmentation fault when radius section is missing --- accel-pppd/radius/radius.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/accel-pppd/radius/radius.c b/accel-pppd/radius/radius.c index 5563c945..139b8b81 100644 --- a/accel-pppd/radius/radius.c +++ b/accel-pppd/radius/radius.c @@ -1041,6 +1041,10 @@ static void radius_init(void) { const char *dict = NULL; struct conf_sect_t *s = conf_get_section("radius"); + + if (!s) + _exit(EXIT_FAILURE); + struct conf_option_t *opt1; rpd_pool = mempool_create(sizeof(struct radius_pd_t)); -- cgit v1.2.3 From a0c08ce019cf88278f882d823f876c6edc2d5218 Mon Sep 17 00:00:00 2001 From: "[anp/hsw]" Date: Sun, 14 Mar 2021 22:33:10 +0700 Subject: Prevent memory corruption on config file reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting program: /usr/sbin/accel-pppd -c /etc/accel-ppp/accel-ppp.conf [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/libthread_db.so.1". [New Thread 0xb7ad9b40 (LWP 24563)] [New Thread 0xb72d8b40 (LWP 24566)] [New Thread 0xb6ad7b40 (LWP 24567)] [New Thread 0xb60ffb40 (LWP 24569)] [New Thread 0xb58feb40 (LWP 24570)] [New Thread 0xb50fdb40 (LWP 24572)] [New Thread 0xb48fcb40 (LWP 24573)] conf_file:/etc/accel-ppp/accel-ppp.conf:93: no section opened memory corruption: malloc(10) at /var/tmp/portage/net-dialup/accel-ppp-9999/work/accel-ppp-9999/accel-pppd/triton/conf_file.c:117 free at /var/tmp/portage/net-dialup/accel-ppp-9999/work/accel-ppp-9999/accel-pppd/triton/conf_file.c:193 *** Error in `/usr/sbin/accel-pppd': corrupted double-linked list: 0xb61018c8 *** Thread 3 "accel-pppd" received signal SIGABRT, Aborted. [Switching to Thread 0xb72d8b40 (LWP 24566)] 0xb7fdc428 in __kernel_vsyscall () (gdb) bt full No symbol table info available. No symbol table info available. No symbol table info available. at /var/tmp/portage/net-dialup/accel-ppp-9999/work/accel-ppp-9999/accel-pppd/memdebug.c:90 mem = 0xb61018d0 r = 0 ctx = {fname = 0xb7fda1c4 "D\036ПЁт!\020╤╓R\005─\\m\005──", file = 0xfa8c7f2b, line = 108205909, items = 0x0} sect = 0x8002f1bf r = -2147097804 sections_bak = {next = 0xb3d01554, prev = 0xb3d016ec} t = 0xb7ff2750 r = 4 set = {__val = {516, 0 }} sig = 10 need_free = 0 stack = 0x0 No symbol table info available. No symbol table info available. --- accel-pppd/triton/conf_file.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/accel-pppd/triton/conf_file.c b/accel-pppd/triton/conf_file.c index ebde6af2..e1d9650b 100644 --- a/accel-pppd/triton/conf_file.c +++ b/accel-pppd/triton/conf_file.c @@ -33,9 +33,6 @@ static int sect_add_item(struct conf_ctx *ctx, const char *name, char *val, char static struct conf_option_t *find_item(struct conf_sect_t *, const char *name); static int load_file(struct conf_ctx *ctx); -static char *buf; -static struct conf_sect_t *cur_sect; - static int __conf_load(struct conf_ctx *ctx, const char *fname) { struct conf_ctx ctx1; @@ -59,10 +56,15 @@ static int __conf_load(struct conf_ctx *ctx, const char *fname) static int load_file(struct conf_ctx *ctx) { - char *str, *str2, *raw; - int len; + char *str2, *raw; + char buf[1024] = {0}; + + static struct conf_sect_t *cur_sect = NULL; while(1) { + int len; + char *str; + if (!fgets(buf, 1024, ctx->file)) break; ctx->line++; @@ -93,13 +95,14 @@ static int load_file(struct conf_ctx *ctx) return -1; } + cur_sect = find_sect(str); + if (cur_sect && ctx->items != &cur_sect->items) { fprintf(stderr, "conf_file:%s:%i: cann't open section inside option\n", ctx->fname, ctx->line); return -1; } *str2 = 0; - cur_sect = find_sect(str); if (!cur_sect) cur_sect = create_sect(str); ctx->items = &cur_sect->items; @@ -184,14 +187,9 @@ int conf_load(const char *fname) } else fname = conf_fname; - buf = _malloc(1024); - - cur_sect = NULL; ctx.items = NULL; r = __conf_load(&ctx, fname); - _free(buf); - return r; } @@ -219,8 +217,6 @@ int conf_reload(const char *fname) list_splice_init(§ions, §ions_bak); - cur_sect = NULL; - r = conf_load(fname); if (r) -- cgit v1.2.3 From 2785d81ec848ebb920c8a612b4eae7876a619a18 Mon Sep 17 00:00:00 2001 From: "[anp/hsw]" Date: Tue, 16 Mar 2021 19:34:06 +0700 Subject: Move debug print to proper place --- accel-pppd/net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel-pppd/net.c b/accel-pppd/net.c index 762841f0..809dfaaa 100644 --- a/accel-pppd/net.c +++ b/accel-pppd/net.c @@ -285,10 +285,10 @@ static struct ap_net *alloc_net(const char *name) close(ns_fd); return NULL; } + log_debug("open ns %s\n", name); } else def_ns_fd = ns_fd = open("/proc/self/ns/net", O_RDONLY); - log_debug("open ns %s\n", name); #endif n = _malloc(sizeof(*n)); -- cgit v1.2.3