public inbox for ofono@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH 01/13] qmi: validate TLV length
@ 2024-10-31 22:06 Denis Kenzior
  2024-10-31 22:06 ` [PATCH 02/13] gobi: Clear out service request queue on shutdown Denis Kenzior
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

For qmi_result_get_(u)int[8,16,32], make sure that the length
corresponds to the size of the basic type prior to performing the
memcpy.
---
 drivers/qmimodem/qmi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/qmimodem/qmi.c b/drivers/qmimodem/qmi.c
index 0ba2e8b9e352..10dbdaac8bf6 100644
--- a/drivers/qmimodem/qmi.c
+++ b/drivers/qmimodem/qmi.c
@@ -2311,7 +2311,7 @@ bool qmi_result_get_uint8(struct qmi_result *result, uint8_t type,
 		return false;
 
 	ptr = tlv_get(result->data, result->length, type, &len);
-	if (!ptr)
+	if (!ptr || len != sizeof(uint8_t))
 		return false;
 
 	if (value)
@@ -2330,7 +2330,7 @@ bool qmi_result_get_int16(struct qmi_result *result, uint8_t type,
 		return false;
 
 	ptr = tlv_get(result->data, result->length, type, &len);
-	if (!ptr)
+	if (!ptr || len != sizeof(int16_t))
 		return false;
 
 	memcpy(&tmp, ptr, 2);
@@ -2351,7 +2351,7 @@ bool qmi_result_get_uint16(struct qmi_result *result, uint8_t type,
 		return false;
 
 	ptr = tlv_get(result->data, result->length, type, &len);
-	if (!ptr)
+	if (!ptr || len != sizeof(uint16_t))
 		return false;
 
 	memcpy(&tmp, ptr, 2);
@@ -2373,7 +2373,7 @@ bool qmi_result_get_uint32(struct qmi_result *result, uint8_t type,
 		return false;
 
 	ptr = tlv_get(result->data, result->length, type, &len);
-	if (!ptr)
+	if (!ptr || len != sizeof(uint32_t))
 		return false;
 
 	memcpy(&tmp, ptr, 4);
@@ -2395,7 +2395,7 @@ bool qmi_result_get_uint64(struct qmi_result *result, uint8_t type,
 		return false;
 
 	ptr = tlv_get(result->data, result->length, type, &len);
-	if (!ptr)
+	if (!ptr || len != sizeof(uint64_t))
 		return false;
 
 	memcpy(&tmp, ptr, 8);
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/13] gobi: Clear out service request queue on shutdown
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 03/13] simutil: Return early if file is not found Denis Kenzior
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

Not doing so can result in a crash on subsequent .enable() calls

ofonod[365853]: #0  0x743188e15ae0 in /usr/lib/libc.so.6
ofonod[365853]: #1  0x472b94 in request_service_cb() at plugins/gobi.c:475
ofonod[365853]: #2  0x45ce9f in qmux_create_client_callback() at drivers/qmimodem/qmi.c:1448
ofonod[365853]: #3  0x45bd87 in __rx_ctl_message() at drivers/qmimodem/qmi.c:993
ofonod[365853]: #4  0x45bf97 in received_qmux_data() at drivers/qmimodem/qmi.c:1041
ofonod[365853]: #5  0x597df3 in io_callback() at ell/io.c:105 (discriminator 1)
ofonod[365853]: #6  0x59674a in l_main_iterate() at ell/main.c:461
ofonod[365853]: #7  0x508898 in event_check() at src/main.c:183
ofonod[365853]: #8  0x743189073ae1 in /usr/lib/libglib-2.0.so.0
ofonod[365853]: #9  0x7431890d4816 in /usr/lib/libglib-2.0.so.0
ofonod[365853]: #10 0x743189073787 in /usr/lib/libglib-2.0.so.0
ofonod[365853]: #11 0x508d35 in main() at src/main.c:302
ofonod[365853]: #12 0x743188dfec88 in /usr/lib/libc.so.6
ofonod[365853]: #13 0x743188dfed4c in /usr/lib/libc.so.6
---
 plugins/gobi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index cb20824cc424..b2c9c3d50ea4 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -251,6 +251,10 @@ static void shutdown_cb(void *user_data)
 	DBG("");
 
 	data->discover_attempts = 0;
+	memset(&data->service_requests, 0, sizeof(data->service_requests));
+	data->cur_service_request = 0;
+	data->num_service_requests = 0;
+	data->features = 0;
 
 	qmi_qmux_device_free(data->device);
 	data->device = NULL;
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/13] simutil: Return early if file is not found
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
  2024-10-31 22:06 ` [PATCH 02/13] gobi: Clear out service request queue on shutdown Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 04/13] simfs: Quiet sanitizer runtime error Denis Kenzior
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

