Add patches for a couple of bugs that make static analysis unhappy

Real bugs, but no security implications. Let's check the boxes and fix
them, to reduce the static analysis tooling noise.

Resolves: https://issues.redhat.com/browse/RHEL-38991
This commit is contained in:
Lubomir Rintel 2024-06-25 10:49:26 +02:00
parent 96221f3d5a
commit 471d385cd1
12 changed files with 355 additions and 2 deletions

View File

@ -0,0 +1,29 @@
From 4c28ea13d9b1e75d0bc0dfa4c588b478635c12c0 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Thu, 23 May 2024 14:43:15 +0200
Subject: [PATCH 01/11] shared-qmi: do not leak a string in error path
---
src/mm-shared-qmi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mm-shared-qmi.c b/src/mm-shared-qmi.c
index 8da4ecc69..923c6b51a 100644
--- a/src/mm-shared-qmi.c
+++ b/src/mm-shared-qmi.c
@@ -4905,9 +4905,10 @@ loc_location_get_server_indication_cb (QmiClientLoc *client,
str = g_strdup ("");
out:
- if (error)
+ if (error) {
+ g_free (str);
g_task_return_error (task, error);
- else {
+ } else {
g_assert (str);
g_task_return_pointer (task, str, g_free);
}
--
2.45.2

View File

@ -0,0 +1,38 @@
From d550a24ffdae4492e9f9e18fdf294adcda28fb65 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Thu, 23 May 2024 14:45:22 +0200
Subject: [PATCH 02/11] ublox/helpers: fix propagation of +URAT response parse
error
We have been constructing a GError too late, just leaking it instead of
propagating.
---
src/plugins/ublox/mm-modem-helpers-ublox.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/plugins/ublox/mm-modem-helpers-ublox.c b/src/plugins/ublox/mm-modem-helpers-ublox.c
index 0fd1c5b0b..ffb1374de 100644
--- a/src/plugins/ublox/mm-modem-helpers-ublox.c
+++ b/src/plugins/ublox/mm-modem-helpers-ublox.c
@@ -1813,14 +1813,13 @@ mm_ublox_parse_urat_read_response (const gchar *response,
}
out:
- if (inner_error) {
- g_propagate_error (error, inner_error);
- return FALSE;
- }
-
if (allowed == MM_MODEM_MODE_NONE) {
inner_error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
"Couldn't parse +URAT response: %s", response);
+ }
+
+ if (inner_error) {
+ g_propagate_error (error, inner_error);
return FALSE;
}
--
2.45.2

View File

@ -0,0 +1,31 @@
From eebd31b92e4e60078911decd87a8c6e658d20cd1 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Thu, 23 May 2024 14:54:03 +0200
Subject: [PATCH 03/11] broadband-modem: fix a potential leak in
cusd_process_string() error handling
On error, *converted may already be allocated and we need to free it
(but not in case we're returning it from the task).
---
src/mm-broadband-modem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 41906c2e5..b924c3a96 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -6306,9 +6306,10 @@ out:
if (error)
g_task_return_error (task, error);
else if (converted)
- g_task_return_pointer (task, converted, g_free);
+ g_task_return_pointer (task, g_steal_pointer(&converted), g_free);
else
g_assert_not_reached ();
+ g_clear_pointer (&converted, g_free);
return;
}
--
2.45.2

View File

@ -0,0 +1,30 @@
From 4a3542b2d00c4104e9735af9eea906c703faed20 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Thu, 23 May 2024 14:56:40 +0200
Subject: [PATCH 04/11] shared-qmi: fix a leak in error handling path
str needs to be freed in pds_get_agps_config_ready().
---
src/mm-shared-qmi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mm-shared-qmi.c b/src/mm-shared-qmi.c
index 923c6b51a..789b92a4d 100644
--- a/src/mm-shared-qmi.c
+++ b/src/mm-shared-qmi.c
@@ -4795,9 +4795,10 @@ pds_get_agps_config_ready (QmiClientPds *client,
str = g_strdup ("");
out:
- if (error)
+ if (error) {
+ g_free (str);
g_task_return_error (task, error);
- else {
+ } else {
g_assert (str);
g_task_return_pointer (task, str, g_free);
}
--
2.45.2

View File

@ -0,0 +1,34 @@
From 84941606219fe440f59fbbc6057ab3782c09bba4 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Sun, 26 May 2024 23:42:41 +0200
Subject: [PATCH] quectel/shared: do not leak name string
---
src/plugins/quectel/mm-shared-quectel.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/plugins/quectel/mm-shared-quectel.c b/src/plugins/quectel/mm-shared-quectel.c
index 816a570..acd7c3c 100644
--- a/src/plugins/quectel/mm-shared-quectel.c
+++ b/src/plugins/quectel/mm-shared-quectel.c
@@ -285,8 +285,8 @@ quectel_at_port_get_firmware_revision_ready (MMBaseModem *self,
MMFirmwareUpdateSettings *update_settings;
MMModemFirmwareUpdateMethod update_methods;
const gchar *revision;
- const gchar *name;
const gchar *id;
+ gchar *name;
g_autoptr(GPtrArray) ids = NULL;
GError *error = NULL;
@@ -309,6 +309,7 @@ quectel_at_port_get_firmware_revision_ready (MMBaseModem *self,
mm_obj_dbg (self, "revision %s converted to modem name %s", revision, name);
id = (const gchar *) g_ptr_array_index (ids, 0);
g_ptr_array_insert (ids, 0, g_strdup_printf ("%s&NAME_%s", id, name));
+ g_free (name);
}
mm_firmware_update_settings_set_device_ids (update_settings, (const gchar **)ids->pdata);
--
2.45.2

View File

@ -0,0 +1,26 @@
From 02d893f8abaffa51ad61b0bb6ce67f55de5cd217 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Sun, 26 May 2024 23:54:50 +0200
Subject: [PATCH 06/11] port-qmi: fix array bound check
There's an off-by-one error.
---
src/mm-port-qmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mm-port-qmi.c b/src/mm-port-qmi.c
index f3c64464a..38703bc20 100644
--- a/src/mm-port-qmi.c
+++ b/src/mm-port-qmi.c
@@ -1641,7 +1641,7 @@ check_data_format_combination (GTask *task)
/* go on to the next supported combination */
for (++ctx->data_format_combination_i;
- ctx->data_format_combination_i <= (gint)G_N_ELEMENTS (data_format_combinations);
+ ctx->data_format_combination_i < (gint)G_N_ELEMENTS (data_format_combinations);
ctx->data_format_combination_i++) {
const DataFormatCombination *combination;
g_autofree gchar *kernel_data_mode_str = NULL;
--
2.45.2

View File

@ -0,0 +1,25 @@
From 7658e4be92a26c5e0245a3169fb61922f0e96811 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Sun, 26 May 2024 23:58:41 +0200
Subject: [PATCH 07/11] mmcli/sms: do not leak message_reference
---
cli/mmcli-sms.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/cli/mmcli-sms.c b/cli/mmcli-sms.c
index 6586d0373..46bc95ef8 100644
--- a/cli/mmcli-sms.c
+++ b/cli/mmcli-sms.c
@@ -191,6 +191,8 @@ print_sms_info (MMSms *sms)
mmcli_output_string (MMC_F_SMS_PROPERTIES_DELIVERY_STATE, delivery_state);
mmcli_output_string (MMC_F_SMS_PROPERTIES_DISCH_TIMESTAMP, mm_sms_get_discharge_timestamp (sms));
mmcli_output_dump ();
+
+ g_free (message_reference);
}
static void
--
2.45.2

View File

@ -0,0 +1,27 @@
From f8f41b677a234f98085854253b2a1a42b0f09d6f Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Sun, 26 May 2024 23:59:47 +0200
Subject: [PATCH 08/11] bearer/qmi: fix a copy'n'paste error
Probably not a real issue, given if there's a password there's probably
an user name too.
---
src/mm-bearer-qmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mm-bearer-qmi.c b/src/mm-bearer-qmi.c
index 504134759..4e64e220c 100644
--- a/src/mm-bearer-qmi.c
+++ b/src/mm-bearer-qmi.c
@@ -1330,7 +1330,7 @@ build_start_network_input (ConnectContext *ctx)
if (ctx->auth != QMI_WDS_AUTHENTICATION_NONE) {
if (ctx->user)
qmi_message_wds_start_network_input_set_username (input, ctx->user, NULL);
- if (ctx->user)
+ if (ctx->password)
qmi_message_wds_start_network_input_set_password (input, ctx->password, NULL);
}
}
--
2.45.2

View File

@ -0,0 +1,33 @@
From 6a11c906d0604748db9a81bf470c821db2b862fb Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Mon, 27 May 2024 00:03:23 +0200
Subject: [PATCH 09/11] sim/qmi: do not leak access technology name
---
src/mm-sim-qmi.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/mm-sim-qmi.c b/src/mm-sim-qmi.c
index 03a442403..1d24c23b4 100644
--- a/src/mm-sim-qmi.c
+++ b/src/mm-sim-qmi.c
@@ -991,11 +991,13 @@ set_preferred_networks_reload_ready (MMBaseSim *self,
}
/* Check if there are access technology bits requested but unset */
if ((loaded_act & set_act) != set_act) {
- MMModemAccessTechnology unset = set_act & ~loaded_act;
+ MMModemAccessTechnology unset = set_act & ~loaded_act;
+ gchar *act;
+ act = mm_modem_access_technology_build_string_from_mask (unset);
mm_obj_warn (self, "access technologies '%s' not set for operator code '%s'",
- mm_modem_access_technology_build_string_from_mask (unset),
- set_op_code);
+ act, set_op_code);
+ g_free (act);
error = g_error_new (MM_CORE_ERROR, MM_CORE_ERROR_UNSUPPORTED,
"Access technology unsupported by modem or SIM");
break;
--
2.45.2

View File

@ -0,0 +1,25 @@
From 91f7bbf85b1fccc0c3297acf4203f32b1f195397 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Mon, 27 May 2024 00:04:38 +0200
Subject: [PATCH 10/11] libmm-glib/signal: fix a copy'n'paste error
---
libmm-glib/mm-signal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libmm-glib/mm-signal.c b/libmm-glib/mm-signal.c
index e71cd24ba..f8d8c4aa5 100644
--- a/libmm-glib/mm-signal.c
+++ b/libmm-glib/mm-signal.c
@@ -379,7 +379,7 @@ mm_signal_get_string (MMSignal *self)
g_string_append_printf (printable, "%serror rate: %f %%", printable->len ? ", " : "", self->priv->error_rate);
if (self->priv->rscp != MM_SIGNAL_UNKNOWN)
g_string_append_printf (printable, "%sRSCP: %f dBm", printable->len ? ", " : "", self->priv->rscp);
- if (self->priv->rscp != MM_SIGNAL_UNKNOWN)
+ if (self->priv->rsrp != MM_SIGNAL_UNKNOWN)
g_string_append_printf (printable, "%sRSRP: %f dBm", printable->len ? ", " : "", self->priv->rsrp);
if (self->priv->rsrq != MM_SIGNAL_UNKNOWN)
g_string_append_printf (printable, "%sRSRQ: %f dB", printable->len ? ", " : "", self->priv->rsrq);
--
2.45.2

View File

@ -0,0 +1,39 @@
From a877ed8015a4fcfb05961b5bfe9d03f47cdc55a5 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Mon, 27 May 2024 00:06:45 +0200
Subject: [PATCH 11/11] modem-helpers: do not leak past PDP on error
If CID parsing from the +CGDCONT response fails, the very last PDP
structure allocated is not put on the list yet and therefore
mm_3gpp_pdp_context_list_free() wouldn't free it.
Let's put it on the list first, as to not leak it on error.
---
src/mm-modem-helpers.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index 8dad5a398..9f0c8a8ce 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -1870,6 +1870,8 @@ mm_3gpp_parse_cgdcont_read_response (const gchar *reply,
MM3gppPdpContext *pdp;
pdp = g_slice_new0 (MM3gppPdpContext);
+ list = g_list_prepend (list, pdp);
+
if (!mm_get_uint_from_match_info (match_info, 1, &pdp->cid)) {
inner_error = g_error_new (MM_CORE_ERROR,
MM_CORE_ERROR_FAILED,
@@ -1879,8 +1881,6 @@ mm_3gpp_parse_cgdcont_read_response (const gchar *reply,
}
pdp->pdp_type = ip_family;
pdp->apn = mm_get_string_unquoted_from_match_info (match_info, 3);
-
- list = g_list_prepend (list, pdp);
}
g_free (str);
--
2.45.2

View File

@ -5,12 +5,25 @@
Name: ModemManager
Version: 1.22.0
Release: 4%{?dist}
Release: 5%{?dist}
Summary: Mobile broadband modem management service
License: GPL-2.0-or-later
URL: http://www.freedesktop.org/wiki/Software/ModemManager/
Source: https://gitlab.com/linux-mobile-broadband/ModemManager/-/archive/%{version}/%{name}-%{version}.tar.bz2
# All of these are applied upstream. Can frop on rebase to 1.24
Patch0: 0001-shared-qmi-do-not-leak-a-string-in-error-path.patch
Patch1: 0002-ublox-helpers-fix-propagation-of-URAT-response-parse.patch
Patch2: 0003-broadband-modem-fix-a-potential-leak-in-cusd_process.patch
Patch3: 0004-shared-qmi-fix-a-leak-in-error-handling-path.patch
Patch4: 0005-quectel-shared-do-not-leak-name-string.patch
Patch5: 0006-port-qmi-fix-array-bound-check.patch
Patch6: 0007-mmcli-sms-do-not-leak-message_reference.patch
Patch7: 0008-bearer-qmi-fix-a-copy-n-paste-error.patch
Patch8: 0009-sim-qmi-do-not-leak-access-technology-name.patch
Patch9: 0010-libmm-glib-signal-fix-a-copy-n-paste-error.patch
Patch10: 0011-modem-helpers-do-not-leak-past-PDP-on-error.patch
# For mbim-proxy and qmi-proxy
Requires: libmbim-utils
Requires: libqmi-utils
@ -190,6 +203,9 @@ cp -a cli/mmcli-completion %{buildroot}%{_datadir}/bash-completion/completions/m
%changelog
* Tue Jun 25 2024 Lubomir Rintel <lkundrak@v3.sk> - 1.22.0-5
- Add patches for a couple of bugs that make static analysis unhappy (RHEL-38991)
* Mon Jun 24 2024 Troy Dawson <tdawson@redhat.com> - 1.22.0-4
- Bump release for June 2024 mass rebuild
@ -202,7 +218,7 @@ cp -a cli/mmcli-completion %{buildroot}%{_datadir}/bash-completion/completions/m
* Tue Jan 09 2024 Dennis Gilmore <dennis@ausil.us> - 1.22.0-1
- update to 1.22.0
* Wed Jul 30 2023 Tao Jin <tao-j@outlook.com> - 1.20.6-3
* Sun Jul 30 2023 Tao Jin <tao-j@outlook.com> - 1.20.6-3
- Rebuilt for RHBZ#2226577
* Wed Jul 19 2023 Fedora Release Engineering <releng@fedoraproject.org> - 1.20.6-2