*: don't pass pointers to a local variables to thread_add_*

We should never pass pointers to local variables to thread_add_* family.
When an event is executed, the library writes into this pointer, which
means it writes into some random memory on a stack.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This commit is contained in:
Igor Ryzhov 2021-10-07 15:53:10 +03:00
parent b7a88ee291
commit 7640e3c60b
4 changed files with 41 additions and 25 deletions

View File

@ -243,7 +243,6 @@ main(int argc, char *argv[])
int pipe_parent2ldpe[2], pipe_parent2ldpe_sync[2];
int pipe_parent2lde[2], pipe_parent2lde_sync[2];
char *ctl_sock_name;
struct thread *thread = NULL;
bool ctl_sock_used = false;
snprintf(ctl_sock_path, sizeof(ctl_sock_path), LDPD_SOCKET,
@ -393,7 +392,7 @@ main(int argc, char *argv[])
frr_config_fork();
/* apply configuration */
thread_add_event(master, ldp_config_fork_apply, NULL, 0, &thread);
thread_add_event(master, ldp_config_fork_apply, NULL, 0, NULL);
/* setup pipes to children */
if ((iev_ldpe = calloc(1, sizeof(struct imsgev))) == NULL ||

View File

@ -63,6 +63,8 @@ static int agentx_read(struct thread *t)
int flags, new_flags = 0;
int nonblock = false;
struct listnode *ln = THREAD_ARG(t);
struct thread **thr = listgetdata(ln);
XFREE(MTYPE_TMP, thr);
list_delete_node(events, ln);
/* fix for non blocking socket */
@ -109,7 +111,7 @@ static void agentx_events_update(void)
struct timeval timeout = {.tv_sec = 0, .tv_usec = 0};
fd_set fds;
struct listnode *ln;
struct thread *thr;
struct thread **thr;
int fd, thr_fd;
thread_cancel(&timeout_thr);
@ -125,7 +127,7 @@ static void agentx_events_update(void)
ln = listhead(events);
thr = ln ? listgetdata(ln) : NULL;
thr_fd = thr ? THREAD_FD(thr) : -1;
thr_fd = thr ? THREAD_FD(*thr) : -1;
/* "two-pointer" / two-list simultaneous iteration
* ln/thr/thr_fd point to the next existing event listener to hit while
@ -135,20 +137,21 @@ static void agentx_events_update(void)
if (thr_fd == fd) {
struct listnode *nextln = listnextnode(ln);
if (!FD_ISSET(fd, &fds)) {
thread_cancel(&thr);
thread_cancel(thr);
XFREE(MTYPE_TMP, thr);
list_delete_node(events, ln);
}
ln = nextln;
thr = ln ? listgetdata(ln) : NULL;
thr_fd = thr ? THREAD_FD(thr) : -1;
thr_fd = thr ? THREAD_FD(*thr) : -1;
}
/* need listener, but haven't hit one where it would be */
else if (FD_ISSET(fd, &fds)) {
struct listnode *newln;
thr = NULL;
thread_add_read(agentx_tm, agentx_read, NULL, fd, &thr);
thr = XCALLOC(MTYPE_TMP, sizeof(struct thread *));
thread_add_read(agentx_tm, agentx_read, NULL, fd, thr);
newln = listnode_add_before(events, ln, thr);
thr->arg = newln;
(*thr)->arg = newln;
}
}
@ -157,7 +160,8 @@ static void agentx_events_update(void)
while (ln) {
struct listnode *nextln = listnextnode(ln);
thr = listgetdata(ln);
thread_cancel(&thr);
thread_cancel(thr);
XFREE(MTYPE_TMP, thr);
list_delete_node(events, ln);
ln = nextln;
}

View File

@ -425,8 +425,7 @@ static int frr_confd_cdb_read_cb(struct thread *thread)
int *subp = NULL;
int reslen = 0;
thread = NULL;
thread_add_read(master, frr_confd_cdb_read_cb, NULL, fd, &thread);
thread_add_read(master, frr_confd_cdb_read_cb, NULL, fd, &t_cdb_sub);
if (cdb_read_subscription_socket2(fd, &cdb_ev, &flags, &subp, &reslen)
!= CONFD_OK) {
@ -1164,15 +1163,10 @@ exit:
}
static int frr_confd_dp_read(struct thread *thread)
static int frr_confd_dp_read(struct confd_daemon_ctx *dctx, int fd)
{
struct confd_daemon_ctx *dctx = THREAD_ARG(thread);
int fd = THREAD_FD(thread);
int ret;
thread = NULL;
thread_add_read(master, frr_confd_dp_read, dctx, fd, &thread);
ret = confd_fd_ready(dctx, fd);
if (ret == CONFD_EOF) {
flog_err_confd("confd_fd_ready");
@ -1187,6 +1181,26 @@ static int frr_confd_dp_read(struct thread *thread)
return 0;
}
static int frr_confd_dp_ctl_read(struct thread *thread)
{
struct confd_daemon_ctx *dctx = THREAD_ARG(thread);
int fd = THREAD_FD(thread);
thread_add_read(master, frr_confd_dp_ctl_read, dctx, fd, &t_dp_ctl);
frr_confd_dp_read(dctx, fd);
}
static int frr_confd_dp_worker_read(struct thread *thread)
{
struct confd_daemon_ctx *dctx = THREAD_ARG(thread);
int fd = THREAD_FD(thread);
thread_add_read(master, frr_confd_dp_worker_read, dctx, fd, &t_dp_worker);
frr_confd_dp_read(dctx, fd);
}
static int frr_confd_subscribe_state(const struct lysc_node *snode, void *arg)
{
struct nb_node *nb_node = snode->priv;
@ -1314,9 +1328,9 @@ static int frr_confd_init_dp(const char *program_name)
goto error;
}
thread_add_read(master, frr_confd_dp_read, dctx, dp_ctl_sock,
thread_add_read(master, frr_confd_dp_ctl_read, dctx, dp_ctl_sock,
&t_dp_ctl);
thread_add_read(master, frr_confd_dp_read, dctx, dp_worker_sock,
thread_add_read(master, frr_confd_dp_worker_read, dctx, dp_worker_sock,
&t_dp_worker);
return 0;

View File

@ -528,19 +528,18 @@ static int frr_sr_notification_send(const char *xpath, struct list *arguments)
static int frr_sr_read_cb(struct thread *thread)
{
sr_subscription_ctx_t *sr_subscription = THREAD_ARG(thread);
struct yang_module *module = THREAD_ARG(thread);
int fd = THREAD_FD(thread);
int ret;
ret = sr_process_events(sr_subscription, session, NULL);
ret = sr_process_events(module->sr_subscription, session, NULL);
if (ret != SR_ERR_OK) {
flog_err(EC_LIB_LIBSYSREPO, "%s: sr_fd_event_process(): %s",
__func__, sr_strerror(ret));
return -1;
}
thread = NULL;
thread_add_read(master, frr_sr_read_cb, sr_subscription, fd, &thread);
thread_add_read(master, frr_sr_read_cb, module, fd, &module->sr_thread);
return 0;
}
@ -703,7 +702,7 @@ static int frr_sr_init(void)
sr_strerror(ret));
goto cleanup;
}
thread_add_read(master, frr_sr_read_cb, module->sr_subscription,
thread_add_read(master, frr_sr_read_cb, module,
event_pipe, &module->sr_thread);
}