From 8d3bf960a0194d4285874de718a2e04be4da6d9d Mon Sep 17 00:00:00 2001 From: Paul Kocialkowski Date: Sun, 31 Mar 2013 18:35:37 +0200 Subject: sec: Refactor code, check for NULL pointers and prevent memory leaks Signed-off-by: Paul Kocialkowski --- samsung-ril.h | 10 +-- sec.c | 254 +++++++++++++++++++++++++++++++--------------------------- 2 files changed, 142 insertions(+), 122 deletions(-) diff --git a/samsung-ril.h b/samsung-ril.h index 2aa53ca..570b2b9 100644 --- a/samsung-ril.h +++ b/samsung-ril.h @@ -303,13 +303,13 @@ void ril_request_sim_io(RIL_Token t, void *data, int length); void ipc_sec_rsim_access(struct ipc_message_info *info); void ipc_sec_sim_status_complete(struct ipc_message_info *info); void ipc_sec_lock_info(struct ipc_message_info *info); -void ril_request_enter_sim_pin(RIL_Token t, void *data, size_t datalen); -void ril_request_change_sim_pin(RIL_Token t, void *data, size_t datalen); -void ril_request_enter_sim_puk(RIL_Token t, void *data, size_t datalen); -void ril_request_query_facility_lock(RIL_Token t, void *data, size_t datalen); +void ril_request_enter_sim_pin(RIL_Token t, void *data, size_t length); +void ril_request_change_sim_pin(RIL_Token t, void *data, size_t length); +void ril_request_enter_sim_puk(RIL_Token t, void *data, size_t length); +void ril_request_query_facility_lock(RIL_Token t, void *data, size_t length); void ipc_sec_phone_lock(struct ipc_message_info *info); void ipc_sec_phone_lock_complete(struct ipc_message_info *info); -void ril_request_set_facility_lock(RIL_Token t, void *data, size_t datalen); +void ril_request_set_facility_lock(RIL_Token t, void *data, size_t length); /* NET */ diff --git a/sec.c b/sec.c index 9487558..4ed0984 100644 --- a/sec.c +++ b/sec.c @@ -29,6 +29,9 @@ ril_sim_state ipc2ril_sim_state(struct ipc_sec_sim_status_response *pin_status) { + if (pin_status == NULL) + return -EINVAL; + switch(pin_status->status) { case IPC_SEC_SIM_STATUS_LOCK_SC: switch(pin_status->facility_lock) { @@ -70,13 +73,6 @@ ril_sim_state ipc2ril_sim_state(struct ipc_sec_sim_status_response *pin_status) } } -/* - * Update the radio state based on SIM state - * - * Out: RIL_UNSOL_RESPONSE_RADIO_STATE_CHANGED - * Indicate when value of RIL_RadioState has changed - * Callee will invoke RIL_RadioStateRequest method on main thread - */ void ril_state_update(ril_sim_state sim_state) { RIL_RadioState radio_state; @@ -132,6 +128,9 @@ void ipc2ril_card_status(struct ipc_sec_sim_status_response *pin_status, RIL_Car int app_index; int i; + if (pin_status == NULL || card_status == NULL) + return; + static RIL_AppStatus app_status_array[] = { /* SIM_ABSENT = 0 */ { RIL_APPTYPE_UNKNOWN, RIL_APPSTATE_UNKNOWN, RIL_PERSOSUBSTATE_UNKNOWN, @@ -204,22 +203,10 @@ void ril_tokens_pin_status_dump(void) \tril_data.tokens.pin_status = %p\n", ril_data.tokens.pin_status); } -/* - * In: IPC_SEC_SIM_STATUS - * Provides SIM initialization/lock status - * - * Out: RIL_UNSOL_RESPONSE_SIM_STATUS_CHANGED - * Indicates that SIM state changes. - * Callee will invoke RIL_REQUEST_GET_SIM_STATUS on main thread - * - * Out: RIL_UNSOL_RESPONSE_RADIO_STATE_CHANGED - * Indicate when value of RIL_RadioState has changed - * Callee will invoke RIL_RadioStateRequest method on main thread - */ void ipc_sec_sim_status(struct ipc_message_info *info) { - RIL_Token t = ril_request_get_token(info->aseq); - struct ipc_sec_sim_status_response *pin_status = (struct ipc_sec_sim_status_response *) info->data; + struct ipc_sec_sim_status_response *pin_status; + RIL_Token t; #if RIL_VERSION >= 6 RIL_CardStatus_v6 card_status; #else @@ -227,6 +214,12 @@ void ipc_sec_sim_status(struct ipc_message_info *info) #endif ril_sim_state sim_state; + if (info == NULL || info->data == NULL || info->length < sizeof(struct ipc_sec_sim_status_response)) + goto error; + + pin_status = (struct ipc_sec_sim_status_response *) info->data; + t = ril_request_get_token(info->aseq); + switch(info->type) { case IPC_TYPE_NOTI: // Don't consider this if modem isn't in normal power mode @@ -267,17 +260,19 @@ void ipc_sec_sim_status(struct ipc_message_info *info) ril_data.tokens.pin_status = (RIL_Token) 0x00; break; default: - LOGE("%s: unhandled ipc method: %d", __FUNCTION__, info->type); + LOGE("%s: unhandled ipc method: %d", __func__, info->type); break; } ril_tokens_pin_status_dump(); + + return; + +error: + if (info != NULL) + ril_request_complete(ril_request_get_token(info->aseq), RIL_E_GENERIC_FAILURE, NULL, 0); } -/* - * In: RIL_REQUEST_GET_SIM_STATUS - * Requests status of the SIM interface and the SIM card - */ void ril_request_get_sim_status(RIL_Token t) { struct ipc_sec_sim_status_response *pin_status; @@ -320,12 +315,18 @@ void ipc_sec_sim_icc_type(struct ipc_message_info *info) { struct ipc_sec_sim_icc_type *sim_icc_type; - if (info == NULL || info->length < sizeof(struct ipc_sec_sim_icc_type)) - return; + if (info == NULL || info->data == NULL || info->length < sizeof(struct ipc_sec_sim_icc_type)) + goto error; sim_icc_type = (struct ipc_sec_sim_icc_type *) info->data; memcpy(&ril_data.state.sim_icc_type, sim_icc_type, sizeof(struct ipc_sec_sim_icc_type)); + + return; + +error: + if (info != NULL) + ril_request_complete(ril_request_get_token(info->aseq), RIL_E_GENERIC_FAILURE, NULL, 0); } /* @@ -496,16 +497,6 @@ void ril_request_sim_io_complete(RIL_Token t, unsigned char command, unsigned sh free(rsim_access_data); } -/* - * In: RIL_REQUEST_SIM_IO - * Request SIM I/O operation. - * This is similar to the TS 27.007 "restricted SIM" operation - * where it assumes all of the EF selection will be done by the - * callee. - * - * Out: IPC_SEC_RSIM_ACCESS - * Performs a restricted SIM read operation - */ void ril_request_sim_io(RIL_Token t, void *data, int length) { struct ril_request_sim_io_info *sim_io_info = NULL; @@ -519,7 +510,7 @@ void ril_request_sim_io(RIL_Token t, void *data, int length) int rc; if (data == NULL || length < (int) sizeof(*sim_io)) - return; + goto error; #if RIL_VERSION >= 6 sim_io = (RIL_SIM_IO_v6 *) data; @@ -565,18 +556,13 @@ void ril_request_sim_io(RIL_Token t, void *data, int length) free(sim_io_data); sim_io_info->data = NULL; sim_io_info->length = 0; + + return; + +error: + ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } -/* - * In: IPC_SEC_RSIM_ACCESS - * Provides restricted SIM read operation result - * - * Out: RIL_REQUEST_SIM_IO - * Request SIM I/O operation. - * This is similar to the TS 27.007 "restricted SIM" operation - * where it assumes all of the EF selection will be done by the - * callee. - */ void ipc_sec_rsim_access(struct ipc_message_info *info) { struct ril_request_sim_io_info *sim_io_info; @@ -591,7 +577,7 @@ void ipc_sec_rsim_access(struct ipc_message_info *info) int i; if (info == NULL || info->data == NULL || info->length < sizeof(struct ipc_sec_rsim_access_response)) - return; + goto error; sim_io_info = ril_request_sim_io_info_find_token(ril_request_get_token(info->aseq)); if (sim_io_info == NULL) { @@ -714,21 +700,21 @@ void ipc_sec_rsim_access(struct ipc_message_info *info) // Send the next SIM I/O in the list ril_request_sim_io_next(); + + return; + +error: + if (info != NULL) + ril_request_complete(ril_request_get_token(info->aseq), RIL_E_GENERIC_FAILURE, NULL, 0); } -/* - * In: IPC_GEN_PHONE_RES - * Provides result of IPC_SEC_SIM_STATUS SET - * - * Out: RIL_REQUEST_ENTER_SIM_PIN - * Returns PIN SIM unlock result - */ void ipc_sec_sim_status_complete(struct ipc_message_info *info) { - struct ipc_gen_phone_res *phone_res = (struct ipc_gen_phone_res *) info->data; + struct ipc_gen_phone_res *phone_res; + int attempts = -1; int rc; - int attempts = -1; + phone_res = (struct ipc_gen_phone_res *) info->data; rc = ipc_gen_phone_res_check(phone_res); if (rc < 0) { @@ -752,71 +738,76 @@ void ipc_sec_sim_status_complete(struct ipc_message_info *info) ril_request_complete(ril_request_get_token(info->aseq), RIL_E_SUCCESS, &attempts, sizeof(attempts)); } -/* - * In: IPC_SEC_LOCK_INFO - * Provides number of retries left for a lock type - */ void ipc_sec_lock_info(struct ipc_message_info *info) { + struct ipc_sec_lock_info_response *lock_info; + int attempts; + + if (info == NULL || info->data == NULL || info->length < sizeof(struct ipc_sec_lock_info_response)) + return; + + lock_info = (struct ipc_sec_lock_info_response *) info->data; + /* * FIXME: solid way of handling lockinfo and sim unlock response together * so we can return the number of attempts left in respondSecPinStatus */ - int attempts; - struct ipc_sec_lock_info_response *lock_info = (struct ipc_sec_lock_info_response *) info->data; if (lock_info->type == IPC_SEC_PIN_TYPE_PIN1) { attempts = lock_info->attempts; - LOGD("%s: PIN1 %d attempts left", __FUNCTION__, attempts); + LOGD("%s: PIN1 %d attempts left", __func__, attempts); } else { - LOGE("%s: unhandled lock type %d", __FUNCTION__, lock_info->type); + LOGE("%s: unhandled lock type %d", __func__, lock_info->type); } } -/* - * In: RIL_REQUEST_ENTER_SIM_PIN - * Supplies SIM PIN. Only called if RIL_CardStatus has RIL_APPSTATE_PIN state - * - * Out: IPC_SEC_SIM_STATUS SET - * Attempts to unlock SIM PIN1 - * - * Out: IPC_SEC_LOCK_INFO - * Retrieves PIN1 lock status - */ -void ril_request_enter_sim_pin(RIL_Token t, void *data, size_t datalen) +void ril_request_enter_sim_pin(RIL_Token t, void *data, size_t length) { struct ipc_sec_pin_status_set pin_status; char *pin = ((char **) data)[0]; unsigned char buf[9]; - /* 1. Send PIN */ + if (data == NULL || length < (int) sizeof(char *)) + goto error; + + // 1. Send PIN if (strlen(data) > 16) { - LOGE("%s: pin exceeds maximum length", __FUNCTION__); + LOGE("%s: pin exceeds maximum length", __func__); ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } ipc_sec_pin_status_set_setup(&pin_status, IPC_SEC_PIN_TYPE_PIN1, pin, NULL); - ipc_gen_phone_res_expect_to_func(ril_request_get_id(t), IPC_SEC_SIM_STATUS, - ipc_sec_sim_status_complete); + ipc_gen_phone_res_expect_to_func(ril_request_get_id(t), IPC_SEC_SIM_STATUS, ipc_sec_sim_status_complete); ipc_fmt_send_set(IPC_SEC_SIM_STATUS, ril_request_get_id(t), (unsigned char *) &pin_status, sizeof(pin_status)); - /* 2. Get lock status */ + // 2. Get lock status // FIXME: This is not clean at all memset(buf, 0, sizeof(buf)); buf[0] = 1; buf[1] = IPC_SEC_PIN_TYPE_PIN1; ipc_fmt_send(IPC_SEC_LOCK_INFO, IPC_TYPE_GET, buf, sizeof(buf), ril_request_get_id(t)); + + return; + +error: + ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } -void ril_request_change_sim_pin(RIL_Token t, void *data, size_t datalen) +void ril_request_change_sim_pin(RIL_Token t, void *data, size_t length) { - char *password_old = ((char **) data)[0]; - char *password_new = ((char **) data)[1]; + char *password_old; + char *password_new; struct ipc_sec_change_locking_pw_set locking_pw; + if (data == NULL || length < (int) (2 * sizeof(char *))) + goto error; + + password_old = ((char **) data)[0]; + password_new = ((char **) data)[1]; + memset(&locking_pw, 0, sizeof(locking_pw)); locking_pw.facility = IPC_SEC_SIM_STATUS_LOCK_SC; @@ -837,13 +828,24 @@ void ril_request_change_sim_pin(RIL_Token t, void *data, size_t datalen) ipc_sec_sim_status_complete); ipc_fmt_send_set(IPC_SEC_CHANGE_LOCKING_PW, ril_request_get_id(t), (unsigned char *) &locking_pw, sizeof(locking_pw)); + + return; + +error: + ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } -void ril_request_enter_sim_puk(RIL_Token t, void *data, size_t datalen) +void ril_request_enter_sim_puk(RIL_Token t, void *data, size_t length) { struct ipc_sec_pin_status_set pin_status; - char *puk = ((char **) data)[0]; - char *pin = ((char **) data)[1]; + char *puk; + char *pin; + + if (data == NULL || length < (int) (2 * sizeof(char *))) + goto error; + + puk = ((char **) data)[0]; + pin = ((char **) data)[1]; ipc_sec_pin_status_set_setup(&pin_status, IPC_SEC_PIN_TYPE_PIN1, pin, puk); @@ -851,35 +853,42 @@ void ril_request_enter_sim_puk(RIL_Token t, void *data, size_t datalen) ipc_sec_sim_status_complete); ipc_fmt_send_set(IPC_SEC_SIM_STATUS, ril_request_get_id(t), (unsigned char *) &pin_status, sizeof(pin_status)); + + return; + +error: + ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } -/* - * In: IPC_SEC_PHONE_LOCK - * - * Out: RIL_REQUEST_QUERY_FACILITY_LOCK - * Query the status of a facility lock state - */ void ipc_sec_phone_lock(struct ipc_message_info *info) { int status; - struct ipc_sec_phone_lock_response *lock = (struct ipc_sec_phone_lock_response *) info->data; + struct ipc_sec_phone_lock_response *lock; + + if (info == NULL || info->data == NULL || info->length < sizeof(struct ipc_sec_phone_lock_response)) + goto error; + lock = (struct ipc_sec_phone_lock_response *) info->data; status = lock->status; ril_request_complete(ril_request_get_token(info->aseq), RIL_E_SUCCESS, &status, sizeof(status)); + + return; + +error: + if (info != NULL) + ril_request_complete(ril_request_get_token(info->aseq), RIL_E_GENERIC_FAILURE, NULL, 0); } -/* - * In: RIL_REQUEST_QUERY_FACILITY_LOCK - * Query the status of a facility lock state - * - * Out: IPC_SEC_PHONE_LOCK GET - */ -void ril_request_query_facility_lock(RIL_Token t, void *data, size_t datalen) +void ril_request_query_facility_lock(RIL_Token t, void *data, size_t length) { struct ipc_sec_phone_lock_get lock_request; + char *facility; - char *facility = ((char **) data)[0]; + if (data == NULL || length < sizeof(char *)) + goto error; + + facility = ((char **) data)[0]; if (!strcmp(facility, "SC")) { lock_request.facility = IPC_SEC_FACILITY_TYPE_SC; @@ -894,31 +903,37 @@ void ril_request_query_facility_lock(RIL_Token t, void *data, size_t datalen) } else if (!strcmp(facility, "PC")) { lock_request.facility = IPC_SEC_FACILITY_TYPE_PC; } else { - LOGE("%s: unsupported facility: %s", __FUNCTION__, facility); + LOGE("%s: unsupported facility: %s", __func__, facility); ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } ipc_fmt_send(IPC_SEC_PHONE_LOCK, IPC_TYPE_GET, (void *) &lock_request, sizeof(lock_request), ril_request_get_id(t)); + + return; + +error: + ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } // Both functions were the same #define ipc_sec_phone_lock_complete \ ipc_sec_sim_status_complete -/* - * In: RIL_REQUEST_SET_FACILITY_LOCK - * Enable/disable one facility lock - * - * Out: IPC_SEC_PHONE_LOCK SET - */ -void ril_request_set_facility_lock(RIL_Token t, void *data, size_t datalen) +void ril_request_set_facility_lock(RIL_Token t, void *data, size_t length) { struct ipc_sec_phone_lock_set lock_request; + char *facility; + char *lock; + char *password; + char *class; + + if (data == NULL || length < (int) (4 * sizeof(char *))) + goto error; - char *facility = ((char **) data)[0]; - char *lock = ((char **) data)[1]; - char *password = ((char **) data)[2]; - char *class = ((char **) data)[3]; + facility = ((char **) data)[0]; + lock = ((char **) data)[1]; + password = ((char **) data)[2]; + class = ((char **) data)[3]; memset(&lock_request, 0, sizeof(lock_request)); @@ -935,7 +950,7 @@ void ril_request_set_facility_lock(RIL_Token t, void *data, size_t datalen) } else if (!strcmp(facility, "PC")) { lock_request.type = IPC_SEC_SIM_STATUS_LOCK_PC; } else { - LOGE("%s: unsupported facility: %s", __FUNCTION__, facility); + LOGE("%s: unsupported facility: %s", __func__, facility); ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } @@ -950,4 +965,9 @@ void ril_request_set_facility_lock(RIL_Token t, void *data, size_t datalen) ipc_sec_phone_lock_complete); ipc_fmt_send(IPC_SEC_PHONE_LOCK, IPC_TYPE_SET, (void *) &lock_request, sizeof(lock_request), ril_request_get_id(t)); + + return; + +error: + ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0); } -- cgit v1.1