sim: Check SIM pin status after changing pin.
PATCH 1/1 sim: Check SIM pin status after changing pin.
From: Marit Henriksen <[email protected]>
When changing pin, it is possible to get in a state where the modem requests puk (if incorrect pin is entered too many times). Need to check the SIM pin status to discover this.
--- src/sim.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)
src/sim.c +8 -7
@@ -628,15 +628,16 @@ static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,static void sim_change_pin_cb (const struct ofono_error *error,void *data) {struct ofono_sim *sim = data; +DBusMessage *reply; -if (error->type !=OFONO_ERROR_TYPE_NO_ERROR ) { -__ofono_dbus_pending_reply (&sim->pending, -__ofono_error_failed (sim->pending)); -return ; - } +if (error->type !=OFONO_ERROR_TYPE_NO_ERROR ) + reply =__ofono_error_failed (sim->pending); +else + reply =dbus_message_new_method_return (sim->pending); -__ofono_dbus_pending_reply (&sim->pending, -dbus_message_new_method_return (sim->pending)); +__ofono_dbus_pending_reply (&sim->pending, reply); + +sim_pin_check (sim); } static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
Signature · 1 line
1.7.0.4
Hi Marit,
Quoted 24 lines · Marit Henriksen
From: Marit Henriksen <[email protected]>
When changing pin, it is possible to get in a state where the modem requests puk (if incorrect pin is entered too many times). Need to check the SIM pin status to discover this.
--- src/sim.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)src/sim.c +1
-5@@ -628,15 +628,16 @@ static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,static void sim_change_pin_cb (const struct ofono_error *error,void *data) {struct ofono_sim *sim = data; +DBusMessage *reply; -if (error->type !=OFONO_ERROR_TYPE_NO_ERROR ) { -__ofono_dbus_pending_reply (&sim->pending, -__ofono_error_failed (sim->pending)); -return ; - }
src/sim.c static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg, · line 640
Quoted 10 lines
src/sim.c +7
-2+if (error->type !=OFONO_ERROR_TYPE_NO_ERROR ) + reply =__ofono_error_failed (sim->pending); +else + reply =dbus_message_new_method_return (sim->pending); -__ofono_dbus_pending_reply (&sim->pending, -dbus_message_new_method_return (sim->pending)); +__ofono_dbus_pending_reply (&sim->pending, reply); + +sim_pin_check (sim);
I don't think that running sim_pin_check is such a good idea. That function initializes the sim interface when +CPIN returns READY.
You also need to tear down the modem state back to pre-sim if the modem asks for a PUK after change pin fails. That entails removing all post_sim/post_online atoms and bring the modem back to the offline state ;)
src/sim.c static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg, · line 643
Quoted 3 lines
src/sim.c +0
-0} static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage *msg,
Regards,
-Denis
Hi Denis.
OK, I can see that this approach was rather too simplistic... Maybe checking the returned CME error would be a better idea?
I’m a bit unsure how (or if) to proceed with this. As this is in the core, and not our driver, it might be more on your table? If you think it is important.
Br Marit
2010/11/2 Denis Kenzior <[email protected]>
Quoted 58 lines
Hi Marit,
Quoted 3 lines · Marit Henriksen
From: Marit Henriksen <[email protected]>
When changing pin, it is possible to get in a state where the modem
requests puk
Quoted 1 line
(if incorrect pin is entered too many times). Need to check the SIM pin
status
src/sim.c static DBusMessage *sim_unlock_pin(DBusConnection
Quoted 10 lines
to discover this.
--- src/sim.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)src/sim.c +0
-0@@ -628,15 +628,16 @@ static DBusMessage *sim_unlock_pin(DBusConnection*conn, DBusMessage *msg,
src/sim.c static DBusMessage *sim_unlock_pin(DBusConnection · line 628
Quoted 1 line
src/sim.c +0
-0static void sim_change_pin_cb(const struct ofono_error *error, void*data)
Quoted 6 lines
src/sim.c +1
-2{struct ofono_sim *sim = data; +DBusMessage *reply; -if (error->type !=OFONO_ERROR_TYPE_NO_ERROR ) { - __ofono_dbus_pending_reply(&sim->pending,src/sim.c static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,
Quoted 10 lines
src/sim.c +4
-5- __ofono_error_failed(sim->pending)); -return ; - } +if (error->type !=OFONO_ERROR_TYPE_NO_ERROR ) + reply =__ofono_error_failed (sim->pending); +else + reply =dbus_message_new_method_return (sim->pending); - __ofono_dbus_pending_reply(&sim->pending, -
dbus_message_new_method_return(sim->pending));src/sim.c static DBusMessage *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg, · line 640
Quoted 3 lines
src/sim.c +3
-0+__ofono_dbus_pending_reply (&sim->pending, reply); + +sim_pin_check (sim);I don't think that running
sim_pin_checkis such a good idea. That function initializes the sim interface when +CPIN returns READY.You also need to tear down the modem state back to pre-sim if the modem asks for a PUK after change pin fails. That entails removing all post_sim/post_online atoms and bring the modem back to the offline state ;)
src/sim.c static DBusMessage *sim_unlock_pin(DBusConnection · line 643
Quoted 3 lines
src/sim.c +0
-0} static DBusMessage *sim_change_pin(DBusConnection *conn, DBusMessage*msg,
Regards,
-Denis
Hi Marit,
Just a gentle reminder not to top-post on this mailing list.
Quoted 7 lines · Marit Sofie Henriksen
Hi Denis.
OK, I can see that this approach was rather too simplistic... Maybe checking the returned CME error would be a better idea?
I’m a bit unsure how (or if) to proceed with this. As this is in the core, and not our driver, it might be more on your table? If you think it is important.
Well this is definitely something we need to get right. So feel free to send a patch to the TODO file adding this task. The description is basically what I outlined in my earlier reply. E.g. if change_pin fails, we should check PIN state. If it is anything other than READY, tear the state back to pre_sim state and set the modem offline.
this task can go through the regular task lifecycle. If you guys feel up to it, this is certainly something you can work on / contribute.
Regards,
-Denis
Hi Marit,
as Denis mentioned and I stated many times, please don't top post on this mailing list. It is against our mailing list etiquette.
Quoted 5 lines
OK, I can see that this approach was rather too simplistic... Maybe checking the returned CME error would be a better idea?
I’m a bit unsure how (or if) to proceed with this. As this is in the core, and not our driver, it might be more on your table? If you think it is important.
So if I follow this logic, then it does mean we should not be fixing issues with the STE driver anymore? And not allowing you to use the generic AT modem and generic MBM modem drivers? Since these are strictly speaking not STE driver either.
This is not how this works. Everybody works for the greater good to improve oFono. This is the way how it makes progress. The core is not a fixed piece of software. It evolves with the modems we support. If it needs to be changed or improved, then that is a task you should take up.
And as explained by Denis, if you have found such a problem area inside the core, the generic drivers or anywhere else, write it down as part of the TODO list. So patches are highly encouraged.
Regards
Marcel