diff options
author | [anp/hsw] <sysop@880.ru> | 2021-03-20 21:14:59 +0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-20 19:14:59 +0500 |
commit | 62f7740033f05053a581e864742575a46ccc6da2 (patch) | |
tree | 4ba9bffb1b3ffe1028b1e150b4bd1fee8122097d /accel-pppd/radius | |
parent | 5c6cf3e627b95a76ccdd21b956a9e77f05f250d9 (diff) | |
download | accel-ppp-62f7740033f05053a581e864742575a46ccc6da2.tar.gz accel-ppp-62f7740033f05053a581e864742575a46ccc6da2.zip |
Fixed some errors found by valgrind and pvs-studio (#11)
* Fix errors found by valgrind
==12312== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
==12312== at 0x486CCF0: sendmsg (in /lib/libpthread-2.22.so)
==12312== by 0x12F57F: rtnl_talk (libnetlink.c:316)
==12312== by 0x132DA3: genl_resolve_mcg (genl.c:52)
==12312== by 0x484E1CB: init (vlan_mon.c:528)
==12312== by 0x484CDC0: vlan_mon_register_proto (vlan_mon.c:48)
==12312== by 0x510B763: load_vlan_mon (pppoe.c:1914)
==12312== by 0x510BFF2: load_config (pppoe.c:2064)
==12312== by 0x510C22A: pppoe_init (pppoe.c:2108)
==12312== by 0x483E9EB: triton_load_modules (triton.c:704)
==12312== by 0x1384B2: main (main.c:339)
==12312== Address 0xbedacdd8 is on thread 1's stack
==12312== in frame #2, created by genl_resolve_mcg (genl.c:23)
==12312== 15 bytes in 1 blocks are definitely lost in loss record 352 of 836
==12312== at 0x482A9A9: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12312== by 0x4B97524: strdup (in /lib/libc-2.22.so)
==12312== by 0x12C30C: init (telnet.c:769)
==12312== by 0x483E9EB: triton_load_modules (triton.c:704)
==12312== by 0x1384B2: main (main.c:339)
==12312==
==12312== 15 bytes in 1 blocks are definitely lost in loss record 353 of 836
==12312== at 0x482A9A9: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12312== by 0x4B97524: strdup (in /lib/libc-2.22.so)
==12312== by 0x12D60A: init (tcp.c:392)
==12312== by 0x483E9EB: triton_load_modules (triton.c:704)
==12312== by 0x1384B2: main (main.c:339)
* Fix another warnings by cppcheck
[accel-pppd/ctrl/ipoe/arp.c:256]: (error) Uninitialized variable: n
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 5) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 6) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 5) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 6) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 5) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 6) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/pppoe.c:738]: (warning) Possible null pointer dereference
[accel-pppd/ipv6/dhcpv6.c:911]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'int *'.
[accel-pppd/ipv6/dhcpv6.c:911]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ipv6/dhcpv6.c:911]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ipv6/dhcpv6.c:911]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/libnetlink/libnetlink.c:515]: (warning) Possible null pointer dereference
[accel-pppd/ppp/ipv6cp_opt_intfid.c:185]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ppp/ipv6cp_opt_intfid.c:298]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'int *'.
[accel-pppd/ppp/ipv6cp_opt_intfid.c:298]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ppp/ipv6cp_opt_intfid.c:298]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ppp/ipv6cp_opt_intfid.c:298]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
* Suppress compiler warnings
* Fix locking errors
/opt/pvs/accel-ppp/accel-pppd/extra/pppd_compat.c 279 warn V1020 The function exited without calling the 'sigchld_unlock' function. Check lines: 279, 249.
/opt/pvs/accel-ppp/accel-pppd/extra/pppd_compat.c 333 warn V1020 The function exited without calling the 'sigchld_unlock' function. Check lines: 333, 315.
/opt/pvs/accel-ppp/accel-pppd/extra/pppd_compat.c 422 warn V1020 The function exited without calling the 'sigchld_unlock' function. Check lines: 422, 372.
/opt/pvs/accel-ppp/accel-pppd/extra/pppd_compat.c 488 warn V1020 The function exited without calling the 'sigchld_unlock' function. Check lines: 488, 468.
/opt/pvs/accel-ppp/accel-pppd/triton/mempool.c 119 warn V1020 The function exited without calling the 'pthread_spin_unlock' function. Check lines: 119, 116.
* Fix array len errors
/opt/pvs/accel-ppp/accel-pppd/triton/conf_file.c 75 warn V557 Array underrun is possible. The value of 'len - 1' index could reach -1.
/opt/pvs/accel-ppp/accel-pppd/triton/conf_file.c 76 warn V557 Array underrun is possible. The value of '-- len' index could reach -1.
* Fix possible memory leaks
/opt/pvs/accel-ppp/accel-pppd/radius/radius.c 936 err V773 The function was exited without releasing the 'str' pointer. A memory leak is possible.
/opt/pvs/accel-ppp/accel-pppd/radius/serv.c 622 err V773 The function was exited without releasing the 'str' pointer. A memory leak is possible.
/opt/pvs/accel-ppp/accel-pppd/triton/conf_file.c 144 err V773 The function was exited without releasing the 'raw' pointer. A memory leak is possible.
* Fix unsafe code
/opt/pvs/accel-ppp/accel-pppd/cli/tcp.c 364 warn V1004 The 'host' pointer was used unsafely after it was verified against nullptr. Check lines: 338, 364.
/opt/pvs/accel-ppp/accel-pppd/cli/telnet.c 701 warn V1004 The 'host' pointer was used unsafely after it was verified against nullptr. Check lines: 675, 701.
/opt/pvs/accel-ppp/accel-pppd/extra/ippool.c 241 err V614 Potentially uninitialized pointer 'pos' used.
/opt/pvs/accel-ppp/accel-pppd/radius/dict.c 165 err V614 Uninitialized pointer 'parent_items' used.
* Remove duplicate code
/opt/pvs/accel-ppp/accel-pppd/radius/serv.c 202 warn V547 Expression 'ts.tv_sec < req->serv->fail_time' is always false.
* Fix treating signed bool variables as unsigned
* Add nullptr checking
/opt/pvs/accel-ppp/accel-pppd/ipv6/dhcpv6.c 886 err V595 The 'opt->val' pointer was utilized before it was verified against nullptr. Check lines: 886, 890.
/opt/pvs/accel-ppp/accel-pppd/ipv6/nd.c 479 err V595 The 'opt->val' pointer was utilized before it was verified against nullptr. Check lines: 479, 483.
/opt/pvs/accel-ppp/accel-pppd/radius/auth.c 152 err V595 The 'rpd->auth_ctx' pointer was utilized before it was verified against nullptr. Check lines: 152, 154.
/opt/pvs/accel-ppp/accel-pppd/triton/conf_file.c 114 err V595 The 'cur_sect' pointer was utilized before it was verified against nullptr. Check lines: 114, 117.
* Add logging of exit conditions
* Clarify calculation
[accel-pppd/ppp/ccp_mppe.c:281]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:282]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:283]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:284]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:285]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:286]: (style) Clarify calculation precedence for '&' and '?'.
[drivers/ipoe/ipoe.c:307]: (style) Clarify calculation precedence for '&' and '?'.
* Fix void calculations
[accel-pppd/ctrl/pppoe/disc.c:211]: (portability) 'pkt' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/ctrl/pptp/pptp.c:150]: (portability) 'buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/acct.c:37]: (portability) 'req.pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/auth.c:35]: (portability) 'req.pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/auth.c:79]: (portability) 'req.pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:43]: (portability) 'pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:47]: (portability) 'pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:57]: (portability) 'pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:65]: (portability) 'req.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:97]: (portability) 'req.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/serv.c:364]: (portability) 'req.pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/triton/mempool.c:115]: (portability) 'mmap_ptr' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/triton/mempool.c:122]: (portability) 'mmap_ptr' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/triton/mempool.c:276]: (portability) 'ptr' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
* Fix void part 2
[accel-pppd/ipv6/dhcpv6.c:844]: (portability) 'conf_dnssl' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/ipv6/nd.c:199]: (portability) '(void*)dnsslinfo' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/ipv6/nd.c:432]: (portability) 'conf_dnssl' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
* Fix null pointer dereference
[accel-pppd/ctrl/ipoe/ipoe.c:2048]: (warning) Possible null pointer dereference: eth
[accel-pppd/ctrl/ipoe/ipoe.c:2049]: (warning) Possible null pointer dereference: iph
* Remove redundant check
/opt/pvs/accel-ppp/accel-pppd/ctrl/l2tp/packet.c 656 warn V547 Expression 'attr->length <= 16' is always false.
* PR fixes
* PR fixes 2
Diffstat (limited to 'accel-pppd/radius')
-rw-r--r-- | accel-pppd/radius/auth.c | 4 | ||||
-rw-r--r-- | accel-pppd/radius/dict.c | 2 | ||||
-rw-r--r-- | accel-pppd/radius/radius.c | 25 | ||||
-rw-r--r-- | accel-pppd/radius/radius.h | 6 | ||||
-rw-r--r-- | accel-pppd/radius/radius_p.h | 12 | ||||
-rw-r--r-- | accel-pppd/radius/serv.c | 10 |
6 files changed, 38 insertions, 21 deletions
diff --git a/accel-pppd/radius/auth.c b/accel-pppd/radius/auth.c index 8665867c..10d0454c 100644 --- a/accel-pppd/radius/auth.c +++ b/accel-pppd/radius/auth.c @@ -147,9 +147,9 @@ static void rad_auth_finalize(struct radius_pd_t *rpd, int r) { hold_pd(rpd); - rpd->auth_ctx->cb(rpd->auth_ctx->cb_arg, r); - if (rpd->auth_ctx) { + rpd->auth_ctx->cb(rpd->auth_ctx->cb_arg, r); + if (r == PWDB_SUCCESS) { rpd->auth_reply = rpd->auth_ctx->req->reply; rpd->auth_ctx->req->reply = NULL; diff --git a/accel-pppd/radius/dict.c b/accel-pppd/radius/dict.c index cb1fb28b..4924c895 100644 --- a/accel-pppd/radius/dict.c +++ b/accel-pppd/radius/dict.c @@ -76,7 +76,7 @@ static int dict_load(const char *fname) struct rad_dict_value_t *val; struct rad_dict_vendor_t *vendor; struct list_head *items; - struct list_head *parent_items; + struct list_head *parent_items = NULL; f = fopen(fname, "r"); if (!f) { diff --git a/accel-pppd/radius/radius.c b/accel-pppd/radius/radius.c index 139b8b81..9137f610 100644 --- a/accel-pppd/radius/radius.c +++ b/accel-pppd/radius/radius.c @@ -930,15 +930,19 @@ static int parse_server(const char *opt, in_addr_t *addr, int *port, char **secr *p1 = 0; if (p2) *p2 = 0; - else + else { + _free(str); return -1; + } *addr = inet_addr(str); if (p1) { *port = atoi(p1 + 1); - if (*port <=0 ) + if (*port <=0) { + _free(str); return -1; + } } p1 = _strdup(p2 + 1); @@ -1042,28 +1046,35 @@ static void radius_init(void) const char *dict = NULL; struct conf_sect_t *s = conf_get_section("radius"); - if (!s) + if (!s) { + log_emerg("radius: config section not found\n"); _exit(EXIT_FAILURE); + } struct conf_option_t *opt1; rpd_pool = mempool_create(sizeof(struct radius_pd_t)); auth_ctx_pool = mempool_create(sizeof(struct radius_auth_ctx)); - if (load_config()) + if (load_config()) { + log_emerg("radius: config load failed\n"); _exit(EXIT_FAILURE); - + } list_for_each_entry(opt1, &s->items, entry) { if (strcmp(opt1->name, "dictionary") || !opt1->val) continue; dict = opt1->val; - if (rad_dict_load(dict)) + if (rad_dict_load(dict)) { + log_emerg("radius: dictionary load failed\n"); _exit(0); + } } - if (!dict && rad_dict_load(DICTIONARY)) + if (!dict && rad_dict_load(DICTIONARY)) { + log_emerg("radius: default dictionary load failed\n"); _exit(0); + } pwdb_register(&pwdb); ipdb_register(&ipdb); diff --git a/accel-pppd/radius/radius.h b/accel-pppd/radius/radius.h index 9fc8869d..6ca94fb0 100644 --- a/accel-pppd/radius/radius.h +++ b/accel-pppd/radius/radius.h @@ -80,7 +80,7 @@ struct rad_dict_attr_t const char *name; int id; int type:30; - int array:1; + unsigned int array:1; int size; struct list_head values; struct list_head tlv; @@ -94,7 +94,7 @@ struct rad_attr_t //struct rad_dict_value_t *val; int len; int cnt; - int alloc:1; + unsigned int alloc:1; void *raw; rad_value_t val; }; @@ -106,7 +106,7 @@ struct rad_packet_t int len; struct timespec tv; struct list_head attrs; - void *buf; + uint8_t *buf; }; struct rad_plugin_t diff --git a/accel-pppd/radius/radius_p.h b/accel-pppd/radius/radius_p.h index 0c185d28..14c1fb2f 100644 --- a/accel-pppd/radius/radius_p.h +++ b/accel-pppd/radius/radius_p.h @@ -47,10 +47,10 @@ struct radius_pd_t { struct ap_session *ses; pthread_mutex_t lock; int refs:8; - int authenticated:1; - int acct_started:1; - int ipv6_dp_assigned:1; - int ipv6_dp_sent:1; + unsigned int authenticated:1; + unsigned int acct_started:1; + unsigned int ipv6_dp_assigned:1; + unsigned int ipv6_dp_sent:1; struct rad_packet_t *auth_reply; @@ -101,8 +101,8 @@ struct rad_req_t { int prio; int try; - int active:1; - int async:1; + unsigned int active:1; + unsigned int async:1; time_t ts; diff --git a/accel-pppd/radius/serv.c b/accel-pppd/radius/serv.c index 65af74d3..6d1eb4c6 100644 --- a/accel-pppd/radius/serv.c +++ b/accel-pppd/radius/serv.c @@ -197,6 +197,8 @@ int rad_server_req_enter(struct rad_req_t *req) pthread_mutex_lock(&req->serv->lock); + clock_gettime(CLOCK_MONOTONIC, &ts); + if (ts.tv_sec < req->serv->fail_time) { pthread_mutex_unlock(&req->serv->lock); return -1; @@ -616,15 +618,19 @@ static int parse_server_old(const char *opt, in_addr_t *addr, int *port, char ** *p1 = 0; if (p2) *p2 = 0; - else + else { + _free(str); return -1; + } *addr = inet_addr(str); if (p1) { *port = atoi(p1 + 1); - if (*port <=0 ) + if (*port <=0) { + _free(str); return -1; + } } *secret = _strdup(p2 + 1); |