From 506273e9a27223b6748b2517069770fc80d726ae Mon Sep 17 00:00:00 2001 From: Vincent Bernat Date: Sun, 13 Jan 2013 02:53:37 +0100 Subject: [PATCH] Small fixes detected by clang analyzer. - log: mark `fatal*()` function as noreturn - event: insert client in the list after its creation - lldpcli: avoid confusion by initializing cargc to 0 - lldpd: avoid ambiguous use of strlen() in initialization - lldp/edp: fix memory leaks - tokenizer: fix a memory leak in low memory condition - cdp: don't accept too short TLV for port description --- src/client/lldpcli.c | 4 ++-- src/client/tokenizer.c | 5 ++++- src/daemon/cdp.c | 4 ++++ src/daemon/edp.c | 24 ++++++++++++++---------- src/daemon/event.c | 2 +- src/daemon/lldp.c | 12 ++++++++---- src/daemon/lldpd.c | 2 +- src/log.h | 4 ++-- 8 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/client/lldpcli.c b/src/client/lldpcli.c index 631fcae1..91873178 100644 --- a/src/client/lldpcli.c +++ b/src/client/lldpcli.c @@ -278,8 +278,8 @@ main(int argc, char *argv[]) do { const char *line; - char **cargv; - int n, cargc; + char **cargv = NULL; + int n, cargc = 0; if (!is_lldpctl(NULL) && (optind >= argc)) { line = readline(prompt()); if (line == NULL) break; /* EOF */ diff --git a/src/client/tokenizer.c b/src/client/tokenizer.c index 784114c4..c1fd758c 100644 --- a/src/client/tokenizer.c +++ b/src/client/tokenizer.c @@ -90,7 +90,10 @@ tokenize_line(const char *line, int *argc, char ***argv) i++) if (input[2*i] != empty) word[j++] = input[2*i]; char **nargv = realloc(iargv, sizeof(char*) * (iargc + 1)); - if (!nargv) goto error; + if (!nargv) { + free(word); + goto error; + } nargv[iargc++] = word; iargv = nargv; wbegin = -1; diff --git a/src/daemon/cdp.c b/src/daemon/cdp.c index 0df5e6db..671d8d52 100644 --- a/src/daemon/cdp.c +++ b/src/daemon/cdp.c @@ -427,6 +427,10 @@ cdp_decode(struct lldpd *cfg, char *frame, int s, } break; case CDP_TLV_PORT: + if (tlv_len == 0) { + log_warn("cd[", "too short port description received"); + goto malformed; + } if ((port->p_descr = (char *)calloc(1, tlv_len + 1)) == NULL) { log_warn("cdp", "unable to allocate memory for port description"); goto malformed; diff --git a/src/daemon/edp.c b/src/daemon/edp.c index 69b5cb4c..063ed466 100644 --- a/src/daemon/edp.c +++ b/src/daemon/edp.c @@ -177,8 +177,11 @@ edp_send(struct lldpd *global, break; } - if ((state == 1) && (v == 0)) /* No VLAN, no need to send another TLV */ + if ((state == 1) && (v == 0)) { + /* No VLAN, no need to send another TLV */ + free(packet); break; + } #endif /* Null TLV */ @@ -410,26 +413,26 @@ edp_decode(struct lldpd *cfg, char *frame, int s, PEEK_DISCARD(4); /* Reserved */ PEEK_BYTES(&address, sizeof(address)); - if ((lvlan->v_name = (char *)calloc(1, - tlv_len + 1 - 12)) == NULL) { - log_warn("edp", "unable to allocate vlan name"); - free(lvlan); - goto malformed; - } - PEEK_BYTES(lvlan->v_name, tlv_len - 12); - if (address.s_addr != INADDR_ANY) { mgmt = lldpd_alloc_mgmt(LLDPD_AF_IPV4, &address, sizeof(struct in_addr), 0); if (mgmt == NULL) { - assert(errno == ENOMEM); log_warn("edp", "Out of memory"); goto malformed; } TAILQ_INSERT_TAIL(&chassis->c_mgmt, mgmt, m_entries); } + + if ((lvlan->v_name = (char *)calloc(1, + tlv_len + 1 - 12)) == NULL) { + log_warn("edp", "unable to allocate vlan name"); + goto malformed; + } + PEEK_BYTES(lvlan->v_name, tlv_len - 12); + TAILQ_INSERT_TAIL(&port->p_vlans, lvlan, v_entries); + lvlan = NULL; #endif gotvlans = 1; break; @@ -500,6 +503,7 @@ edp_decode(struct lldpd *cfg, char *frame, int s, return 1; malformed: + free(lvlan); lldpd_chassis_cleanup(chassis, 1); lldpd_port_cleanup(port, 1); free(port); diff --git a/src/daemon/event.c b/src/daemon/event.c index 23c27dda..8b05ff3f 100644 --- a/src/daemon/event.c +++ b/src/daemon/event.c @@ -359,6 +359,7 @@ levent_ctl_accept(evutil_socket_t fd, short what, void *arg) } client->cfg = cfg; evutil_make_socket_nonblocking(s); + TAILQ_INSERT_TAIL(&lldpd_clients, client, next); if ((client->bev = bufferevent_socket_new(cfg->g_base, s, BEV_OPT_CLOSE_ON_FREE)) == NULL) { log_warnx("event", "unable to allocate a new buffer event for new client"); @@ -370,7 +371,6 @@ levent_ctl_accept(evutil_socket_t fd, short what, void *arg) client); bufferevent_enable(client->bev, EV_READ | EV_WRITE); log_debug("event", "new client accepted"); - TAILQ_INSERT_TAIL(&lldpd_clients, client, next); return; accept_failed: levent_ctl_free_client(client); diff --git a/src/daemon/lldp.c b/src/daemon/lldp.c index b8d45979..fce84528 100644 --- a/src/daemon/lldp.c +++ b/src/daemon/lldp.c @@ -483,10 +483,10 @@ lldp_decode(struct lldpd *cfg, char *frame, int s, u_int8_t *pos, *tlv; char *b; #ifdef ENABLE_DOT1 - struct lldpd_vlan *vlan; + struct lldpd_vlan *vlan = NULL; int vlan_len; struct lldpd_ppvid *ppvid; - struct lldpd_pi *pi; + struct lldpd_pi *pi = NULL; #endif struct lldpd_mgmt *mgmt; int af; @@ -671,12 +671,12 @@ lldp_decode(struct lldpd *cfg, char *frame, int s, log_warn("lldp", "unable to alloc vlan name for " "tlv received on %s", hardware->h_ifname); - free(vlan); goto malformed; } PEEK_BYTES(vlan->v_name, vlan_len); TAILQ_INSERT_TAIL(&port->p_vlans, vlan, v_entries); + vlan = NULL; break; case LLDP_TLV_DOT1_PVID: CHECK_TLV_SIZE(6, "PVID"); @@ -726,12 +726,12 @@ lldp_decode(struct lldpd *cfg, char *frame, int s, log_warn("lldp", "unable to alloc pid name for " "tlv received on %s", hardware->h_ifname); - free(pi); goto malformed; } PEEK_BYTES(pi->p_pi, pi->p_pi_len); TAILQ_INSERT_TAIL(&port->p_pids, pi, p_entries); + pi = NULL; break; default: /* Unknown Dot1 TLV, ignore it */ @@ -1040,6 +1040,10 @@ lldp_decode(struct lldpd *cfg, char *frame, int s, *newport = port; return 1; malformed: +#ifdef ENABLE_DOT1 + free(vlan); + free(pi); +#endif lldpd_chassis_cleanup(chassis, 1); lldpd_port_cleanup(port, 1); free(port); diff --git a/src/daemon/lldpd.c b/src/daemon/lldpd.c index 3f7c012a..9833891f 100644 --- a/src/daemon/lldpd.c +++ b/src/daemon/lldpd.c @@ -1073,7 +1073,7 @@ lldpd_started_by_systemd() struct iovec iov = { .iov_base = "READY=1", - .iov_len = strlen(iov.iov_base) + .iov_len = strlen("READY=1") }; struct msghdr hdr = { .msg_name = &su, diff --git a/src/log.h b/src/log.h index 375d8ca0..94a122d0 100644 --- a/src/log.h +++ b/src/log.h @@ -24,8 +24,8 @@ void log_warn(const char *, const char *, ...) __attribute__ ((forma void log_warnx(const char *, const char *, ...) __attribute__ ((format (printf, 2, 3))); void log_info(const char *, const char *, ...) __attribute__ ((format (printf, 2, 3))); void log_debug(const char *, const char *, ...) __attribute__ ((format (printf, 2, 3))); -void fatal(const char*, const char *); -void fatalx(const char *); +void fatal(const char*, const char *) __attribute__((__noreturn__)); +void fatalx(const char *) __attribute__((__noreturn__)); void log_register(void (*cb)(int, const char*)); void log_accept(const char *); -- 2.39.5