Re: [PATCH v3] atmodem: Query the list of supported s from the modem
Hi Slava, On 10/23/2017 03:52 PM, Slava Monich wrote: Not all modems support all s (particularly, "PS"), let's be polite and not ask them for the ones they don't support. --- drivers/atmodem/sim.c | 57 --- 1 file changed, 45 insertions(+), 12 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] gatmux: Remove write watch source at shutdown
Hi Slava, On 10/23/2017 04:05 PM, Slava Monich wrote: Otherwise write_watcher_destroy_notify can be invoked after GAtMux has been deallocated which results in write after free: ==3952== Invalid write of size 4 ==3952==at 0xABF54: write_watcher_destroy_notify (gatmux.c:285) ==3952==by 0x4AF21E7: g_source_callback_unref (gmain.c:1561) ==3952==by 0x4AF2E53: g_source_destroy_internal.constprop.8 (gmain.c:1207) ==3952==by 0x4AF61CF: g_main_dispatch (gmain.c:3177) ==3952==by 0x4AF61CF: g_main_context_dispatch (gmain.c:3769) ==3952==by 0x4AF658F: g_main_loop_run (gmain.c:4034) ==3952==by 0xBDDBB: main (main.c:261) ==3952== Address 0x50c6cb0 is 8 bytes inside a block of size 4,396 free'd ==3952==at 0x4840B28: free (vg_replace_malloc.c:530) ==3952==by 0xACB53: g_at_mux_unref (gatmux.c:642) ==3952== Block was alloc'd at ==3952==at 0x4841BF0: calloc (vg_replace_malloc.c:711) ==3952==by 0xAC9DF: g_at_mux_new (gatmux.c:603) ==3952==by 0xADF2F: g_at_mux_new_gsm0710_basic (gatmux.c:1160) --- gatchat/gatmux.c | 3 +++ 1 file changed, 3 insertions(+) Applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
[PATCH] gatmux: Remove write watch source at shutdown
Otherwise write_watcher_destroy_notify can be invoked after GAtMux has been deallocated which results in write after free: ==3952== Invalid write of size 4 ==3952==at 0xABF54: write_watcher_destroy_notify (gatmux.c:285) ==3952==by 0x4AF21E7: g_source_callback_unref (gmain.c:1561) ==3952==by 0x4AF2E53: g_source_destroy_internal.constprop.8 (gmain.c:1207) ==3952==by 0x4AF61CF: g_main_dispatch (gmain.c:3177) ==3952==by 0x4AF61CF: g_main_context_dispatch (gmain.c:3769) ==3952==by 0x4AF658F: g_main_loop_run (gmain.c:4034) ==3952==by 0xBDDBB: main (main.c:261) ==3952== Address 0x50c6cb0 is 8 bytes inside a block of size 4,396 free'd ==3952==at 0x4840B28: free (vg_replace_malloc.c:530) ==3952==by 0xACB53: g_at_mux_unref (gatmux.c:642) ==3952== Block was alloc'd at ==3952==at 0x4841BF0: calloc (vg_replace_malloc.c:711) ==3952==by 0xAC9DF: g_at_mux_new (gatmux.c:603) ==3952==by 0xADF2F: g_at_mux_new_gsm0710_basic (gatmux.c:1160) --- gatchat/gatmux.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gatchat/gatmux.c b/gatchat/gatmux.c index 909eca6..d492edb 100644 --- a/gatchat/gatmux.c +++ b/gatchat/gatmux.c @@ -684,6 +684,9 @@ gboolean g_at_mux_shutdown(GAtMux *mux) if (mux->read_watch > 0) g_source_remove(mux->read_watch); + if (mux->write_watch > 0) + g_source_remove(mux->write_watch); + for (i = 0; i < MAX_CHANNELS; i++) { if (mux->dlcs[i] == NULL) continue; -- 1.9.1 ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
[PATCH v3] atmodem: Query the list of supported s from the modem
Not all modems support all s (particularly, "PS"), let's be polite and not ask them for the ones they don't support. --- drivers/atmodem/sim.c | 57 --- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c index 6395a04..8665274 100644 --- a/drivers/atmodem/sim.c +++ b/drivers/atmodem/sim.c @@ -51,6 +51,7 @@ struct sim_data { GAtChat *chat; unsigned int vendor; guint ready_id; + guint passwd_type_mask; struct at_util_sim_state_query *sim_state_query; }; @@ -1459,9 +1460,8 @@ static void at_pin_enable(struct ofono_sim *sim, struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; int ret; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL) + if (!(sd->passwd_type_mask & (1 << passwd_type))) goto error; snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"", @@ -1490,10 +1490,8 @@ static void at_change_passwd(struct ofono_sim *sim, struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; int ret; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || - at_clck_cpwd_fac[passwd_type] == NULL) + if (!(sd->passwd_type_mask & (1 << passwd_type))) goto error; snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"", @@ -1550,9 +1548,8 @@ static void at_query_clck(struct ofono_sim *sim, struct sim_data *sd = ofono_sim_get_data(sim); struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL) + if (!(sd->passwd_type_mask & (1 << passwd_type))) goto error; snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2", @@ -1568,13 +1565,42 @@ error: CALLBACK_WITH_FAILURE(cb, -1, data); } -static gboolean at_sim_register(gpointer user) +static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user) { struct ofono_sim *sim = user; + struct sim_data *sd = ofono_sim_get_data(sim); + GAtResultIter iter; + const char *fac; - ofono_sim_register(sim); + if (!ok) + goto done; + + g_at_result_iter_init(&iter, result); + + /* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */ + if (!g_at_result_iter_next(&iter, "+CLCK:") || + !g_at_result_iter_open_list(&iter)) + goto done; + + /* Clear the default mask */ + sd->passwd_type_mask = 0; - return FALSE; + /* Set the bits for s that are actually supported */ + while (g_at_result_iter_next_string(&iter, &fac)) { + unsigned int i; + + /* Find it in the list of known s */ + for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) { + if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) { + sd->passwd_type_mask |= (1 << i); + DBG("found %s", fac); + break; + } + } + } + +done: + ofono_sim_register(sim); } static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor, @@ -1582,6 +1608,7 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor, { GAtChat *chat = data; struct sim_data *sd; + unsigned int i; sd = g_new0(struct sim_data, 1); sd->chat = g_at_chat_clone(chat); @@ -1591,9 +1618,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor, g_at_chat_send(sd->chat, "AT*EPEE=1", NULL, NULL, NULL, NULL); ofono_sim_set_data(sim, sd); - g_idle_add(at_sim_register, sim); - return 0; + /* s supported by default */ + for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) + if (at_clck_cpwd_fac[i]) + sd->passwd_type_mask |= (1 << i); + + /* Query supported s */ + return g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix, + at_clck_query_cb, sim, NULL) ? 0 : -1; } static void at_sim_remove(struct ofono_sim *sim) -- 1.9.1 ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH v2] atmodem: Query the list of supported s from the modem
On 23/10/17 23:33, Denis Kenzior wrote: Hi Slava, On 10/23/2017 03:21 PM, Slava Monich wrote: Not all modems support all s (particularly, "PS"), let's be polite and not ask them for the ones they don't support. --- drivers/atmodem/sim.c | 57 --- 1 file changed, 45 insertions(+), 12 deletions(-) drivers/atmodem/sim.c: In function ‘at_clck_query_cb’: drivers/atmodem/sim.c:1593:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) { ^ drivers/atmodem/sim.c: In function ‘at_sim_probe’: drivers/atmodem/sim.c:1623:16: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) Apparently we are using slightly different compilers. Will fix. -Slava ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH v2] atmodem: Query the list of supported s from the modem
Hi Slava, On 10/23/2017 03:21 PM, Slava Monich wrote: Not all modems support all s (particularly, "PS"), let's be polite and not ask them for the ones they don't support. --- drivers/atmodem/sim.c | 57 --- 1 file changed, 45 insertions(+), 12 deletions(-) drivers/atmodem/sim.c: In function ‘at_clck_query_cb’: drivers/atmodem/sim.c:1593:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) { ^ drivers/atmodem/sim.c: In function ‘at_sim_probe’: drivers/atmodem/sim.c:1623:16: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) ^ drivers/atmodem/sim.c: At top level: cc1: error: unrecognized command line option ‘-Wno-format-truncation’ [-Werror] Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] gatmux: Remove finalized watches from the list
Hi Slava, On 10/23/2017 04:17 AM, Slava Monich wrote: Leaving them there may result in invalid reads like this: ==2312== Invalid read of size 4 ==2312==at 0xAB8C0: dispatch_sources (gatmux.c:134) ==2312==by 0xAC5D3: channel_close (gatmux.c:479) ==2312==by 0x4AE8885: g_io_channel_shutdown (giochannel.c:523) ==2312==by 0x4AE8A1D: g_io_channel_unref (giochannel.c:240) ==2312==by 0xAC423: watch_finalize (gatmux.c:426) ==2312==by 0x4AF2CC9: g_source_unref_internal (gmain.c:2048) ==2312==by 0x4AF44E1: g_source_destroy_internal (gmain.c:1230) ==2312==by 0x4AF44E1: g_source_destroy (gmain.c:1256) ==2312==by 0x4AF5257: g_source_remove (gmain.c:2282) ==2312==by 0xAB5CB: io_shutdown (gatio.c:325) ==2312==by 0xAB667: g_at_io_unref (gatio.c:345) ==2312==by 0xA72C7: at_chat_unref (gatchat.c:972) ==2312==by 0xA829B: g_at_chat_unref (gatchat.c:1446) ==2312== Address 0x51420f0 is 56 bytes inside a block of size 60 free'd ==2312==at 0x4840B28: free (vg_replace_malloc.c:530) ==2312==by 0x4AF2D33: g_source_unref_internal (gmain.c:2075) ==2312==by 0x4AF44E1: g_source_destroy_internal (gmain.c:1230) ==2312==by 0x4AF44E1: g_source_destroy (gmain.c:1256) ==2312==by 0x4AF5257: g_source_remove (gmain.c:2282) ==2312==by 0xAB46B: g_at_io_set_write_handler (gatio.c:283) ==2312==by 0xA713F: at_chat_suspend (gatchat.c:938) ==2312==by 0xA72B7: at_chat_unref (gatchat.c:971) ==2312==by 0xA829B: g_at_chat_unref (gatchat.c:1446) ==2312== Block was alloc'd at ==2312==at 0x4841BF0: calloc (vg_replace_malloc.c:711) ==2312==by 0x4AFB117: g_malloc0 (gmem.c:124) ==2312==by 0x4AF401F: g_source_new (gmain.c:892) ==2312==by 0xAC6A7: channel_create_watch (gatmux.c:506) ==2312==by 0x4AE7C4F: g_io_add_watch_full (giochannel.c:649) ==2312==by 0xAB4EB: g_at_io_set_write_handler (gatio.c:297) ==2312==by 0xA7103: chat_wakeup_writer (gatchat.c:931) ==2312==by 0xA753F: at_chat_send_common (gatchat.c:1045) ==2312==by 0xA850F: g_at_chat_send (gatchat.c:1502) It's also necessary to add additional references to the sources for the duration of the dispatch_sources loop because any source can be removed when any callback is invoked (and not necessarily the one being dispatched). --- gatchat/gatmux.c | 109 +++ 1 file changed, 77 insertions(+), 32 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
[PATCH v2] atmodem: Query the list of supported s from the modem
Not all modems support all s (particularly, "PS"), let's be polite and not ask them for the ones they don't support. --- drivers/atmodem/sim.c | 57 --- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c index 6395a04..06bfb90 100644 --- a/drivers/atmodem/sim.c +++ b/drivers/atmodem/sim.c @@ -51,6 +51,7 @@ struct sim_data { GAtChat *chat; unsigned int vendor; guint ready_id; + guint passwd_type_mask; struct at_util_sim_state_query *sim_state_query; }; @@ -1459,9 +1460,8 @@ static void at_pin_enable(struct ofono_sim *sim, struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; int ret; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL) + if (!(sd->passwd_type_mask & (1 << passwd_type))) goto error; snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"", @@ -1490,10 +1490,8 @@ static void at_change_passwd(struct ofono_sim *sim, struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; int ret; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || - at_clck_cpwd_fac[passwd_type] == NULL) + if (!(sd->passwd_type_mask & (1 << passwd_type))) goto error; snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"", @@ -1550,9 +1548,8 @@ static void at_query_clck(struct ofono_sim *sim, struct sim_data *sd = ofono_sim_get_data(sim); struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL) + if (!(sd->passwd_type_mask & (1 << passwd_type))) goto error; snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2", @@ -1568,13 +1565,42 @@ error: CALLBACK_WITH_FAILURE(cb, -1, data); } -static gboolean at_sim_register(gpointer user) +static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user) { struct ofono_sim *sim = user; + struct sim_data *sd = ofono_sim_get_data(sim); + GAtResultIter iter; + const char *fac; - ofono_sim_register(sim); + if (!ok) + goto done; + + g_at_result_iter_init(&iter, result); + + /* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */ + if (!g_at_result_iter_next(&iter, "+CLCK:") || + !g_at_result_iter_open_list(&iter)) + goto done; + + /* Clear the default mask */ + sd->passwd_type_mask = 0; - return FALSE; + /* Set the bits for s that are actually supported */ + while (g_at_result_iter_next_string(&iter, &fac)) { + int i; + + /* Find it in the list of known s */ + for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) { + if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) { + sd->passwd_type_mask |= (1 << i); + DBG("found %s", fac); + break; + } + } + } + +done: + ofono_sim_register(sim); } static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor, @@ -1582,6 +1608,7 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor, { GAtChat *chat = data; struct sim_data *sd; + int i; sd = g_new0(struct sim_data, 1); sd->chat = g_at_chat_clone(chat); @@ -1591,9 +1618,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor, g_at_chat_send(sd->chat, "AT*EPEE=1", NULL, NULL, NULL, NULL); ofono_sim_set_data(sim, sd); - g_idle_add(at_sim_register, sim); - return 0; + /* s supported by default */ + for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) + if (at_clck_cpwd_fac[i]) + sd->passwd_type_mask |= (1 << i); + + /* Query supported s */ + return g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix, + at_clck_query_cb, sim, NULL) ? 0 : -1; } static void at_sim_remove(struct ofono_sim *sim) -- 1.9.1 ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] atmodem: Query the list of supported s from the modem
Hi Slava, On 10/23/2017 12:57 PM, Slava Monich wrote: Not all modems support all s (particularly, "PS"), let's be polite and not ask them for the ones they don't support. --- drivers/atmodem/sim.c | 69 ++- 1 file changed, 57 insertions(+), 12 deletions(-) diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c index 6395a04..effce6e 100644 --- a/drivers/atmodem/sim.c +++ b/drivers/atmodem/sim.c @@ -52,6 +52,7 @@ struct sim_data { unsigned int vendor; guint ready_id; struct at_util_sim_state_query *sim_state_query; + const char *passwd_fac[OFONO_SIM_PASSWORD_INVALID]; Okay, but why not just make this a bitmap instead? I think we have like 15 or 16 SIM passwords, so this could be all of 2 to 4 bytes. Or use idmap... }; static const char *crsm_prefix[] = { "+CRSM:", NULL }; @@ -1439,7 +1440,7 @@ static void at_lock_unlock_cb(gboolean ok, GAtResult *result, cb(&error, cbd->data); } -static const char *const at_clck_cpwd_fac[] = { +static const char *const at_clck_cpwd_fac[OFONO_SIM_PASSWORD_INVALID] = { Is this really needed? ARRAY_SIZE should still work... [OFONO_SIM_PASSWORD_SIM_PIN] = "SC", [OFONO_SIM_PASSWORD_SIM_PIN2] = "P2", [OFONO_SIM_PASSWORD_PHSIM_PIN] = "PS", @@ -1459,13 +1460,13 @@ static void at_pin_enable(struct ofono_sim *sim, struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; int ret; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL) + if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) || + sd->passwd_fac[passwd_type] == NULL) goto error; snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"", - at_clck_cpwd_fac[passwd_type], enable ? 1 : 0, passwd); + sd->passwd_fac[passwd_type], enable ? 1 : 0, passwd); at_clck_cpwd_fac and passwd_fac contain the same info at a given index (if valid). So this chunk can be omitted. See using a bitmap above... ret = g_at_chat_send(sd->chat, buf, none_prefix, at_lock_unlock_cb, cbd, g_free); @@ -1490,14 +1491,13 @@ static void at_change_passwd(struct ofono_sim *sim, struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; int ret; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || - at_clck_cpwd_fac[passwd_type] == NULL) + if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) || + sd->passwd_fac[passwd_type] == NULL) goto error; snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"", - at_clck_cpwd_fac[passwd_type], old_passwd, new_passwd); + sd->passwd_fac[passwd_type], old_passwd, new_passwd); ret = g_at_chat_send(sd->chat, buf, none_prefix, at_lock_unlock_cb, cbd, g_free); @@ -1550,13 +1550,13 @@ static void at_query_clck(struct ofono_sim *sim, struct sim_data *sd = ofono_sim_get_data(sim); struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL) + if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) || + sd->passwd_fac[passwd_type] == NULL) goto error; snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2", - at_clck_cpwd_fac[passwd_type]); + sd->passwd_fac[passwd_type]); if (g_at_chat_send(sd->chat, buf, clck_prefix, at_lock_status_cb, cbd, g_free) > 0) @@ -1568,6 +1568,43 @@ error: CALLBACK_WITH_FAILURE(cb, -1, data); } +static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user) +{ + struct ofono_sim *sim = user; + struct sim_data *sd = ofono_sim_get_data(sim); + GAtResultIter iter; + const char *fac; + + if (!ok) + goto done; + + g_at_result_iter_init(&iter, result); + + /* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */ + if (!g_at_result_iter_next(&iter, "+CLCK:") || + !g_at_result_iter_open_list(&iter)) + goto done; + + memset(sd->passwd_fac, 0, sizeof(sd->passwd_fac)); + + while (g_at_result_iter_next_string(&iter, &fac)) { + int i; + + /* Find it in the list of known s */ + for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) { + if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) { + DBG("found %s", fac); + /* at_clck_cpwd_fac[i] is a stat
[PATCH] atmodem: Query the list of supported s from the modem
Not all modems support all s (particularly, "PS"), let's be polite and not ask them for the ones they don't support. --- drivers/atmodem/sim.c | 69 ++- 1 file changed, 57 insertions(+), 12 deletions(-) diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c index 6395a04..effce6e 100644 --- a/drivers/atmodem/sim.c +++ b/drivers/atmodem/sim.c @@ -52,6 +52,7 @@ struct sim_data { unsigned int vendor; guint ready_id; struct at_util_sim_state_query *sim_state_query; + const char *passwd_fac[OFONO_SIM_PASSWORD_INVALID]; }; static const char *crsm_prefix[] = { "+CRSM:", NULL }; @@ -1439,7 +1440,7 @@ static void at_lock_unlock_cb(gboolean ok, GAtResult *result, cb(&error, cbd->data); } -static const char *const at_clck_cpwd_fac[] = { +static const char *const at_clck_cpwd_fac[OFONO_SIM_PASSWORD_INVALID] = { [OFONO_SIM_PASSWORD_SIM_PIN] = "SC", [OFONO_SIM_PASSWORD_SIM_PIN2] = "P2", [OFONO_SIM_PASSWORD_PHSIM_PIN] = "PS", @@ -1459,13 +1460,13 @@ static void at_pin_enable(struct ofono_sim *sim, struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; int ret; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL) + if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) || + sd->passwd_fac[passwd_type] == NULL) goto error; snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"", - at_clck_cpwd_fac[passwd_type], enable ? 1 : 0, passwd); + sd->passwd_fac[passwd_type], enable ? 1 : 0, passwd); ret = g_at_chat_send(sd->chat, buf, none_prefix, at_lock_unlock_cb, cbd, g_free); @@ -1490,14 +1491,13 @@ static void at_change_passwd(struct ofono_sim *sim, struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; int ret; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || - at_clck_cpwd_fac[passwd_type] == NULL) + if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) || + sd->passwd_fac[passwd_type] == NULL) goto error; snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"", - at_clck_cpwd_fac[passwd_type], old_passwd, new_passwd); + sd->passwd_fac[passwd_type], old_passwd, new_passwd); ret = g_at_chat_send(sd->chat, buf, none_prefix, at_lock_unlock_cb, cbd, g_free); @@ -1550,13 +1550,13 @@ static void at_query_clck(struct ofono_sim *sim, struct sim_data *sd = ofono_sim_get_data(sim); struct cb_data *cbd = cb_data_new(cb, data); char buf[64]; - unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac); - if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL) + if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) || + sd->passwd_fac[passwd_type] == NULL) goto error; snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2", - at_clck_cpwd_fac[passwd_type]); + sd->passwd_fac[passwd_type]); if (g_at_chat_send(sd->chat, buf, clck_prefix, at_lock_status_cb, cbd, g_free) > 0) @@ -1568,6 +1568,43 @@ error: CALLBACK_WITH_FAILURE(cb, -1, data); } +static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user) +{ + struct ofono_sim *sim = user; + struct sim_data *sd = ofono_sim_get_data(sim); + GAtResultIter iter; + const char *fac; + + if (!ok) + goto done; + + g_at_result_iter_init(&iter, result); + + /* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */ + if (!g_at_result_iter_next(&iter, "+CLCK:") || + !g_at_result_iter_open_list(&iter)) + goto done; + + memset(sd->passwd_fac, 0, sizeof(sd->passwd_fac)); + + while (g_at_result_iter_next_string(&iter, &fac)) { + int i; + + /* Find it in the list of known s */ + for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) { + if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) { + DBG("found %s", fac); + /* at_clck_cpwd_fac[i] is a static string */ + sd->passwd_fac[i] = at_clck_cpwd_fac[i]; + break; + } + } + } + +done: + ofono_sim_register(sim); +} + static gboolean at_sim_register(gpointer user) { struct ofono_sim *sim = user; @@ -1591,7 +1628,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
Re: [PATCHv2] simauth: fixup adding more dbus return checks
Hi James, On 16/10/17 19:06, James Prestwood wrote: --- src/sim-auth.c | 23 +-- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/sim-auth.c b/src/sim-auth.c index b9552b5..9ae5574 100644 --- a/src/sim-auth.c +++ b/src/sim-auth.c @@ -427,6 +427,7 @@ static DBusMessage *usim_gsm_authenticate(DBusConnection *conn, DBusMessageIter array; int i; struct sim_app_record *app; + int rands; if (sim->pending) return __ofono_error_busy(msg); @@ -436,11 +437,16 @@ static DBusMessage *usim_gsm_authenticate(DBusConnection *conn, if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY) return __ofono_error_invalid_format(msg); + rands = dbus_message_iter_get_element_count(&iter); It appears that dbus_message_iter_get_element_count() requires dbus 1.9.16 or later, would it be possible to avoid it? Thanks, -Slava ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
[PATCH] gatmux: Remove finalized watches from the list
Leaving them there may result in invalid reads like this: ==2312== Invalid read of size 4 ==2312==at 0xAB8C0: dispatch_sources (gatmux.c:134) ==2312==by 0xAC5D3: channel_close (gatmux.c:479) ==2312==by 0x4AE8885: g_io_channel_shutdown (giochannel.c:523) ==2312==by 0x4AE8A1D: g_io_channel_unref (giochannel.c:240) ==2312==by 0xAC423: watch_finalize (gatmux.c:426) ==2312==by 0x4AF2CC9: g_source_unref_internal (gmain.c:2048) ==2312==by 0x4AF44E1: g_source_destroy_internal (gmain.c:1230) ==2312==by 0x4AF44E1: g_source_destroy (gmain.c:1256) ==2312==by 0x4AF5257: g_source_remove (gmain.c:2282) ==2312==by 0xAB5CB: io_shutdown (gatio.c:325) ==2312==by 0xAB667: g_at_io_unref (gatio.c:345) ==2312==by 0xA72C7: at_chat_unref (gatchat.c:972) ==2312==by 0xA829B: g_at_chat_unref (gatchat.c:1446) ==2312== Address 0x51420f0 is 56 bytes inside a block of size 60 free'd ==2312==at 0x4840B28: free (vg_replace_malloc.c:530) ==2312==by 0x4AF2D33: g_source_unref_internal (gmain.c:2075) ==2312==by 0x4AF44E1: g_source_destroy_internal (gmain.c:1230) ==2312==by 0x4AF44E1: g_source_destroy (gmain.c:1256) ==2312==by 0x4AF5257: g_source_remove (gmain.c:2282) ==2312==by 0xAB46B: g_at_io_set_write_handler (gatio.c:283) ==2312==by 0xA713F: at_chat_suspend (gatchat.c:938) ==2312==by 0xA72B7: at_chat_unref (gatchat.c:971) ==2312==by 0xA829B: g_at_chat_unref (gatchat.c:1446) ==2312== Block was alloc'd at ==2312==at 0x4841BF0: calloc (vg_replace_malloc.c:711) ==2312==by 0x4AFB117: g_malloc0 (gmem.c:124) ==2312==by 0x4AF401F: g_source_new (gmain.c:892) ==2312==by 0xAC6A7: channel_create_watch (gatmux.c:506) ==2312==by 0x4AE7C4F: g_io_add_watch_full (giochannel.c:649) ==2312==by 0xAB4EB: g_at_io_set_write_handler (gatio.c:297) ==2312==by 0xA7103: chat_wakeup_writer (gatchat.c:931) ==2312==by 0xA753F: at_chat_send_common (gatchat.c:1045) ==2312==by 0xA850F: g_at_chat_send (gatchat.c:1502) It's also necessary to add additional references to the sources for the duration of the dispatch_sources loop because any source can be removed when any callback is invoked (and not necessarily the one being dispatched). --- gatchat/gatmux.c | 109 +++ 1 file changed, 77 insertions(+), 32 deletions(-) diff --git a/gatchat/gatmux.c b/gatchat/gatmux.c index 0e275b5..909eca6 100644 --- a/gatchat/gatmux.c +++ b/gatchat/gatmux.c @@ -116,66 +116,109 @@ static inline void debug(GAtMux *mux, const char *format, ...) static void dispatch_sources(GAtMuxChannel *channel, GIOCondition condition) { - GAtMuxWatch *source; GSList *c; GSList *p; - GSList *t; + GSList *refs; + + /* +* Don't reference destroyed sources, they may have zero reference +* count if this function is invoked from the source's finalize +* callback, in which case incrementing and then decrementing +* the count would result in double free (first when we decrement +* the reference count and then when we return from the finalize +* callback). +*/ p = NULL; - c = channel->sources; + refs = NULL; - while (c) { - gboolean destroy = FALSE; + for (c = channel->sources; c; c = c->next) { + GSource *s = c->data; + + if (!g_source_is_destroyed(s)) { + GSList *l = g_slist_append(NULL, g_source_ref(s)); + + if (p) + p->next = l; + else + refs = l; - source = c->data; + p = l; + } + } - debug(channel->mux, "checking source: %p", source); + /* +* Keep the references to all sources for the duration of the loop. +* Callbacks may add and remove the sources, i.e. channel->sources +* may keep changing during the loop. +*/ - if (condition & source->condition) { + for (c = refs; c; c = c->next) { + GAtMuxWatch *w = c->data; + GSource *s = &w->source; + + if (g_source_is_destroyed(s)) + continue; + + debug(channel->mux, "checking source: %p", s); + + if (condition & w->condition) { gpointer user_data = NULL; GSourceFunc callback = NULL; - GSourceCallbackFuncs *cb_funcs; - gpointer cb_data; - gboolean (*dispatch) (GSource *, GSourceFunc, gpointer); - - debug(channel->mux, "dispatching source: %p", source); + GSourceCallbackFuncs *cb_funcs = s->callback_funcs; + gpointer cb_data = s->callback_data; + gboolean destroy; -