fixeria has uploaded this change for review. ( 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(-)
git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/28/28728/1
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();
-
fixeria