Certain Elementary Files are only present on SIM (2G) or USIMs (3G/4G).
If the file is not relevant to a given generation, its parent2g or
parent3g member will be set to 0.  Return early if the file is found to
be irrelevant for a given phase.  This also fixes a runtime sanitizer
warning:

src/simutil.c:1345:10: runtime error: null pointer passed as argument 1, which is declared to never be null
---
 src/simutil.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/simutil.c b/src/simutil.c
index a504e9aa8e3b..0fafcb7f77de 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -1302,7 +1302,7 @@ unsigned int sim_ef_db_get_path_2g(unsigned short id, unsigned char out_path[])
 
 	info = bsearch(GUINT_TO_POINTER((unsigned int) id), ef_db, nelem,
 				sizeof(struct sim_ef_info), find_ef_by_id);
-	if (info == NULL)
+	if (info == NULL || !info->parent2g)
 		return 0;
 
 	path[i++] = info->parent2g & 0xff;
@@ -1335,7 +1335,7 @@ unsigned int sim_ef_db_get_path_3g(unsigned short id, unsigned char out_path[])
 
 	info = bsearch(GUINT_TO_POINTER((unsigned int) id), ef_db, nelem,
 				sizeof(struct sim_ef_info), find_ef_by_id);
-	if (info == NULL)
+	if (info == NULL || !info->parent3g)
 		return 0;
 
 	path[i++] = info->parent3g & 0xff;
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/13] simfs: Quiet sanitizer runtime error
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
  2024-10-31 22:06 ` [PATCH 02/13] gobi: Clear out service request queue on shutdown Denis Kenzior
  2024-10-31 22:06 ` [PATCH 03/13] simutil: Return early if file is not found Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 05/13] radio-settings: quiet " Denis Kenzior
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

The arguments passed into memcpy for src and 'n' are NULL pointer and 0
respectively.  Strictly speaking null pointers should not be passed to
memcpy, and this raises a sanitizer runtime error:
src/simfs.c:1057:2: runtime error: null pointer passed as argument 2, which is declared to never be null

Use the newly introduced l_memcpy function to avoid triggering the
sanitizers
---
 src/simfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/simfs.c b/src/simfs.c
index c86e8d11fd0b..7f0390032bf6 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -9,7 +9,6 @@
 #include <config.h>
 #endif
 
-#include <string.h>
 #include <stdio.h>
 
 #include <glib.h>
@@ -27,6 +26,8 @@
 #include "storage.h"
 #include "missing.h"
 
+#include <ell/ell.h>
+
 #define SIM_CACHE_MODE 0600
 #define SIM_CACHE_BASEPATH STORAGEDIR "/%s-%i"
 #define SIM_CACHE_VERSION SIM_CACHE_BASEPATH "/version"
@@ -1054,7 +1055,7 @@ int sim_fs_read(struct ofono_sim_context *context, int id,
 	op->num_bytes = num_bytes;
 	op->info_only = FALSE;
 	op->context = context;
-	memcpy(op->path, path, path_len);
+	l_memcpy(op->path, path, path_len);
 	op->path_len = path_len;
 
 	g_queue_push_tail(fs->op_q, op);
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/13] radio-settings: quiet sanitizer runtime error
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (2 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 04/13] simfs: Quiet sanitizer runtime error Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 06/13] gprs: Default CID range to 1..NUM_CONTEXTS -1 Denis Kenzior
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

src/radio-settings.c:241:17: runtime error: left shift of 1 by 31 places cannot be represented in type 'int'
---
 src/radio-settings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/radio-settings.c b/src/radio-settings.c
index 1b56cad0a32a..588a355d4e40 100644
--- a/src/radio-settings.c
+++ b/src/radio-settings.c
@@ -238,7 +238,7 @@ static DBusMessage *radio_get_properties_reply(DBusMessage *msg,
 		unsigned int i;
 
 		for (i = 0; i < sizeof(uint32_t) * CHAR_BIT; i++) {
-			int tech = 1 << i;
+			unsigned int tech = 1U << i;
 
 			if (!(rs->available_rats & tech))
 				continue;
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/13] gprs: Default CID range to 1..NUM_CONTEXTS -1
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (3 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 05/13] radio-settings: quiet " Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 07/13] qmimodem: Drop call to ofono_gprs_set_cid_range Denis Kenzior
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

For those drivers where CIDs have no meaning (such as non-AT command
based modems), set a default range between 1 and NUM_CONTEXTS - 1 (255).
This allows the driver to omit this method invocation.
---
 src/gprs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/gprs.c b/src/gprs.c
index 3c71f8084417..c5287661b9d9 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -3082,6 +3082,7 @@ OFONO_DEFINE_ATOM_CREATE(gprs, OFONO_ATOM_TYPE_GPRS, {
 	atom->status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
 	atom->netreg_status = -1;
 	atom->used_pids = l_uintset_new(MAX_CONTEXTS);
+	atom->used_cids = l_uintset_new_from_range(1, MAX_CONTEXTS - 1);
 })
 
 static void netreg_watch(struct ofono_atom *atom,
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/13] qmimodem: Drop call to ofono_gprs_set_cid_range
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (4 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 06/13] gprs: Default CID range to 1..NUM_CONTEXTS -1 Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 08/13] gobi: Remove support for qmi_wwan_q Denis Kenzior
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

The intent was to set cid range to only support the default profile
number.  However, the gprs-context atom never ended up using cid to
correlate to the underlying profile number.  cid is only used to inform
the core about the context that was disconnected.
---
 drivers/qmimodem/gprs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
index 547b32059da4..27060f9fba44 100644
--- a/drivers/qmimodem/gprs.c
+++ b/drivers/qmimodem/gprs.c
@@ -400,7 +400,6 @@ static void get_default_profile_number_cb(struct qmi_result *result,
 
 	DBG("Default profile index: %hhd", index);
 	data->default_profile = index;
-	ofono_gprs_set_cid_range(gprs, index, index);
 
 	if (set_event_report_request(gprs) >= 0)
 		return;
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/13] gobi: Remove support for qmi_wwan_q
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (5 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 07/13] qmimodem: Drop call to ofono_gprs_set_cid_range Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 09/13] udevng: Remove non-upstream qmi_wwan_q support Denis Kenzior
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

Focus this driver purely on qmi_wwan and its quirks.  This simplifies
the logic considerably.
---
 plugins/gobi.c | 60 +++++++++++---------------------------------------
 1 file changed, 13 insertions(+), 47 deletions(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index b2c9c3d50ea4..5c3ae0737a15 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -88,8 +88,6 @@ struct gobi_data {
 	uint32_t max_aggregation_size;
 	uint32_t set_powered_id;
 	bool using_mux : 1;
-	bool using_qmi_wwan : 1;
-	bool using_qmi_wwan_q : 1;
 };
 
 static void gobi_debug(const char *str, void *user_data)
@@ -111,15 +109,15 @@ static void gobi_io_debug(const char *str, void *user_data)
  * in order to initialize the driver properly:
  *
  * NetworkInterface
- *   The string that contains the 'main' network device.  This can be
- *   "rmnet_ipa" on SoC systems, or "wwan0" for upstream linux systems.
+ *   The string that contains the 'main' network device.  This is typically
+ *   'wwanX' on upstream linux systems.
  *
  * NetworkInterfaceIndex
  *   The index of the main interface given by NetworkInterface
  *
  * NetworkInterfaceKernelDriver
- *   The kernel driver that is being used by the main network device.  Certain
- *   drivers such as 'qmi_wwan' or 'qmi_wwan_q' are treated specifically.
+ *   The kernel driver that is being used by the main network device.  Only
+ *   'qmi_wwan' is supported.
  *
  * Bus
  *   The bus of the modem.  Values can be "usb", "embedded", or "pci"
@@ -131,7 +129,6 @@ static int gobi_probe(struct ofono_modem *modem)
 	const char *ifname;
 	int ifindex;
 	const char *bus;
-	int n_premux;
 
 	DBG("%p", modem);
 
@@ -140,32 +137,22 @@ static int gobi_probe(struct ofono_modem *modem)
 	ifname = ofono_modem_get_string(modem, "NetworkInterface");
 	ifindex = ofono_modem_get_integer(modem, "NetworkInterfaceIndex");
 	bus = ofono_modem_get_string(modem, "Bus");
-	n_premux = ofono_modem_get_integer(modem, "NumPremuxInterfaces");
 
 	DBG("net: %s[%s](%d) %s", ifname, if_driver, ifindex, bus);
 
-	if (!if_driver || !ifname || !ifindex || !bus || n_premux < 0)
+	if (!if_driver || !ifname || !ifindex || !bus)
 		return -EPROTO;
 
-	data = l_new(struct gobi_data, 1);
-
-	if (!strcmp(if_driver, "qmi_wwan_q"))
-		data->using_qmi_wwan_q = true;
-	else if (!strcmp(if_driver, "qmi_wwan"))
-		data->using_qmi_wwan = true;
-
-	if (n_premux > MAX_CONTEXTS) {
-		l_warn("NumPremuxInterfaces > %d, limiting to %d",
-				MAX_CONTEXTS, MAX_CONTEXTS);
-		n_premux = MAX_CONTEXTS;
-	}
+	if (!L_IN_STRSET(if_driver, "qmi_wwan"))
+		return -ENOTSUP;
 
-	data->n_premux = n_premux;
+	data = l_new(struct gobi_data, 1);
 	data->main_net_ifindex =
 		ofono_modem_get_integer(modem, "NetworkInterfaceIndex");
 	l_strlcpy(data->main_net_name,
 			ofono_modem_get_string(modem, "NetworkInterface"),
 			sizeof(data->main_net_name));
+
 	ofono_modem_set_data(modem, data);
 	ofono_modem_set_capabilities(modem, OFONO_MODEM_CAPABILITY_LTE);
 
@@ -423,12 +410,7 @@ static void get_data_format_cb(struct qmi_result *result, void *user_data)
 	if (!qmi_result_get_uint32(result, QMI_WDA_LL_PROTOCOL, &llproto))
 		goto done;
 
-	if (data->using_qmi_wwan) {
-		const char *interface =
-			ofono_modem_get_string(modem, "NetworkInterface");
-
-		setup_qmi_wwan(interface, llproto);
-	}
+	setup_qmi_wwan(data->main_net_name, llproto);
 
 done:
 	if (qmi_service_send(data->dms, QMI_DMS_GET_CAPS, NULL,
@@ -674,10 +656,6 @@ static void powered_up_cb(int error, uint16_t type,
 	if (!param)
 		goto error;
 
-	if (data->using_qmi_wwan_q)
-		l_sysctl_set_u32(1, "/sys/class/net/%s/link_state",
-					data->main_net_name);
-
 	cb_data_ref(cbd);
 
 	if (qmi_service_send(data->dms, QMI_DMS_SET_OPER_MODE, param,
@@ -711,10 +689,6 @@ static void powered_down_cb(int error, uint16_t type,
 	if (!param)
 		goto error;
 
-	if (data->using_qmi_wwan_q)
-		l_sysctl_set_u32(0, "/sys/class/net/%s/link_state",
-					data->main_net_name);
-
 	cb_data_ref(cbd);
 
 	if (qmi_service_send(data->dms, QMI_DMS_SET_OPER_MODE, param,
@@ -792,8 +766,6 @@ static void gobi_setup_gprs(struct ofono_modem *modem)
 	struct gobi_data *data = ofono_modem_get_data(modem);
 	struct ofono_gprs *gprs;
 	struct ofono_gprs_context *gc;
-	const char *interface;
-	char buf[256];
 	int i;
 
 	gprs = ofono_gprs_create(modem, 0, "qmimodem",
@@ -810,8 +782,6 @@ static void gobi_setup_gprs(struct ofono_modem *modem)
 		struct qmi_service *ipv4 = data->context_services[0].wds_ipv4;
 		struct qmi_service *ipv6 = data->context_services[0].wds_ipv6;
 
-		interface = ofono_modem_get_string(modem, "NetworkInterface");
-
 		gc = ofono_gprs_context_create(modem, 0, "qmimodem", -1,
 						qmi_service_clone(ipv4),
 						qmi_service_clone(ipv6));
@@ -822,20 +792,16 @@ static void gobi_setup_gprs(struct ofono_modem *modem)
 		}
 
 		ofono_gprs_add_context(gprs, gc);
-		ofono_gprs_context_set_interface(gc, interface);
+		ofono_gprs_context_set_interface(gc, data->main_net_name);
 
 		return;
 	}
 
-	data->using_mux = true;
-
-	data->max_aggregation_size =
-		ofono_modem_get_integer(modem, "MaxAggregationSize");
-	DBG("max_aggregation_size: %u", data->max_aggregation_size);
-
 	for (i = 0; i < data->n_premux; i++) {
 		struct qmi_service *ipv4 = data->context_services[i].wds_ipv4;
 		struct qmi_service *ipv6 = data->context_services[i].wds_ipv6;
+		const char *interface;
+		char buf[256];
 		int mux_id;
 
 		sprintf(buf, "PremuxInterface%dMuxId", i + 1);
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/13] udevng: Remove non-upstream qmi_wwan_q support
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (6 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 08/13] gobi: Remove support for qmi_wwan_q Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 10/13] gobi: Bring down the main interface at startup Denis Kenzior
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

This reverts commit:
fd1b44514315 ("udevng: Detect Quectel devices that use qmi_wwan_q driver")
---
 plugins/udevng.c | 71 ++++--------------------------------------------
 1 file changed, 5 insertions(+), 66 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index f34cdcb731ee..64875a47752b 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1108,19 +1108,6 @@ static gboolean setup_quectel(struct modem_info *modem)
 		return FALSE;
 }
 
-static gboolean is_premultiplexed(const struct device_info *net)
-{
-	struct udev_device *parent = udev_device_get_parent(net->udev_device);
-
-	if (!parent)
-		return FALSE;
-
-	if (g_strcmp0(udev_device_get_subsystem(parent), "net") == 0)
-		return TRUE;
-
-	return FALSE;
-}
-
 static gboolean setup_quectelqmi(struct modem_info *modem)
 {
 	const struct device_info *net = NULL;
@@ -1128,11 +1115,6 @@ static gboolean setup_quectelqmi(struct modem_info *modem)
 	const char *gps = NULL;
 	const char *aux = NULL;
 	GSList *list;
-	const char *premux_interfaces[8];
-	int n_premux = 0;
-	const char *qmap_size;
-
-	memset(premux_interfaces, 0, sizeof(premux_interfaces));
 
 	DBG("%s", modem->syspath);
 
@@ -1144,22 +1126,12 @@ static gboolean setup_quectelqmi(struct modem_info *modem)
 		DBG("%s %s %s %s %s", info->devnode, info->interface,
 				info->number, info->label, subsystem);
 
-		if (g_strcmp0(info->interface, "255/255/255") == 0) {
-			if (g_strcmp0(subsystem, "usbmisc") == 0) {
+		if (g_strcmp0(info->interface, "255/255/255") == 0 &&
+				g_strcmp0(info->number, "04") == 0) {
+			if (g_strcmp0(subsystem, "net") == 0)
+				net = info;
+			else if (g_strcmp0(subsystem, "usbmisc") == 0)
 				qmi = info;
-				continue;
-			}
-
-			if (g_strcmp0(subsystem, "net"))
-				continue;
-
-			if (is_premultiplexed(info)) {
-				premux_interfaces[n_premux] = info->devnode;
-				n_premux += 1;
-				continue;
-			}
-
-			net = info;
 		} else if (g_strcmp0(info->interface, "255/0/0") == 0 &&
 				g_strcmp0(info->number, "01") == 0) {
 			gps = info->devnode;
@@ -1177,44 +1149,12 @@ static gboolean setup_quectelqmi(struct modem_info *modem)
 	if (setup_qmi_qmux(modem, qmi, net) < 0)
 		return FALSE;
 
-	qmap_size = udev_device_get_sysattr_value(net->udev_device,
-							"qmap_size");
-	if (qmap_size) {
-		uint32_t max_aggregation_size;
-
-		if (l_safe_atou32(qmap_size, &max_aggregation_size) == 0)
-			ofono_modem_set_integer(modem->modem,
-						"MaxAggregationSize",
-						max_aggregation_size);
-	}
-
 	if (gps)
 		ofono_modem_set_string(modem->modem, "GPS", gps);
 
 	if (aux)
 		ofono_modem_set_string(modem->modem, "Aux", aux);
 
-	if (n_premux) {
-		char buf[256];
-		int i;
-
-		ofono_modem_set_integer(modem->modem,
-					"NumPremuxInterfaces", n_premux);
-		for (i = 0; i < n_premux; i++) {
-			const char *device = premux_interfaces[i];
-			int len = strlen(device);
-
-			if (!len)
-				continue;
-
-			sprintf(buf, "PremuxInterface%d", i + 1);
-			ofono_modem_set_string(modem->modem, buf, device);
-			sprintf(buf, "PremuxInterface%dMuxId", i + 1);
-			ofono_modem_set_integer(modem->modem, buf,
-						0x80 + device[len - 1] - '0');
-		}
-	}
-
 	return TRUE;
 }
 
@@ -2160,7 +2100,6 @@ static struct {
 	{ "quectelqmi", "qmi_wwan",	"2c7c", "0800"	},
 	{ "quectelqmi", "qcserial",	"2c7c", "0800"	},
 	{ "quectelqmi", "option",	"2c7c", "0800"	},
-	{ "quectelqmi", "qmi_wwan_q",	"2c7c", "0452"	},
 	{ "ublox",	"cdc_acm",	"1546", "1010"	},
 	{ "ublox",	"cdc_ncm",	"1546", "1010"	},
 	{ "ublox",	"cdc_acm",	"1546", "1102"	},
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/13] gobi: Bring down the main interface at startup
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (7 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 09/13] udevng: Remove non-upstream qmi_wwan_q support Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 11/13] gobi: Support only "usb" Bus values Denis Kenzior
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

The current gobi logic tried to set the qmi_wwan 'raw_ip' state based on
the results of wda_get_data_format.  If link_layer protocol was
'raw_ip', then 'raw_ip' setting on qmi_wwan was set to 'Y'.  For 802.3,
'raw_ip' setting was set to 'N'.

Unfortunately this was broken at some point and the setting could not be
toggled when the main network device was IFF_UP (default when USB device
is detected by the kernel).

Fix this by having .enable callback bring the device down at
initialization time.  oFono core will set the device IFF_UP once the
context is activated, and bring it down when the context is deactivated.
---
 plugins/gobi.c | 46 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index 5c3ae0737a15..e75a0312f6d0 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -230,13 +230,10 @@ static void add_service_request(struct gobi_data *data,
 	data->service_requests[data->num_service_requests++] = req;
 }
 
-static void shutdown_cb(void *user_data)
+static void __shutdown_device(struct ofono_modem *modem)
 {
-	struct ofono_modem *modem = user_data;
 	struct gobi_data *data = ofono_modem_get_data(modem);
 
-	DBG("");
-
 	data->discover_attempts = 0;
 	memset(&data->service_requests, 0, sizeof(data->service_requests));
 	data->cur_service_request = 0;
@@ -245,7 +242,15 @@ static void shutdown_cb(void *user_data)
 
 	qmi_qmux_device_free(data->device);
 	data->device = NULL;
+}
+
+static void shutdown_cb(void *user_data)
+{
+	struct ofono_modem *modem = user_data;
 
+	DBG("");
+
+	__shutdown_device(modem);
 	ofono_modem_set_powered(modem, FALSE);
 }
 
@@ -552,11 +557,32 @@ error:
 	shutdown_device(modem);
 }
 
+static void init_powered_down_cb(int error, uint16_t type,
+					const void *msg, uint32_t len,
+					void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct gobi_data *data = ofono_modem_get_data(modem);
+	int r;
+
+	DBG("error: %d", error);
+
+	data->set_powered_id = 0;
+
+	if (error)
+		goto error;
+
+	r = qmi_qmux_device_discover(data->device, discover_cb, modem, NULL);
+	if (!r)
+		return;
+error:
+	__shutdown_device(modem);
+}
+
 static int gobi_enable(struct ofono_modem *modem)
 {
 	struct gobi_data *data = ofono_modem_get_data(modem);
 	const char *device;
-	int r;
 
 	DBG("%p", modem);
 
@@ -575,11 +601,15 @@ static int gobi_enable(struct ofono_modem *modem)
 		qmi_qmux_device_set_io_debug(data->device,
 						gobi_io_debug, "QMI: ");
 
-	r = qmi_qmux_device_discover(data->device, discover_cb, modem, NULL);
-	if (!r)
+	data->set_powered_id =
+		l_rtnl_set_powered(l_rtnl_get(), data->main_net_ifindex,
+					false, init_powered_down_cb,
+					modem, NULL);
+	if (data->set_powered_id)
 		return -EINPROGRESS;
 
-	return r;
+	__shutdown_device(modem);
+	return -EIO;
 }
 
 static void power_disable_cb(struct qmi_result *result, void *user_data)
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 11/13] gobi: Support only "usb" Bus values
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (8 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 10/13] gobi: Bring down the main interface at startup Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 12/13] gobi: document and validate "interfaceNumber" Denis Kenzior
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

This may change in the future, but for now the gobi driver will only
support devices using the qmi_wwan network interface driver.  This
driver is limited to USB.
---
 plugins/gobi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index e75a0312f6d0..6ad8a4b7f923 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -120,7 +120,7 @@ static void gobi_io_debug(const char *str, void *user_data)
  *   'qmi_wwan' is supported.
  *
  * Bus
- *   The bus of the modem.  Values can be "usb", "embedded", or "pci"
+ *   The bus of the modem.  Values can be "usb"
  */
 static int gobi_probe(struct ofono_modem *modem)
 {
@@ -146,6 +146,9 @@ static int gobi_probe(struct ofono_modem *modem)
 	if (!L_IN_STRSET(if_driver, "qmi_wwan"))
 		return -ENOTSUP;
 
+	if (!L_IN_STRSET(bus, "usb"))
+		return -ENOTSUP;
+
 	data = l_new(struct gobi_data, 1);
 	data->main_net_ifindex =
 		ofono_modem_get_integer(modem, "NetworkInterfaceIndex");
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 12/13] gobi: document and validate "interfaceNumber"
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (9 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 11/13] gobi: Support only "usb" Bus values Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-10-31 22:06 ` [PATCH 13/13] qmi: wda: Convert #defines to an enum Denis Kenzior
  2024-11-04 22:20 ` [PATCH 01/13] qmi: validate TLV length patchwork-bot+ofono
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

---
 plugins/gobi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/plugins/gobi.c b/plugins/gobi.c
index 6ad8a4b7f923..54b64acb7871 100644
--- a/plugins/gobi.c
+++ b/plugins/gobi.c
@@ -85,6 +85,7 @@ struct gobi_data {
 	uint8_t oper_mode;
 	int main_net_ifindex;
 	char main_net_name[IFNAMSIZ];
+	uint8_t interface_number;
 	uint32_t max_aggregation_size;
 	uint32_t set_powered_id;
 	bool using_mux : 1;
@@ -115,6 +116,9 @@ static void gobi_io_debug(const char *str, void *user_data)
  * NetworkInterfaceIndex
  *   The index of the main interface given by NetworkInterface
  *
+ * InterfaceNumber
+ *   The USB interface number of the network interface
+ *
  * NetworkInterfaceKernelDriver
  *   The kernel driver that is being used by the main network device.  Only
  *   'qmi_wwan' is supported.
@@ -127,6 +131,7 @@ static int gobi_probe(struct ofono_modem *modem)
 	struct gobi_data *data;
 	const char *if_driver;
 	const char *ifname;
+	uint8_t interface_number;
 	int ifindex;
 	const char *bus;
 
@@ -149,12 +154,17 @@ static int gobi_probe(struct ofono_modem *modem)
 	if (!L_IN_STRSET(bus, "usb"))
 		return -ENOTSUP;
 
+	if (l_safe_atox8(ofono_modem_get_string(modem, "InterfaceNumber"),
+				&interface_number) < 0)
+		return -EINVAL;
+
 	data = l_new(struct gobi_data, 1);
 	data->main_net_ifindex =
 		ofono_modem_get_integer(modem, "NetworkInterfaceIndex");
 	l_strlcpy(data->main_net_name,
 			ofono_modem_get_string(modem, "NetworkInterface"),
 			sizeof(data->main_net_name));
+	data->interface_number = interface_number;
 
 	ofono_modem_set_data(modem, data);
 	ofono_modem_set_capabilities(modem, OFONO_MODEM_CAPABILITY_LTE);
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 13/13] qmi: wda: Convert #defines to an enum
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (10 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 12/13] gobi: document and validate "interfaceNumber" Denis Kenzior
@ 2024-10-31 22:06 ` Denis Kenzior
  2024-11-04 22:20 ` [PATCH 01/13] qmi: validate TLV length patchwork-bot+ofono
  12 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2024-10-31 22:06 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

---
 drivers/qmimodem/wda.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/qmimodem/wda.h b/drivers/qmimodem/wda.h
index 25bc01eb7565..fd1e1f63ab1e 100644
--- a/drivers/qmimodem/wda.h
+++ b/drivers/qmimodem/wda.h
@@ -10,6 +10,9 @@
 
 /* Get and set data format interface */
 #define QMI_WDA_LL_PROTOCOL	0x11	/* uint32_t */
-#define QMI_WDA_DATA_LINK_PROTOCOL_UNKNOWN	0
-#define QMI_WDA_DATA_LINK_PROTOCOL_802_3	1
-#define QMI_WDA_DATA_LINK_PROTOCOL_RAW_IP	2
+
+enum qmi_wda_data_link_protocol {
+	QMI_WDA_DATA_LINK_PROTOCOL_UNKNOWN =			0x00,
+	QMI_WDA_DATA_LINK_PROTOCOL_802_3 =			0x01,
+	QMI_WDA_DATA_LINK_PROTOCOL_RAW_IP =			0x02,
+};
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 01/13] qmi: validate TLV length
  2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
                   ` (11 preceding siblings ...)
  2024-10-31 22:06 ` [PATCH 13/13] qmi: wda: Convert #defines to an enum Denis Kenzior
@ 2024-11-04 22:20 ` patchwork-bot+ofono
  12 siblings, 0 replies; 14+ messages in thread
From: patchwork-bot+ofono @ 2024-11-04 22:20 UTC (permalink / raw)
  To: Denis Kenzior; +Cc: ofono

Hello:

This series was applied to ofono.git (master)
by Denis Kenzior <denkenz@gmail.com>:

On Thu, 31 Oct 2024 17:06:08 -0500 you wrote:
> For qmi_result_get_(u)int[8,16,32], make sure that the length
> corresponds to the size of the basic type prior to performing the
> memcpy.
> ---
>  drivers/qmimodem/qmi.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Here is the summary with links:
  - [01/13] qmi: validate TLV length
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=ba768fa893e8
  - [02/13] gobi: Clear out service request queue on shutdown
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=3c6eda5e3470
  - [03/13] simutil: Return early if file is not found
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=b59850d0a4e3
  - [04/13] simfs: Quiet sanitizer runtime error
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=ede833c92f54
  - [05/13] radio-settings: quiet sanitizer runtime error
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=b697463bc642
  - [06/13] gprs: Default CID range to 1..NUM_CONTEXTS -1
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=0fba72b098a1
  - [07/13] qmimodem: Drop call to ofono_gprs_set_cid_range
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=40ecee4aff07
  - [08/13] gobi: Remove support for qmi_wwan_q
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=4b854f0736d5
  - [09/13] udevng: Remove non-upstream qmi_wwan_q support
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=da340fd02b80
  - [10/13] gobi: Bring down the main interface at startup
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=147de7a149f9
  - [11/13] gobi: Support only "usb" Bus values
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=f5d36c1c11b9
  - [12/13] gobi: document and validate "interfaceNumber"
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=d4df0fb906f9
  - [13/13] qmi: wda: Convert #defines to an enum
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=5677ffdbe643

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-11-04 22:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-31 22:06 [PATCH 01/13] qmi: validate TLV length Denis Kenzior
2024-10-31 22:06 ` [PATCH 02/13] gobi: Clear out service request queue on shutdown Denis Kenzior
2024-10-31 22:06 ` [PATCH 03/13] simutil: Return early if file is not found Denis Kenzior
2024-10-31 22:06 ` [PATCH 04/13] simfs: Quiet sanitizer runtime error Denis Kenzior
2024-10-31 22:06 ` [PATCH 05/13] radio-settings: quiet " Denis Kenzior
2024-10-31 22:06 ` [PATCH 06/13] gprs: Default CID range to 1..NUM_CONTEXTS -1 Denis Kenzior
2024-10-31 22:06 ` [PATCH 07/13] qmimodem: Drop call to ofono_gprs_set_cid_range Denis Kenzior
2024-10-31 22:06 ` [PATCH 08/13] gobi: Remove support for qmi_wwan_q Denis Kenzior
2024-10-31 22:06 ` [PATCH 09/13] udevng: Remove non-upstream qmi_wwan_q support Denis Kenzior
2024-10-31 22:06 ` [PATCH 10/13] gobi: Bring down the main interface at startup Denis Kenzior
2024-10-31 22:06 ` [PATCH 11/13] gobi: Support only "usb" Bus values Denis Kenzior
2024-10-31 22:06 ` [PATCH 12/13] gobi: document and validate "interfaceNumber" Denis Kenzior
2024-10-31 22:06 ` [PATCH 13/13] qmi: wda: Convert #defines to an enum Denis Kenzior
2024-11-04 22:20 ` [PATCH 01/13] qmi: validate TLV length patchwork-bot+ofono

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox