From cb6a56fa0cb7215471ee8572a929ca4fc0510584 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Tue, 29 Oct 2024 08:39:02 +0000 Subject: [PATCH] =?UTF-8?q?quectel:=20fix=20crash=20when=20parsing=20+CTZU?= =?UTF-8?q?=3D=3F=20response?= The logic should consider that mm_parse_uint_list() may return NULL without setting the output GError, in the case where an empty string is given. This should ideally be changed in the mm_parse_uint_list() itself, but that approach is not trivial as the method is being used in multiple different places. This change exclusively tries to solve this crash. 0x000057435d315eca (ModemManager - mm-shared-quectel.c: 1060) ctzu_test_ready 0x00007eea3d019e86 (libgio-2.0.so.0 - gtask.c: 1309) g_task_return_now 0x00007eea3d018e50 (libgio-2.0.so.0 - gtask.c: 1378) g_task_return 0x000057435d268eab (ModemManager - mm-base-modem-at.c: 544) at_command_ready 0x00007eea3d019e86 (libgio-2.0.so.0 - gtask.c: 1309) g_task_return_now 0x00007eea3d018e50 (libgio-2.0.so.0 - gtask.c: 1378) g_task_return 0x000057435d318a18 (ModemManager - mm-port-serial-at.c: 350) serial_command_ready 0x00007eea3d019e86 (libgio-2.0.so.0 - gtask.c: 1309) g_task_return_now 0x00007eea3d018e50 (libgio-2.0.so.0 - gtask.c: 1378) g_task_return 0x000057435d31c7bd (ModemManager - mm-port-serial.c: 734) port_serial_got_response 0x000057435d31cc3e (ModemManager - mm-port-serial.c: 911) parse_response_buffer 0x000057435d31cc3e (ModemManager - mm-port-serial.c: 1028) common_input_available 0x00007eea3d16591b (libglib-2.0.so.0 - gmain.c: 3460) g_main_dispatch 0x00007eea3d16591b (libglib-2.0.so.0 - gmain.c: 4200) g_main_context_dispatch 0x00007eea3d165c37 (libglib-2.0.so.0 - gmain.c: 4276) g_main_context_iterate 0x00007eea3d165eb5 (libglib-2.0.so.0 - gmain.c: 4479) g_main_loop_run 0x000057435d25f5bf (ModemManager - main.c: 236) main 0x00007eea3c980705 (libc.so.6 - libc_start_call_main.h: 58) __libc_start_call_main 0x00007eea3c9807c1 (libc.so.6 - libc-start.c: 360) __libc_start_main_impl 0x000057435d25ef10 (ModemManager + 0x000b3f10) _start 0x00007ffcc0b5d517 --- .../quectel/mm-modem-helpers-quectel.c | 16 ++++++---- .../tests/test-modem-helpers-quectel.c | 29 +++++++++++++------ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/plugins/quectel/mm-modem-helpers-quectel.c b/src/plugins/quectel/mm-modem-helpers-quectel.c index 7b0fe96d..126642ee 100644 --- a/src/plugins/quectel/mm-modem-helpers-quectel.c +++ b/src/plugins/quectel/mm-modem-helpers-quectel.c @@ -31,9 +31,10 @@ mm_quectel_parse_ctzu_test_response (const gchar *response, gboolean *supports_enable_update_rtc, GError **error) { - g_auto(GStrv) split = NULL; - g_autoptr(GArray) modes = NULL; - guint i; + g_auto(GStrv) split = NULL; + g_autoptr(GArray) modes = NULL; + GError *inner_error = NULL; + guint i; /* * Response may be: @@ -57,9 +58,14 @@ mm_quectel_parse_ctzu_test_response (const gchar *response, return FALSE; } - modes = mm_parse_uint_list (split[0], error); + modes = mm_parse_uint_list (split[0], &inner_error); + if (inner_error) { + g_propagate_prefixed_error (error, inner_error, "Failed to parse integer list in +CTZU test response: "); + return FALSE; + } if (!modes) { - g_prefix_error (error, "Failed to parse integer list in +CTZU test response: "); + g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, + "Unexpected empty integer list in +CTZU test response: "); return FALSE; } diff --git a/src/plugins/quectel/tests/test-modem-helpers-quectel.c b/src/plugins/quectel/tests/test-modem-helpers-quectel.c index 6c695f00..43990e01 100644 --- a/src/plugins/quectel/tests/test-modem-helpers-quectel.c +++ b/src/plugins/quectel/tests/test-modem-helpers-quectel.c @@ -34,18 +34,23 @@ typedef struct { gboolean expect_supports_disable; gboolean expect_supports_enable; gboolean expect_supports_enable_update_rtc; + gboolean expect_error; } TestCtzuResponse; static const TestCtzuResponse test_ctzu_response[] = { - { "+CTZU: (0,1)", TRUE, TRUE, FALSE }, - { "+CTZU: (0,1,3)", TRUE, TRUE, TRUE }, + { "+CTZU: ", FALSE, FALSE, FALSE, TRUE }, + { "+CTZU: ()", FALSE, FALSE, FALSE, TRUE }, + { "+CTZU: (,)", FALSE, FALSE, FALSE, TRUE }, + { "+CTZU: (0,1)", TRUE, TRUE, FALSE, FALSE }, + { "+CTZU: (0,1,3)", TRUE, TRUE, TRUE, FALSE }, }; static void common_test_ctzu (const gchar *response, gboolean expect_supports_disable, gboolean expect_supports_enable, - gboolean expect_supports_enable_update_rtc) + gboolean expect_supports_enable_update_rtc, + gboolean expect_error) { g_autoptr(GError) error = NULL; gboolean res; @@ -59,12 +64,17 @@ common_test_ctzu (const gchar *response, &supports_enable, &supports_enable_update_rtc, &error); - g_assert_no_error (error); - g_assert (res); + if (expect_error) { + g_assert (error); + g_assert (!res); + } else { + g_assert_no_error (error); + g_assert (res); - g_assert_cmpuint (expect_supports_disable, ==, supports_disable); - g_assert_cmpuint (expect_supports_enable, ==, supports_enable); - g_assert_cmpuint (expect_supports_enable_update_rtc, ==, supports_enable_update_rtc); + g_assert_cmpuint (expect_supports_disable, ==, supports_disable); + g_assert_cmpuint (expect_supports_enable, ==, supports_enable); + g_assert_cmpuint (expect_supports_enable_update_rtc, ==, supports_enable_update_rtc); + } } static void @@ -76,7 +86,8 @@ test_ctzu (void) common_test_ctzu (test_ctzu_response[i].response, test_ctzu_response[i].expect_supports_disable, test_ctzu_response[i].expect_supports_enable, - test_ctzu_response[i].expect_supports_enable_update_rtc); + test_ctzu_response[i].expect_supports_enable_update_rtc, + test_ctzu_response[i].expect_error); } /*****************************************************************************/