[PATCH] Hardening PORTCFG parse reply

Carlo Lobrano c.lobrano at gmail.com
Wed Jun 1 12:51:31 UTC 2016


A brief explaination of the issue that this patch solves.
I observed a probing failure at resume from memory (S3), caused by a
missing or wrong reply to #PORTCFG? AT command sent during port probing.
This error is probably due to the overposition of multiple messages and replies,
since at the same time the ModemManager is trying and close the last connection,
and probing a new modem device, and the PORTCFG parser receive an empty string to
parse, causing the failure.
Therefore the fix consists in the hardening of the reply to this command and
in providing a default port configuration in case of the retries are not enough.

---

- Increase PORTCFG retries during initialization from 3 to 5 retries.
- Provide a default PORTCFG configuration in case of missing or wrong reply.
---
 plugins/telit/mm-plugin-telit.c | 44 +++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/plugins/telit/mm-plugin-telit.c b/plugins/telit/mm-plugin-telit.c
index a3620e7..19eed8f 100644
--- a/plugins/telit/mm-plugin-telit.c
+++ b/plugins/telit/mm-plugin-telit.c
@@ -165,8 +165,10 @@ cache_port_mode (MMDevice *device,
     r = g_regex_new ("#PORTCFG:\\s*(\\d+),(\\d+)", flags, 0, NULL);
     g_assert (r != NULL);
 
-    if (!g_regex_match_full (r, reply, strlen (reply), 0, 0, &match_info, &error))
+    if (!g_regex_match_full (r, reply, strlen (reply), 0, 0, &match_info, &error)) {
+        mm_dbg ("Unexpected PORTCFG reply '%s'. Could not parse it.", reply);
         goto out;
+    }
 
     if (!mm_get_uint_from_match_info (match_info, 2, &portcfg_current)) {
         mm_dbg ("telit: unrecognized #PORTCFG <active> value");
@@ -201,6 +203,7 @@ cache_port_mode (MMDevice *device,
         break;
     default:
         /* portcfg value not supported */
+        mm_err ("Invalid portcfg value from reply '%s'", reply);
         goto out;
     }
     ret = TRUE;
@@ -222,29 +225,40 @@ getportcfg_ready (MMPortSerialAt *port,
 {
     const gchar *response;
     GError *error = NULL;
+    MMDevice *device;
 
+    device = mm_port_probe_peek_device (ctx->probe);
     response = mm_port_serial_at_command_finish (port, res, &error);
-    if (error) {
-        mm_dbg ("telit: couldn't get port mode: '%s'",
-                error->message);
 
-        /* If ERROR or COMMAND NOT SUPPORT occur then do not retry the
-         * command.
-         */
-        if (g_error_matches (error,
-                             MM_MOBILE_EQUIPMENT_ERROR,
-                             MM_MOBILE_EQUIPMENT_ERROR_UNKNOWN))
-            ctx->getportcfg_done = TRUE;
+    if (error) {
+        mm_dbg ("telit: couldn't get port mode: '%s' (retries left %d)",
+                error->message,
+                ctx->getportcfg_retries);
+
+        if (ctx->getportcfg_retries == 0) {
+            mm_dbg ("telit: getportcfg last retry failed. Try and setting default port config.");
+            if (g_object_get_data (G_OBJECT (device), TAG_GETPORTCFG_SUPPORTED) == NULL) {
+                mm_dbg ("telit: Defaulting to 00 and 06 for modem and aux");
+                g_object_set_data (G_OBJECT (device), TAG_TELIT_MODEM_PORT, "00");
+                g_object_set_data (G_OBJECT (device), TAG_TELIT_AUX_PORT, "06");
+                g_object_set_data (G_OBJECT (device), TAG_GETPORTCFG_SUPPORTED, GUINT_TO_POINTER (TRUE));
+                ctx->getportcfg_done = TRUE;
+            }
+        }
     } else {
-        MMDevice *device;
-        device = mm_port_probe_peek_device (ctx->probe);
-
         /* Results are cached in the parent device object */
         if (g_object_get_data (G_OBJECT (device), TAG_GETPORTCFG_SUPPORTED) == NULL) {
             mm_dbg ("telit: retrieving port mode layout");
             if (cache_port_mode (device, response)) {
                 g_object_set_data (G_OBJECT (device), TAG_GETPORTCFG_SUPPORTED, GUINT_TO_POINTER (TRUE));
                 ctx->getportcfg_done = TRUE;
+            } else {
+                mm_dbg ("telit: Error in cache port mode");
+                mm_dbg ("telit: Defaulting to 00 and 06 for modem and aux");
+                g_object_set_data (G_OBJECT (device), TAG_TELIT_MODEM_PORT, "00");
+                g_object_set_data (G_OBJECT (device), TAG_TELIT_AUX_PORT, "06");
+                g_object_set_data (G_OBJECT (device), TAG_GETPORTCFG_SUPPORTED, GUINT_TO_POINTER (TRUE));
+                ctx->getportcfg_done = TRUE;
             }
         }
 
@@ -334,7 +348,7 @@ telit_custom_init (MMPortProbe *probe,
     ctx->port = g_object_ref (port);
     ctx->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
     ctx->getportcfg_done = FALSE;
-    ctx->getportcfg_retries = 3;
+    ctx->getportcfg_retries = 5;
 
     /* If the device is tagged for supporting #PORTCFG do the custom init */
     if (g_udev_device_get_property_as_boolean (udevDevice, "ID_MM_TELIT_PORTS_TAGGED")) {
-- 
2.1.4



More information about the ModemManager-devel mailing list