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 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged
