From 90a016a8f662710b3f72027abe4f671ac8f75675 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Sat, 12 Dec 2020 23:59:53 +0100 Subject: [PATCH 1/7] src: be more careful before dereferencing potentially NULL pointers --- src/at.c | 17 +++++++++++++---- src/gpio.c | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/at.c b/src/at.c index 742f50e..67c6d69 100644 --- a/src/at.c +++ b/src/at.c @@ -52,7 +52,7 @@ static int configure_serial(const char *tty) static gboolean send_at_command(struct EG25Manager *manager) { char command[256]; - struct AtCommand *at_cmd = g_list_nth_data(manager->at_cmds, 0); + struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL; if (at_cmd) { if (at_cmd->subcmd == NULL && at_cmd->value == NULL && at_cmd->expected == NULL) @@ -88,7 +88,10 @@ static gboolean send_at_command(struct EG25Manager *manager) static void next_at_command(struct EG25Manager *manager) { - struct AtCommand *at_cmd = g_list_nth_data(manager->at_cmds, 0); + struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL; + + if (!at_cmd) + return; if (at_cmd->cmd) g_free(at_cmd->cmd); @@ -106,7 +109,10 @@ static void next_at_command(struct EG25Manager *manager) static void retry_at_command(struct EG25Manager *manager) { - struct AtCommand *at_cmd = g_list_nth_data(manager->at_cmds, 0); + struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL; + + if (!at_cmd) + return; at_cmd->retries++; if (at_cmd->retries > 3) { @@ -119,7 +125,10 @@ static void retry_at_command(struct EG25Manager *manager) static void process_at_result(struct EG25Manager *manager, char *response) { - struct AtCommand *at_cmd = g_list_nth_data(manager->at_cmds, 0); + struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL; + + if (!at_cmd) + return; if (at_cmd->expected && !strstr(response, at_cmd->expected)) { if (at_cmd->value) diff --git a/src/gpio.c b/src/gpio.c index d22b30d..53d87d6 100644 --- a/src/gpio.c +++ b/src/gpio.c @@ -177,7 +177,7 @@ gboolean gpio_check_poweroff(struct EG25Manager *manager, gboolean keep_down) if (manager->gpio_in[GPIO_IN_STATUS] && gpiod_line_get_value(manager->gpio_in[GPIO_IN_STATUS]) == 1) { - if (keep_down) { + if (keep_down && manager->gpio_out[GPIO_OUT_RESET]) { // Asserting RESET line to prevent modem from rebooting gpiod_line_set_value(manager->gpio_out[GPIO_OUT_RESET], 1); } From 8c9a2b21f91901b1b0f8882124c52d642d850499 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Sun, 13 Dec 2020 00:35:17 +0100 Subject: [PATCH 2/7] gpio: exit if we can't request output GPIOs This means the system is in a very bad shape as no other software should make use of those, so exit the daemon (will be restarted by systemd). --- src/gpio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gpio.c b/src/gpio.c index 53d87d6..e91bb5c 100644 --- a/src/gpio.c +++ b/src/gpio.c @@ -131,13 +131,13 @@ int gpio_init(struct EG25Manager *manager) manager->gpio_out[i] = gpiod_chip_get_line(manager->gpiochip[chipidx], offset); if (!manager->gpio_out[i]) { - g_critical("Unable to get output GPIO %d", i); + g_error("Unable to get output GPIO %d", i); return 1; } ret = gpiod_line_request_output(manager->gpio_out[i], "eg25manager", 0); if (ret < 0) { - g_critical("Unable to request output GPIO %d", i); + g_error("Unable to request output GPIO %d", i); return 1; } } From dd904bc8c1cd912026e6c815eec35ef4878f2cc2 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Sun, 13 Dec 2020 00:37:48 +0100 Subject: [PATCH 3/7] at: get rid of compiler warnings --- src/at.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/at.c b/src/at.c index 67c6d69..e035d34 100644 --- a/src/at.c +++ b/src/at.c @@ -53,20 +53,23 @@ static gboolean send_at_command(struct EG25Manager *manager) { char command[256]; struct AtCommand *at_cmd = manager->at_cmds ? g_list_nth_data(manager->at_cmds, 0) : NULL; + int ret, len = 0; if (at_cmd) { if (at_cmd->subcmd == NULL && at_cmd->value == NULL && at_cmd->expected == NULL) - sprintf(command, "AT+%s\r\n", at_cmd->cmd); + len = sprintf(command, "AT+%s\r\n", at_cmd->cmd); else if (at_cmd->subcmd == NULL && at_cmd->value == NULL) - sprintf(command, "AT+%s?\r\n", at_cmd->cmd); + len = sprintf(command, "AT+%s?\r\n", at_cmd->cmd); else if (at_cmd->subcmd == NULL && at_cmd->value) - sprintf(command, "AT+%s=%s\r\n", at_cmd->cmd, at_cmd->value); + len = sprintf(command, "AT+%s=%s\r\n", at_cmd->cmd, at_cmd->value); else if (at_cmd->subcmd && at_cmd->value == NULL) - sprintf(command, "AT+%s=\"%s\"\r\n", at_cmd->cmd, at_cmd->subcmd); + len = sprintf(command, "AT+%s=\"%s\"\r\n", at_cmd->cmd, at_cmd->subcmd); else if (at_cmd->subcmd && at_cmd->value) - sprintf(command, "AT+%s=\"%s\",%s\r\n", at_cmd->cmd, at_cmd->subcmd, at_cmd->value); + len = sprintf(command, "AT+%s=\"%s\",%s\r\n", at_cmd->cmd, at_cmd->subcmd, at_cmd->value); - write(manager->at_fd, command, strlen(command)); + ret = write(manager->at_fd, command, len); + if (ret < len) + g_warning("Couldn't write full AT command: wrote %d/%d bytes", ret, len); g_message("Sending command: %s", g_strstrip(command)); } else if (manager->modem_state < EG25_STATE_CONFIGURED) { @@ -238,9 +241,9 @@ void at_sequence_configure(struct EG25Manager *manager) { /* * Default parameters in megi's driver which differ with our own: - * - urc/ri/* are always set the same way on both BH and CE - * - urc/ri/* pulse duration is 1 ms and urc/delay is 0 (no need to delay - * URCs if the pulse is that short) + * - urc/ri/x are always set the same way on both BH and CE + * - urc/ri/x pulse duration is 1 ms and urc/delay is 0 (no need to delay + * URCs if the pulse is that short, so this is expected) * - apready is disabled * * Parameters set in megi's kernel but not here: From b495d6c747b0d9c9626cd84826b91b787065ca07 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Sun, 13 Dec 2020 00:46:46 +0100 Subject: [PATCH 4/7] gpio: get rid of compiler warnings --- src/manager.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/manager.c b/src/manager.c index 739a8f0..bc90985 100644 --- a/src/manager.c +++ b/src/manager.c @@ -112,18 +112,23 @@ void modem_configure(struct EG25Manager *manager) void modem_reset(struct EG25Manager *manager) { - int fd; + int fd, ret, len = strlen(manager->modem_usb_id); fd = open("/sys/bus/usb/drivers/usb/unbind", O_WRONLY); if (fd < 0) goto error; - write(fd, manager->modem_usb_id, strlen(manager->modem_usb_id)); + ret = write(fd, manager->modem_usb_id, len); + if (ret < len) + g_warning("Couldn't unbind modem: wrote %d/%d bytes", ret, len); close(fd); fd = open("/sys/bus/usb/drivers/usb/bind", O_WRONLY); if (fd < 0) goto error; - write(fd, manager->modem_usb_id, strlen(manager->modem_usb_id)); + ret = write(fd, manager->modem_usb_id, len); + if (ret < len) + g_warning("Couldn't unbind modem: wrote %d/%d bytes", ret, len); + close(fd); return; From 9c9169a9727a658c2e8e911b14a35c1525e08ca6 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Sun, 13 Dec 2020 00:55:22 +0100 Subject: [PATCH 5/7] manager: get rid of compiler warnings --- src/manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/manager.c b/src/manager.c index bc90985..26e0d67 100644 --- a/src/manager.c +++ b/src/manager.c @@ -159,7 +159,7 @@ int main(int argc, char *argv[]) { struct EG25Manager manager; char compatible[32]; - int fd; + int fd, ret; memset(&manager, 0, sizeof(manager)); manager.at_fd = -1; @@ -172,8 +172,8 @@ int main(int argc, char *argv[]) g_critical("Unable to read 'compatible' string from device tree"); return 1; } - read(fd, compatible, sizeof(compatible)); - if (!strstr(compatible, "pine64,pinephone-1.2")) + ret = read(fd, compatible, sizeof(compatible)); + if (ret > 0 && !strstr(compatible, "pine64,pinephone-1.2")) manager.braveheart = TRUE; close(fd); From 1bb2f80feff7565608f642813fcc49763d1f9e5b Mon Sep 17 00:00:00 2001 From: fortysixandtwo Date: Sat, 12 Dec 2020 12:57:19 +0100 Subject: [PATCH 6/7] use g_autoptr for GError --- src/mm-iface.c | 2 +- src/suspend.c | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/mm-iface.c b/src/mm-iface.c index f6ec6eb..8f622e4 100644 --- a/src/mm-iface.c +++ b/src/mm-iface.c @@ -135,7 +135,7 @@ static void mm_manager_new_cb(GDBusConnection *connection, GAsyncResult *res, struct EG25Manager *manager) { - GError *error = NULL; + g_autoptr (GError) *error = NULL; manager->mm_manager = mm_manager_new_finish(res, &error); if (!manager->mm_manager) diff --git a/src/suspend.c b/src/suspend.c index 873a985..54c4b6a 100644 --- a/src/suspend.c +++ b/src/suspend.c @@ -42,14 +42,13 @@ static void inhibit_done(GObject *source, { GDBusProxy *suspend_proxy = G_DBUS_PROXY(source); struct EG25Manager *manager = user_data; - GError *error = NULL; + g_autoptr (GError) error = NULL; GVariant *res; GUnixFDList *fd_list; res = g_dbus_proxy_call_with_unix_fd_list_finish(suspend_proxy, &fd_list, result, &error); if (!res) { g_warning("inhibit failed: %s", error->message); - g_error_free(error); } else { if (!fd_list || g_unix_fd_list_get_length(fd_list) != 1) g_warning("didn't get a single fd back"); @@ -127,13 +126,12 @@ static void on_proxy_acquired(GObject *object, GAsyncResult *res, struct EG25Manager *manager) { - GError *error = NULL; + g_autoptr (GError) error = NULL; char *owner; manager->suspend_proxy = g_dbus_proxy_new_for_bus_finish(res, &error); if (!manager->suspend_proxy) { g_warning("failed to acquire logind proxy: %s", error->message); - g_clear_error(&error); return; } From aa85cd873cbbe3227ccb110da7f179db73f433e4 Mon Sep 17 00:00:00 2001 From: fortysixandtwo Date: Sun, 13 Dec 2020 16:54:10 +0100 Subject: [PATCH 7/7] mm-iface: fix GError pointer --- src/mm-iface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm-iface.c b/src/mm-iface.c index 8f622e4..0d7e5b6 100644 --- a/src/mm-iface.c +++ b/src/mm-iface.c @@ -135,7 +135,7 @@ static void mm_manager_new_cb(GDBusConnection *connection, GAsyncResult *res, struct EG25Manager *manager) { - g_autoptr (GError) *error = NULL; + g_autoptr (GError) error = NULL; manager->mm_manager = mm_manager_new_finish(res, &error); if (!manager->mm_manager)