From patchwork Tue Mar 19 16:42:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Denis Kenzior X-Patchwork-Id: 13596911 Received: from mail-oo1-f47.google.com (mail-oo1-f47.google.com [209.85.161.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 93F91107A8 for ; Tue, 19 Mar 2024 16:43:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.47 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710866595; cv=none; b=gG8DhuaIQRT6e6W0+q35YGWhYAvvDeQvo8idvwVXPGdPio8g7ggqz4b43VevAUDoFW3hGSR3cAukD7q9YqnJwRioEshPlr5bRkIU4r9bReVd6sdq6ZYOR2QptwzH2f5S6yslF0sD1jXPcdcKlI22glACXotrLyYSWnwjv2NFiJ8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710866595; c=relaxed/simple; bh=yc15sj0nFclzzLsu8eLtNMTHipvMK6JbdYUz45o3+YQ=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=llfewT10ik5cmDc9YBVUogOHbRqbVCElsgK7J+WUAQ8vnTeMNon7v+o+BSaVcoldh/xuXoQKyNPxcAaNeun6WhpfPAGpDWMV2GVt5KOZDqszQhf2HEPJ/VpuToXH+oeaghcqoPXVnpA+7UDVCA7U5Il8IDwGdo0tmms1hXaBnXk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=QSOMR5jo; arc=none smtp.client-ip=209.85.161.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="QSOMR5jo" Received: by mail-oo1-f47.google.com with SMTP id 006d021491bc7-5a0932aa9ecso2219498eaf.3 for ; Tue, 19 Mar 2024 09:43:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710866592; x=1711471392; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=IiFZpeUi4cNegRwxUgbdjXLX0QkxWFAfQSGfhMos0bo=; b=QSOMR5jo3+WUE9QRZOsg7f6J3LV3fBFNx3Ufi9V+TfTqNWO9/RWXyhGyZsJmfkmKeW OACW1RW7CIea4v5ws2dfm2R1GLQjhqSBUHqwqk3veGdl7HUclAmvGGO9mSPvqmVKNKg+ HznzNbxLVrq+T+MgDlW6+YszOqealIH9PQRspL1TuBBdBhKimXxg0+4steLGebQj6bkj 8e5Q8NZXrNv8Jl3yEZrn5Yqu7AoIak3JVEbbz0/KjCvstmiR2NlqoIpMkt/nxmd9vz5z ZyU4nVRrk5wkTsghjD+HEO3T31l0XOQBRwQRPuLU/bH3Kr5Y1Lopy0qIck2nkqx5IhAe XNmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710866592; x=1711471392; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=IiFZpeUi4cNegRwxUgbdjXLX0QkxWFAfQSGfhMos0bo=; b=PvEM3eZVd7Qg9lhiINKnuuXPynXtW6HNgB5Bx5txkn5i/3xLpA+TDBJQe3YrITyBVT 0nB2S99BdteOjysQnnwjnLNovrTNMUa+fHAuVM8V4S0VwUfkn6EDcJcR8uSDFhLxpjct ghkEPQI8xJLMO32lSo84B3iEddvAj3kU/Er1g7PO6Azmui7KIxQx2JFtptf7vM/iNtUF fgxTHI6ywxS95oYXhSmUpyPywbsS/7VGj98M5IjhMq164/0OBv9oKzkXY+0opzessEJe qMF4qjb9eXI/D7W61DPwhGalU0Bw4CcaQfvQHiPOjgYiJIK8L/4lH7WW/cg1eTZEfkp6 zIew== X-Gm-Message-State: AOJu0YzMtOgnvlU80N6B1H52N2/K39/uR4V9x1hjLxN7MOWL1u1jZbnt PEY9xmlfMbuV4oReEdKrLUEMeBLQLWhqwp5uUUcIAHszT0sqgy4y5nm/qvME X-Google-Smtp-Source: AGHT+IF3y9TZtiAYKViO5bDlaul+PLQfYd6myqqDzu6TMfDOnbi3goA2RFiVnuMonqJJ6B6ZrBwt+Q== X-Received: by 2002:a05:6820:1b8e:b0:5a1:8ca1:ae47 with SMTP id cb14-20020a0568201b8e00b005a18ca1ae47mr14954616oob.5.1710866592618; Tue, 19 Mar 2024 09:43:12 -0700 (PDT) Received: from localhost.localdomain (070-114-247-242.res.spectrum.com. [70.114.247.242]) by smtp.gmail.com with ESMTPSA id cp1-20020a056820240100b005a4dc7abc01sm191642oob.11.2024.03.19.09.43.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Mar 2024 09:43:12 -0700 (PDT) From: Denis Kenzior To: ofono@lists.linux.dev Cc: Denis Kenzior Subject: [PATCH 1/2] qmimodem: Rework GET_CARD_STATUS retry logic Date: Tue, 19 Mar 2024 11:42:53 -0500 Message-ID: <20240319164309.2676887-1-denkenz@gmail.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: ofono@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Port the retry logic from glib based implementation that uses g_timeout_add to use l_timeout instead. While here, fix the existing logic to not leak memory when a retry is performed. This happens in one of two ways: - When retry_cbd is allocated via cb_data_new, it is never freed when the timeout GSource fires. - If the timeout GSource is removed early (i.e. due to remove() being called), the associated retry_cbd is not freed Fix this by using cb_data_ref / cb_data_unref and utilize destroy callbacks properly. --- drivers/qmimodem/sim.c | 86 +++++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 39 deletions(-) diff --git a/drivers/qmimodem/sim.c b/drivers/qmimodem/sim.c index 8605e03c6f4f..e561e269f1f2 100644 --- a/drivers/qmimodem/sim.c +++ b/drivers/qmimodem/sim.c @@ -64,13 +64,10 @@ struct sim_data { uint32_t event_mask; uint8_t app_type; uint32_t retry_count; - guint poll_source; + struct l_timeout *retry_timer; uint16_t card_status_indication_id; }; -static void qmi_query_passwd_state(struct ofono_sim *sim, - ofono_sim_passwd_cb_t cb, void *user_data); - static int create_fileid_data(uint8_t app_type, int fileid, const unsigned char *path, unsigned int path_len, @@ -574,22 +571,28 @@ static enum get_card_status_result handle_get_card_status_result( return handle_get_card_status_data(result, sim_stat); } -static gboolean query_passwd_state_retry(gpointer userdata) +static void query_passwd_state_cb(struct qmi_result *result, void *user_data); + +static void query_passwd_state_retry(struct l_timeout *timeout, void *user) { - struct cb_data *cbd = userdata; + struct cb_data *cbd = user; ofono_sim_passwd_cb_t cb = cbd->cb; struct ofono_sim *sim = cbd->user; struct sim_data *data = ofono_sim_get_data(sim); - data->poll_source = 0; + if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL, + query_passwd_state_cb, cbd, cb_data_unref) > 0) { + cb_data_ref(cbd); + return; + } - qmi_query_passwd_state(sim, cb, cbd->data); + CALLBACK_WITH_FAILURE(cb, -1, cbd->data); - return FALSE; + l_timeout_remove(data->retry_timer); + data->retry_timer = NULL; } -static void query_passwd_state_cb(struct qmi_result *result, - void *user_data) +static void query_passwd_state_cb(struct qmi_result *result, void *user_data) { struct cb_data *cbd = user_data; ofono_sim_passwd_cb_t cb = cbd->cb; @@ -597,49 +600,54 @@ static void query_passwd_state_cb(struct qmi_result *result, struct sim_data *data = ofono_sim_get_data(sim); struct sim_status sim_stat; enum get_card_status_result res; - struct cb_data *retry_cbd; unsigned int i; for (i = 0; i < OFONO_SIM_PASSWORD_INVALID; i++) sim_stat.retries[i] = -1; res = handle_get_card_status_result(result, &sim_stat); + if (res == GET_CARD_STATUS_RESULT_TEMP_ERROR && + ++data->retry_count <= MAX_RETRY_COUNT) { + DBG("Retry command"); + + if (!data->retry_timer) { + cb_data_ref(cbd); + data->retry_timer = l_timeout_create_ms(20, + query_passwd_state_retry, + cbd, cb_data_unref); + } else + l_timeout_modify_ms(data->retry_timer, 20); + + return; + } + + l_timeout_remove(data->retry_timer); + data->retry_timer = NULL; + data->retry_count = 0; + switch (res) { case GET_CARD_STATUS_RESULT_OK: DBG("passwd state %d", sim_stat.passwd_state); - data->retry_count = 0; - if (sim_stat.passwd_state == OFONO_SIM_PASSWORD_INVALID) { - CALLBACK_WITH_FAILURE(cb, -1, cbd->data); - ofono_sim_inserted_notify(sim, false); - } else + + if (sim_stat.passwd_state != OFONO_SIM_PASSWORD_INVALID) { CALLBACK_WITH_SUCCESS(cb, sim_stat.passwd_state, cbd->data); + return; + } + break; case GET_CARD_STATUS_RESULT_TEMP_ERROR: - data->retry_count++; - if (data->retry_count > MAX_RETRY_COUNT) { - DBG("Failed after %d attempts. Card state:%d", - data->retry_count, - sim_stat.card_state); - data->retry_count = 0; - CALLBACK_WITH_FAILURE(cb, -1, cbd->data); - ofono_sim_inserted_notify(sim, false); - } else { - DBG("Retry command"); - retry_cbd = cb_data_new(cb, cbd->data); - retry_cbd->user = sim; - data->poll_source = g_timeout_add(20, - query_passwd_state_retry, - retry_cbd); - } + DBG("Failed after %d attempts. Card state:%d", + data->retry_count, + sim_stat.card_state); break; case GET_CARD_STATUS_RESULT_ERROR: DBG("Command failed"); - data->retry_count = 0; - CALLBACK_WITH_FAILURE(cb, -1, cbd->data); - ofono_sim_inserted_notify(sim, false); break; } + + CALLBACK_WITH_FAILURE(cb, -1, cbd->data); + ofono_sim_inserted_notify(sim, false); } static void qmi_query_passwd_state(struct ofono_sim *sim, @@ -653,7 +661,7 @@ static void qmi_query_passwd_state(struct ofono_sim *sim, cbd->user = sim; if (qmi_service_send(data->uim, QMI_UIM_GET_CARD_STATUS, NULL, - query_passwd_state_cb, cbd, l_free) > 0) + query_passwd_state_cb, cbd, cb_data_unref) > 0) return; CALLBACK_WITH_FAILURE(cb, -1, cbd->data); @@ -920,8 +928,8 @@ static void qmi_sim_remove(struct ofono_sim *sim) ofono_sim_set_data(sim, NULL); - if (data->poll_source > 0) - g_source_remove(data->poll_source); + l_timeout_remove(data->retry_timer); + data->retry_timer = NULL; if (data->uim) { if (data->card_status_indication_id) { From patchwork Tue Mar 19 16:42:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Denis Kenzior X-Patchwork-Id: 13596912 Received: from mail-oo1-f42.google.com (mail-oo1-f42.google.com [209.85.161.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A32D1107B2 for ; Tue, 19 Mar 2024 16:43:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.42 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710866596; cv=none; b=sgeDXYzIhLalr/f4N9d1vy/wxipGLLkjqoYwI2gb2WKsBfCrQUSBVa/pc+SECif9dSlKQw8J4qR3tKNdCYdJlErhsOVtrK2uHsBUNkVdP3GsaGeDW5u/MKRWviQKUjrRh1PT2S0awGosjpHqDJMOvaG/gQm7YpeoYxOzK0HqAQ4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710866596; c=relaxed/simple; bh=DhEBG91b/0ZEFQ+mtX0ICkbBTgU3lqyxCj4TEXZoa10=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=BmuTnZhlpk+AoySkRrevmoFLPiFt/j9hhkEkJFr9/Z6LkvNpSPJMpK8RtrCrM3BjQA8oLYkFa2bTokQWq57ywVm215/yo2pJagUcpODv49Me68/pw/MryaYFLdcPUVX8xbgXo4rZY9IO3kPUiaZdMbIFD5uAOwc2tAQBli7Dms0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=hLQEAp+J; arc=none smtp.client-ip=209.85.161.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="hLQEAp+J" Received: by mail-oo1-f42.google.com with SMTP id 006d021491bc7-5a496fde460so1842049eaf.1 for ; Tue, 19 Mar 2024 09:43:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710866593; x=1711471393; darn=lists.linux.dev; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=F9ito3dE7JNsK4PqXiFtywLoobhqtpTu5mMxTr5Ncms=; b=hLQEAp+JgPqbvmvv44+DWO9f35Ad3CKATWs/PDRqgtAiehf4VsrXSLhrDi/S59OLZY ZjgLT0GIUfGwgA5nAjcKBu4F26vlOWtmX2uRSIu8eCt6NYH26E5y32t3IaHntkJ1waAE bUoUjbzpeDTa0I+rK3aeSPxno3+CAvuqMvWAQ4c87Hn0HTdYP9KmoaIL5XKM7N/Qlb1o u5SNtMndDN0nSL/P+rKvp+RfUkE2FY5E42KeqBjWeouL2Ay4mVDA/+SQ9nyRlHDMcaBl QC7qhgZY9AzVLxDAp6ZRyQgFETfqnhM1X8CwmeT3uDhQVWTKNmr4HMBrvzog6ufQ0ZCp Y06A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710866593; x=1711471393; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=F9ito3dE7JNsK4PqXiFtywLoobhqtpTu5mMxTr5Ncms=; b=i0Ba9VlsDu0VD2dXNTRIMTSdpiR1LEZrdDSNNn4VwFn/NF4ebeVFcAjzJK+dkMgVZz fzvGrfkDD8fn7hrpK1z+1++mUlwge/YUmo5OOxyGJs4lqlNgBHHkgVd3UQfhq06Dt4H9 b2BEoPvJ/d6Z29BQmz7CcPx74JfKESAQ3k88c5zcojrW8Vc+/tJsmKELJQst5Y6O5tBp r4oMXZPjwjhZvLuo2pQmVMAvHzKVe2BnT/cIfuyaghX2vyBcxbwJsylhoImvv3FosqvO nH5yPKQ20TsNAR8ZZ1quvwgxWdA0/Le/y658OF0yfCqTdLgiIaVcdO+RB+lXqjJsr+uB Eqmg== X-Gm-Message-State: AOJu0Yyp7zgPissMLxe6I8F46NrzvTlHPZqzgrooF0aPvp15EAxP89Jb /m77ZXYwdD7agm0ylqwWx/BCR0oA/wEiWbUwDG1JwSDDCnHifZ2nEp/hEpgc X-Google-Smtp-Source: AGHT+IFUvI0llUkq8RPBdrtyREAej5VljZIyW6hokgBhFIWCFmqo0M3YeUoIQVxXdPeudbk2ZzktTA== X-Received: by 2002:a05:6820:a0a:b0:5a4:b8dc:bfa8 with SMTP id ch10-20020a0568200a0a00b005a4b8dcbfa8mr6032785oob.7.1710866593541; Tue, 19 Mar 2024 09:43:13 -0700 (PDT) Received: from localhost.localdomain (070-114-247-242.res.spectrum.com. [70.114.247.242]) by smtp.gmail.com with ESMTPSA id cp1-20020a056820240100b005a4dc7abc01sm191642oob.11.2024.03.19.09.43.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Mar 2024 09:43:13 -0700 (PDT) From: Denis Kenzior To: ofono@lists.linux.dev Cc: Denis Kenzior Subject: [PATCH 2/2] qmimodem: sms: Silence valgrind warning Date: Tue, 19 Mar 2024 11:42:54 -0500 Message-ID: <20240319164309.2676887-2-denkenz@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240319164309.2676887-1-denkenz@gmail.com> References: <20240319164309.2676887-1-denkenz@gmail.com> Precedence: bulk X-Mailing-List: ofono@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 ofonod[2670789]: drivers/qmimodem/sms.c:get_msg_protocol_cb() ==2670789== Conditional jump or move depends on uninitialised value(s) ==2670789== at 0x46552A: get_msg_protocol_cb (sms.c:565) ==2670789== by 0x45D5C1: service_send_callback (qmi.c:2762) ==2670789== by 0x4594F5: __rx_message (qmi.c:846) ==2670789== by 0x45A6A4: received_qmux_data (qmi.c:1393) ==2670789== by 0x58D71C: io_callback (io.c:105) ==2670789== by 0x58C073: l_main_iterate (main.c:461) ==2670789== by 0x500EC0: event_check (main.c:190) ==2670789== by 0x48FC09D: ??? (in /usr/lib/libglib-2.0.so.0.7800.3) ==2670789== by 0x49591CF: ??? (in /usr/lib/libglib-2.0.so.0.7800.3) ==2670789== by 0x48FBB96: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.7800.3) ==2670789== by 0x5011F5: main (main.c:284) ==2670789== ofonod[2670789]: drivers/qmimodem/sms.c:get_msg_list() The warning is triggered because GET_MSG_PROTOCOL command succeeds and qmi_result_set_error() returns false. It seems the intent in this case is to use the msg_mode reported by the device by obtaining it using qmi_result_get_uint8. In case GET_MSG_PROTOCOL command fails, both CDMA and WCDMA messages should be queried. --- drivers/qmimodem/sms.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/qmimodem/sms.c b/drivers/qmimodem/sms.c index 498a6d778303..b9fe963520d6 100644 --- a/drivers/qmimodem/sms.c +++ b/drivers/qmimodem/sms.c @@ -556,21 +556,23 @@ static void get_msg_protocol_cb(struct qmi_result *result, void *user_data) DBG(""); - if (qmi_result_set_error(result, &err) && - (err != QMI_ERR_OP_DEVICE_UNSUPPORTED)) { - DBG("Err: protocol %d - %s", err, qmi_result_get_error(result)); - return; - } + if (qmi_result_set_error(result, &err)) { + if (err != QMI_ERR_OP_DEVICE_UNSUPPORTED) { + DBG("Err: protocol %d - %s", + err, qmi_result_get_error(result)); + return; + } - if (err != QMI_ERR_OP_DEVICE_UNSUPPORTED) { - /* modem supports only 1 protocol */ - qmi_result_get_uint8(result, QMI_WMS_PARAM_PROTOCOL, - &data->msg_mode); - } else { - /* check both, start with 1 then switch to other */ - DBG("device supports CDMA and WCDMA msg protocol"); + /* Get Message Protocol operation is not supported */ + DBG("query both CDMA and WCDMA"); data->msg_mode_all = true; data->msg_mode = QMI_WMS_MESSAGE_MODE_CDMA; + } else { + /* Query of current protocol succeeded, use that */ + qmi_result_get_uint8(result, QMI_WMS_PARAM_PROTOCOL, + &data->msg_mode); + + DBG("msg_mode: %s", data->msg_mode ? "WCDMA" : "CDMA"); } /* check for messages */