fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/28728 ) Change subject: trxcon: allocate memory in l1ctl_server_start() ...................................................................... trxcon: allocate memory in l1ctl_server_start() Calling l1ctl_server_shutdown() whenever the server is not initialized will result in accessing uninitialized values. This can happen if we goto exit before l1ctl_server_start() was called. Initializing the server with zeroes is not an option, because we need to initilize llist_head and osmo_fd structures with proper values. Allocate and initialize struct l1ctl_server in l1ctl_server_start(), deinitialize and free() in l1ctl_server_shutdown(). Change-Id: Idf13914fd0b0ae09b2ce5ece1f4203ebcae05d6e Related: CID#275254 --- M src/host/trxcon/include/osmocom/bb/trxcon/l1ctl_server.h M src/host/trxcon/src/l1ctl_server.c M src/host/trxcon/src/trxcon.c 3 files changed, 16 insertions(+), 13 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/src/host/trxcon/include/osmocom/bb/trxcon/l1ctl_server.h b/src/host/trxcon/include/osmocom/bb/trxcon/l1ctl_server.h index 84420f2..003d146 100644 --- a/src/host/trxcon/include/osmocom/bb/trxcon/l1ctl_server.h +++ b/src/host/trxcon/include/osmocom/bb/trxcon/l1ctl_server.h @@ -24,8 +24,6 @@ struct l1ctl_server_cfg { /* UNIX socket path to listen on */ const char *sock_path; - /* talloc context to be used for new clients */ - void *talloc_ctx; /* maximum number of connected clients */ unsigned int num_clients_max; /* functions to be called on various events */ @@ -56,8 +54,7 @@ void *priv; }; -int l1ctl_server_start(struct l1ctl_server *server, - const struct l1ctl_server_cfg *cfg); +struct l1ctl_server *l1ctl_server_start(void *ctx, const struct l1ctl_server_cfg *cfg); void l1ctl_server_shutdown(struct l1ctl_server *server); int l1ctl_client_send(struct l1ctl_client *client, struct msgb *msg); diff --git a/src/host/trxcon/src/l1ctl_server.c b/src/host/trxcon/src/l1ctl_server.c index 6047bed..8d57fff 100644 --- a/src/host/trxcon/src/l1ctl_server.c +++ b/src/host/trxcon/src/l1ctl_server.c @@ -128,7 +128,7 @@ return -ENOMEM; } - client = talloc_zero(server->cfg->talloc_ctx, struct l1ctl_client); + client = talloc_zero(server, struct l1ctl_client); if (client == NULL) { LOGP(DL1C, LOGL_ERROR, "Failed to allocate an L1CTL client\n"); close(client_fd); @@ -206,13 +206,16 @@ talloc_free(client); } -int l1ctl_server_start(struct l1ctl_server *server, - const struct l1ctl_server_cfg *cfg) +struct l1ctl_server *l1ctl_server_start(void *ctx, const struct l1ctl_server_cfg *cfg) { + struct l1ctl_server *server; int rc; LOGP(DL1C, LOGL_NOTICE, "Init L1CTL server (sock_path=%s)\n", cfg->sock_path); + server = talloc(ctx, struct l1ctl_server); + OSMO_ASSERT(server != NULL); + *server = (struct l1ctl_server) { .clients = LLIST_HEAD_INIT(server->clients), .cfg = cfg, @@ -230,10 +233,10 @@ LOGP(DL1C, LOGL_ERROR, "Could not create UNIX socket: %s\n", strerror(errno)); talloc_free(server); - return rc; + return NULL; } - return 0; + return server; } void l1ctl_server_shutdown(struct l1ctl_server *server) @@ -254,4 +257,6 @@ close(server->ofd.fd); server->ofd.fd = -1; } + + talloc_free(server); } diff --git a/src/host/trxcon/src/trxcon.c b/src/host/trxcon/src/trxcon.c index 9f672e8..7fa774d 100644 --- a/src/host/trxcon/src/trxcon.c +++ b/src/host/trxcon/src/trxcon.c @@ -508,7 +508,7 @@ int main(int argc, char **argv) { struct l1ctl_server_cfg server_cfg; - struct l1ctl_server server; + struct l1ctl_server *server = NULL; int rc = 0; printf("%s", COPYRIGHT); @@ -557,14 +557,14 @@ /* Start the L1CTL server */ server_cfg = (struct l1ctl_server_cfg) { .sock_path = app_data.bind_socket, - .talloc_ctx = tall_trxcon_ctx, .num_clients_max = 1, /* only one connection for now */ .conn_read_cb = &l1ctl_rx_cb, .conn_accept_cb = &l1ctl_conn_accept_cb, .conn_close_cb = &l1ctl_conn_close_cb, }; - if (l1ctl_server_start(&server, &server_cfg) != 0) { + server = l1ctl_server_start(tall_trxcon_ctx, &server_cfg); + if (server == NULL) { rc = EXIT_FAILURE; goto exit; } @@ -586,7 +586,8 @@ osmo_select_main(0); exit: - l1ctl_server_shutdown(&server); + if (server != NULL) + l1ctl_server_shutdown(server); /* Deinitialize logging */ log_fini(); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/28728 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idf13914fd0b0ae09b2ce5ece1f4203ebcae05d6e Gerrit-Change-Number: 28728 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged