From 1dc51132943017adb9726ffa0de6b440b729b490 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Fri, 17 Feb 2017 15:25:43 +0900 Subject: [PATCH 01/16] pass: Add new PASS_GOV_BASIC type and adjust the unique number of governor This patch adds new PASS_GOV_BASIC type which uses the 'HAL interface' and 'Resource controller'. For example, the bus/gpu resources use this governor type in order to control the min/max frequency when pass recevies the pmqos debus message with scenario name. * Detailed description according to each governor type: --------------------------------------------------------------------------- | Governor name | HAL | Resource | Hotplug | Runtime | | | interface | controller | interface | governor | -------------------------------------------------------------------------- | GOV_DUMMY | Used | Un-used | Un-used | Un-used | | GOV_BASIC | Used | Used | Un-used | Un-used | | GOV_HOTPLUG_ONLY | Used | Used | Used | Un-used | | GOV_RADIATION | Used | Used | Used | Used (Radiation)| | GOV_STEP | Used | Used | Used | Used (Step) | --------------------------------------------------------------------------- * Detailed description according to PASS's feature: - HAL interface : Call the HAL implementation such as pass-hal-(board).rpm - Resource controller: Control the h/w resource such as CPU/BUS/GPU resource. - Hotplug interface : Turn on/off the CPU h/w resource. - Runtime governor : Monitor the system status and then decide the proper level. After deciding the level, it control the h/w resource by using both 'Resource controller' and 'Hotplug interface'. Change-Id: I3b7d7a8f5142baa409cbbfe221afad8decfb2149 Signed-off-by: Chanwoo Choi --- src/pass/pass-gov.c | 8 ++++++++ src/pass/pass.h | 18 +++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index 3f1528a..5e94a9b 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -191,10 +191,13 @@ struct pass_hotplug* pass_get_hotplug(struct pass_policy *policy, switch (type) { case PASS_GOV_DUMMY: + case PASS_GOV_BASIC: + /* Don't use Hotplug interface */ return NULL; case PASS_GOV_HOTPLUG_ONLY: case PASS_GOV_STEP: case PASS_GOV_RADIATION: + /* Use Hotplug interface */ hotplug = calloc(1, sizeof(struct pass_hotplug)); if (!hotplug) { _E("cannot allocate the memory of struct pass_hotplug"); @@ -687,11 +690,16 @@ struct pass_governor* pass_get_governor(struct pass_policy *policy, { switch (type) { case PASS_GOV_DUMMY: + return NULL; + case PASS_GOV_BASIC: case PASS_GOV_HOTPLUG_ONLY: + /* Use the Resource controller only */ return &pass_gov_dummy; case PASS_GOV_STEP: + /* Use the Resource controller and Step governor. */ return &pass_gov_step; case PASS_GOV_RADIATION: + /* Use the Resource controller and Radiation governor. */ return &pass_gov_radiation; default: _E("Unknown governor type"); diff --git a/src/pass/pass.h b/src/pass/pass.h index 000a39c..6271799 100644 --- a/src/pass/pass.h +++ b/src/pass/pass.h @@ -50,12 +50,24 @@ enum pass_state { /* * PASS Governor type + * + * --------------------------------------------------------------------------- + * | Governor name | HAL | Resource | Hotplug | Runtime | + * | | interface | controller | interface | governor | + * --------------------------------------------------------------------------- + * | GOV_DUMMY | Used | Un-used | Un-used | Un-used | + * | GOV_BASIC | Used | Used | Un-used | Un-used | + * | GOV_HOTPLUG_ONLY | Used | Used | Used | Un-used | + * | GOV_RADIATION | Used | Used | Used | Used (Radiation)| + * | GOV_STEP | Used | Used | Used | Used (Step) | + * --------------------------------------------------------------------------- */ enum pass_gov_type { PASS_GOV_DUMMY = -1, - PASS_GOV_HOTPLUG_ONLY = 0, - PASS_GOV_STEP = 1, - PASS_GOV_RADIATION = 2, + PASS_GOV_BASIC = 0, + PASS_GOV_HOTPLUG_ONLY = 1, + PASS_GOV_STEP = 2, + PASS_GOV_RADIATION = 3, PASS_GOV_END, }; -- 2.7.4 From 9b6368201c08883a94687e3492e1be2389d3bfc7 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 28 Feb 2017 14:19:07 +0900 Subject: [PATCH 02/16] pass: parser: Add gov_timeout to Level section in resource conf file This patch adds a gov_timeout entry to the Level section in the resource configuration file. Comparing with pass_gov_timeout in the Pass section, this entry indicates the timer interval for a specific level. If this entry does not appear in the Level section, the value of pass_gov_timeout will be assigned to the governor timer interval for that level by default. Change-Id: I57ed24f8666711c11c3ceeeeaab9bfcc13c550a1 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-parser.c | 8 ++++++++ src/pass/pass.c | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index e10ed52..6e58f22 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -157,6 +157,14 @@ static int pass_parse_level(struct parse_result *result, policy->pass_table[level].right_cond[0].nr_running = atoi(result->value); else if (MATCH(result->name, "num_right_cond_busy_cpu")) policy->pass_table[level].right_cond[0].busy_cpu = atoi(result->value); + else if (MATCH(result->name, "gov_timeout")) { + double gov_timeout = atof(result->value); + + if (gov_timeout < PASS_MIN_GOV_TIMEOUT) + gov_timeout = PASS_MIN_GOV_TIMEOUT; + + policy->pass_table[level].gov_timeout = gov_timeout; + } return 0; } diff --git a/src/pass/pass.c b/src/pass/pass.c index 5a7d6e7..0fe797d 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -158,6 +158,8 @@ static int pass_resource_init(struct pass_policy *policy) max_freq = policy->pass_table[i].limit_max_freq; if (max_cpu < policy->pass_table[i].limit_min_cpu) max_cpu = policy->pass_table[i].limit_min_cpu; + if (policy->pass_table[i].gov_timeout == 0) + policy->pass_table[i].gov_timeout = policy->gov_timeout; } policy->freq.max_freq = max_freq; -- 2.7.4 From 61a676d38ea6d126ef6abb7775e5a08e148682ed Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 28 Feb 2017 14:38:59 +0900 Subject: [PATCH 03/16] pass: Remove policy-specific data from struct pass_governor This patch removes policy-specific data from struct pass_governor. The policy-specific data should be included in struct pass_policy only. Change-Id: Id9bd77e9dbd8b0668170696b40a0d40c9c166e42 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-gov.c | 35 +++++++++++++++++------------------ src/pass/pass.c | 1 - src/pass/pass.h | 3 +-- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index 5e94a9b..27d72d6 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -417,18 +417,18 @@ static Eina_Bool pass_governor_core_timer(void *data) * Change the period of govenor timer according to PASS level */ if (policy->pass_table[level].gov_timeout >= PASS_MIN_GOV_TIMEOUT && - (policy->governor->gov_timeout + (policy->gov_timeout != policy->pass_table[level].gov_timeout)) { _I("Change the period of governor timer from %fs to %fs\n", - policy->governor->gov_timeout, + policy->gov_timeout, policy->pass_table[level].gov_timeout); - policy->governor->gov_timeout = + policy->gov_timeout = policy->pass_table[level].gov_timeout; - ecore_timer_interval_set(policy->governor->gov_timer_id, - policy->governor->gov_timeout); - ecore_timer_reset(policy->governor->gov_timer_id); + ecore_timer_interval_set(policy->gov_timer_id, + policy->gov_timeout); + ecore_timer_reset(policy->gov_timer_id); } return ECORE_CALLBACK_RENEW; @@ -455,18 +455,18 @@ static void __pass_governor_start(struct pass_policy *policy) } /* Create the core timer of PASS governor */ - if (policy->governor->gov_timeout > 0) { - policy->governor->gov_timer_id = ecore_timer_add( - policy->governor->gov_timeout, + if (policy->gov_timeout > 0) { + policy->gov_timer_id = ecore_timer_add( + policy->gov_timeout, (Ecore_Task_Cb)pass_governor_core_timer, (void *)policy); - if (!policy->governor->gov_timer_id) { + if (!policy->gov_timer_id) { _E("cannot add core timer for governor"); pass_governor_update(policy, PASS_GOV_STOP); return; } } else { - policy->governor->gov_timer_id = NULL; + policy->gov_timer_id = NULL; } /* @@ -509,9 +509,9 @@ static void __pass_governor_stop(struct pass_policy *policy) pass_hotplug_stop(policy); - if (policy->governor->gov_timer_id) { - ecore_timer_del(policy->governor->gov_timer_id); - policy->governor->gov_timer_id = NULL; + if (policy->gov_timer_id) { + ecore_timer_del(policy->gov_timer_id); + policy->gov_timer_id = NULL; } /* Set PASS state as PASS_GOV_STOP */ @@ -525,9 +525,8 @@ static int __pass_governor_init(struct pass_policy *policy) struct pass_resource *res = to_pass_resource(policy); struct pass_conf_data *cdata = &res->cdata; - if(policy->governor->gov_timeout < 0) { - _E("invalid timeout value [%d]!", - policy->governor->gov_timeout); + if(policy->gov_timeout < 0) { + _E("invalid timeout value [%d]!", policy->gov_timeout); pass_governor_update(policy, PASS_GOV_STOP); return -EINVAL; } @@ -583,7 +582,7 @@ static int __pass_governor_exit(struct pass_policy *policy) policy->pass_table = NULL; policy->num_pass_cpu_stats = 0; - policy->governor->gov_timeout = 0; + policy->gov_timeout = 0; policy->governor = NULL; diff --git a/src/pass/pass.c b/src/pass/pass.c index 0fe797d..c7399dc 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -182,7 +182,6 @@ static int pass_resource_init(struct pass_policy *policy) _E("cannot get the instance of PASS governor"); return -1; } - policy->governor->gov_timeout = policy->gov_timeout; /* Get the instance of PASS hotplug */ policy->hotplug = pass_get_hotplug(policy, policy->gov_type); diff --git a/src/pass/pass.h b/src/pass/pass.h index 6271799..8eaa82e 100644 --- a/src/pass/pass.h +++ b/src/pass/pass.h @@ -95,8 +95,6 @@ struct pass_governor { int (*exit)(struct pass_policy *); int (*update)(struct pass_policy *, enum pass_gov_state state); int (*governor)(struct pass_policy *); - Ecore_Timer *gov_timer_id; - double gov_timeout; }; /****************************************************** @@ -247,6 +245,7 @@ struct pass_policy { struct pass_governor *governor; double gov_timeout; + Ecore_Timer *gov_timer_id; struct pass_hotplug *hotplug; }; -- 2.7.4 From 10b2ca13dcd1b63de08b6dec17324f176fbf3fbe Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 28 Feb 2017 17:15:11 +0900 Subject: [PATCH 04/16] pass: gov: Modify timer interval setting mechanism This patch modifies the timer setting mechanism for the governor to select either the level-specific timer interval or the default one. The default timer interval is used when the gov_timeout does not appear in the Level section of the resource configuration file. Change-Id: Id1b9d31a3249846ee59b34d00cb41f126440f5bc Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-gov.c | 26 +++++++++++++------------- src/pass/pass-parser.c | 1 + 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index 27d72d6..d7d359c 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -251,7 +251,7 @@ static int pass_governor_change_level(struct pass_policy *policy, int new_level) limit_max_freq = table[new_level].limit_max_freq; limit_min_freq = table[new_level].limit_min_freq; - policy->prev_level = policy->curr_level; + policy->prev_level = curr_level; policy->curr_level = new_level; /* Turn on/off CPUs according the maximum number of online CPU */ @@ -372,6 +372,7 @@ static Eina_Bool pass_governor_core_timer(void *data) { struct pass_policy *policy = (struct pass_policy *)data; static int count = 0; + double curr_gov_timeout, next_gov_timeout; int level; int ret; @@ -402,6 +403,9 @@ static Eina_Bool pass_governor_core_timer(void *data) /* Calculate the number of busy cpu */ pass_calculate_busy_cpu(policy); + /* Store current governor timeout */ + curr_gov_timeout = policy->pass_table[policy->curr_level].gov_timeout; + /* Determine the amount of proper resource */ if (policy->governor->governor) { level = policy->governor->governor(policy); @@ -413,21 +417,16 @@ static Eina_Bool pass_governor_core_timer(void *data) return ECORE_CALLBACK_CANCEL; } - /* - * Change the period of govenor timer according to PASS level - */ - if (policy->pass_table[level].gov_timeout >= PASS_MIN_GOV_TIMEOUT && - (policy->gov_timeout - != policy->pass_table[level].gov_timeout)) { + /* Store next governor timeout */ + next_gov_timeout = policy->pass_table[policy->curr_level].gov_timeout; + /* Change the period of govenor timer according to PASS level */ + if (curr_gov_timeout != next_gov_timeout) { _I("Change the period of governor timer from %fs to %fs\n", - policy->gov_timeout, - policy->pass_table[level].gov_timeout); - - policy->gov_timeout = - policy->pass_table[level].gov_timeout; + curr_gov_timeout, + next_gov_timeout); ecore_timer_interval_set(policy->gov_timer_id, - policy->gov_timeout); + next_gov_timeout); ecore_timer_reset(policy->gov_timer_id); } @@ -476,6 +475,7 @@ static void __pass_governor_start(struct pass_policy *policy) policy->curr_level = -1; if (policy->init_level > policy->max_level) policy->init_level = policy->max_level; + pass_governor_change_level(policy, policy->init_level); /* Set PASS state as PASS_GOV_START */ diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index 6e58f22..e953c88 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -308,6 +308,7 @@ int pass_get_table(struct pass_policy *policy, char *pass_conf_path) policy->up_threshold = 0; policy->down_threshold = 0; policy->init_level = 0; + policy->prev_level = policy->init_level; policy->level_up_threshold = 0; policy->gov_timeout = 0; policy->last_time = 0; -- 2.7.4 From 5ef9c9555b3dafccfc493d335230393952ecaf7b Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 28 Feb 2017 15:54:07 +0900 Subject: [PATCH 05/16] pass: pmqos: Prevent from accessing variables before initialization This patch modifies the callback function of DEVICE_NOTIFIER_PMQOS notifier to prevent from accessing variables before their initialization. Change-Id: Idc8a760ec8a6a53a1cfbc34f17626ea6c186ed57 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-pmqos.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/pass/pass-pmqos.c b/src/pass/pass-pmqos.c index 20be7e4..3929dff 100644 --- a/src/pass/pass-pmqos.c +++ b/src/pass/pass-pmqos.c @@ -95,9 +95,9 @@ static int find_scenario_index(struct pass_scenario_policy *scenario, #define MAX(a,b) (a > b ? a : b) int pass_notifier_pmqos_func(struct pass_policy *policy, void *data) { - struct pass_resource *pass_res = to_pass_resource(policy); - struct pass_conf_data *cdata = &pass_res->cdata; - struct pass_scenario_policy *scenario = &policy->scenario; + struct pass_resource *pass_res; + struct pass_conf_data *cdata; + struct pass_scenario_policy *scenario; struct pass_scenario *scn = NULL; enum pass_state locked = PASS_UNUSED; char name[PASS_NAME_LEN] = {""}; @@ -109,6 +109,10 @@ int pass_notifier_pmqos_func(struct pass_policy *policy, void *data) if (!is_pmqos_enabled(policy)) return 0; + pass_res = to_pass_resource(policy); + cdata = &pass_res->cdata; + scenario = &policy->scenario; + /* * Parse scenario name(data) whether to include 'Lock' or 'Unlock' * string and divide correct scenario name. -- 2.7.4 From aae2ba1d4cf3205628c9db641d26a68c4c6be3bc Mon Sep 17 00:00:00 2001 From: Wook Song Date: Fri, 24 Feb 2017 16:36:41 +0900 Subject: [PATCH 06/16] pass: Add init_done and exit_done callbacks This patch adds init_done and exit_done callbacks to the pass module. These callbacks are in charge of actual initialization and finalization of the daemon, instead of the init and exit functions. Callback functions for dbus messages such as start and stop are also modified to use the init_done and exit_done functions now. Especially, the init_done is registered as a booting-done callback at the probe time. Change-Id: I2f1a929b8b0b6b64b877c3f627d30af42bb26cfb Signed-off-by: Wook Song --- src/pass/pass-gov.c | 29 +-------- src/pass/pass.c | 165 +++++++++++++++++++++++++++++++++------------------- 2 files changed, 108 insertions(+), 86 deletions(-) diff --git a/src/pass/pass-gov.c b/src/pass/pass-gov.c index d7d359c..0decd8b 100644 --- a/src/pass/pass-gov.c +++ b/src/pass/pass-gov.c @@ -50,22 +50,6 @@ static bool is_enabled(struct pass_policy *policy) } /* - * pass_notifier_booting_done - Callback func of DEVICE_NOTIFIER_BOOTING_DONE - * @data: NULL, this parameter isn't used - * @user_data: the instance of struct pass_policy - */ -static int pass_notifier_booting_done(void *data, void *user_data) -{ - struct pass_policy *policy = user_data; - - /* Start PASS governor if 'pass_support' in pass.conf is true */ - if (policy->state == PASS_ON) - pass_governor_update(policy, PASS_GOV_START); - - return 0; -} - -/* * pass_notifier_pmqos - Callback func of DEVICE_NOTIFIER_PMQOS * @data: the scenario name * @user_data: the instance of struct pass_policy @@ -89,10 +73,6 @@ static int pass_notifier_init(struct pass_policy *policy) /* Register DEVICE_NOTIFIER_PMQOS */ register_notifier(DEVICE_NOTIFIER_PMQOS, pass_notifier_pmqos, policy); - /* Register DEVICE_NOTIFIER_BOOTING_DONE */ - register_notifier(DEVICE_NOTIFIER_BOOTING_DONE, - pass_notifier_booting_done, policy); - return 0; } @@ -105,10 +85,6 @@ static int pass_notifier_exit(struct pass_policy *policy) /* Un-register DEVICE_NOTIFIER_PMQOS */ unregister_notifier(DEVICE_NOTIFIER_PMQOS, pass_notifier_pmqos); - /* Un-Register DEVICE_NOTIFIER_BOOTING_DONE */ - unregister_notifier(DEVICE_NOTIFIER_BOOTING_DONE, - pass_notifier_booting_done); - return 0; } @@ -533,12 +509,13 @@ static int __pass_governor_init(struct pass_policy *policy) /* Set default PASS state */ policy->gov_state = PASS_GOV_STOP; - - /* Initialize notifier */ pass_notifier_init(policy); _I("Initialize governor for '%s' resource", cdata->res_name); + if (policy->state == PASS_ON) + pass_governor_update(policy, PASS_GOV_START); + return 0; } diff --git a/src/pass/pass.c b/src/pass/pass.c index c7399dc..ce85a02 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -27,10 +27,12 @@ #include "pass-hal.h" #include "pass-gov.h" +#include "core/device-notifier.h" #include "core/devices.h" #include "core/common.h" #include "core/edbus-handler.h" +#define PASS_CONF_PATH "/etc/pass/pass.conf" #define PASS_DEFAULT_MIN_LEVEL 0 #define PASS_DEFAULT_CPU_THRESHOLD 20 #define PASS_DEFAULT_LEVEL_UP_THRESHOLD 30 @@ -39,50 +41,47 @@ static struct pass g_pass; /****************************************************** + * PASS interfaces: Probe/Init/Exit * + * (Declarations) * + ******************************************************/ +static int pass_probe(void *data); +static void pass_init(void *data); +static int pass_init_done(void *data, void *user_data); +static void pass_exit(void *data); +static int pass_exit_done(void); + +/****************************************************** * PASS D-Bus interface * ******************************************************/ static DBusMessage* e_dbus_start_cb(E_DBus_Object *obj, DBusMessage* msg) { DBusMessage *ret_dbus = NULL; - int i, ret; - - for (i = 0; i < g_pass.num_resources; i++) { - struct pass_resource *pass_res = &g_pass.res[i]; - struct pass_policy *policy = &pass_res->policy; + int ret; - ret = pass_governor_update(policy, PASS_GOV_START); - if (ret < 0) { - _E("cannot start the governor with dbus"); - return ret_dbus; - } - ret_dbus = dbus_message_new_method_return(msg); - if (!ret_dbus) - return ret_dbus; + ret = pass_init_done(NULL, NULL); + if (ret < 0) { + _E("failed to initialize the daemon in dbus callback \ + for a start message\n"); + return ret_dbus; } + ret_dbus = dbus_message_new_method_return(msg); return ret_dbus; } static DBusMessage* e_dbus_stop_cb(E_DBus_Object *obj, DBusMessage* msg) { DBusMessage *ret_dbus = NULL; - int i, ret; - - for (i = 0; i < g_pass.num_resources; i++) { - struct pass_resource *pass_res = &g_pass.res[i]; - struct pass_policy *policy = &pass_res->policy; - - ret = pass_governor_update(policy, PASS_GOV_STOP); - if (ret < 0) { - _E("cannot stop the governor"); - return ret_dbus; - } + int ret; - ret_dbus = dbus_message_new_method_return(msg); - if (!ret_dbus) - return ret_dbus; + ret = pass_exit_done(); + if (ret < 0) { + _E("failed to exit the daemon in dbus callback \ + for a stop message\n"); + return ret_dbus; } + ret_dbus = dbus_message_new_method_return(msg); return ret_dbus; } @@ -220,16 +219,24 @@ static int pass_resource_exit(struct pass_policy *policy) return 0; } -/* - * pass_init - Initialize PASS(Power Aware System Service) - * - * @data: the instance of structre pass_policy - */ - -static void pass_init(void *data) +static int pass_init_done(void *data, void *user_data) { int i, ret; + /* Parse configuration file (/etc/pass/pass.conf) */ + ret = pass_parse_resource(&g_pass, PASS_CONF_PATH); + if (ret < 0) { + _E("cannot parse %s\n", PASS_CONF_PATH); + return ret; + } + + /* Initialize pass resources data based on parsed configuration */ + ret = pass_get_resource(&g_pass); + if (ret < 0) { + _E("cannot get the pass resource\n"); + return ret; + } + for (i = 0; i < g_pass.num_resources; i++) { struct pass_resource *pass_res = &g_pass.res[i]; struct pass_policy *policy = &pass_res->policy; @@ -245,17 +252,6 @@ static void pass_init(void *data) _I("Resource%d first cpu : %d\n\n", i, cdata->cpu); } - /* - * Initialzie D-Bus interface of PASS. User can be able to - * turn on/off PASS through D-Bus interface. - */ - ret = register_edbus_method(PASS_PATH_CORE, edbus_methods, - ARRAY_SIZE(edbus_methods)); - if (ret < 0) { - _I("cannot initialize PASS D-Bus (%d)", ret); - return; - } - for (i = 0; i < g_pass.num_resources; i++) { struct pass_resource *pass_res = &g_pass.res[i]; struct pass_policy *policy = &pass_res->policy; @@ -263,51 +259,100 @@ static void pass_init(void *data) ret = pass_resource_init(policy); if (ret < 0) { _E("cannot initialize the pass resource\n"); - return; + pass_exit(NULL); + return -EINVAL; } } + + return 0; } -/* - * pass_exit - Exit PASS - * - * @data: the instance of structre pass_policy - */ -static void pass_exit(void *data) +static int pass_exit_done(void) { int i, ret; + ret = 0; for (i = 0; i < g_pass.num_resources; i++) { struct pass_resource *pass_res = &g_pass.res[i]; struct pass_policy *policy = &pass_res->policy; ret = pass_resource_exit(policy); if (ret < 0) { - _E("cannot exit the pass resource"); + _E("failed to clean up the pass resource #%d\n", i); + break; } } + + return ret; } -int pass_probe(void *data) +/* + * pass_init - Initialize PASS(Power Aware System Service) + * + * @data: When the data is passed from pass_main(), it is NULL. + * On the other hand, it will contain the result of this function + */ +static void pass_init(void *data) +{ + /* This is the only case of daemon creation: DO NOTHING */ +} + +/* + * pass_exit - Exit PASS + * + * @data: When the data is passed from pass_main(), it is NULL. + * On the other hand, it will contain the result of this function + */ +static void pass_exit(void *data) { int ret; + unregister_notifier(DEVICE_NOTIFIER_BOOTING_DONE, pass_init_done); + + ret = pass_exit_done(); + if (ret < 0) { + _E("cannot exit PASS daemon\n"); + return; + } + + _I("exit PASS daemon without any errors\n"); +} + +/* + * pass_probe - Probe PASS + * + * @data: the data passed from pass_main(), currently NULL + */ +static int pass_probe(void *data) +{ + int ret = 0; + /* - * Initialize pass resources data by parsing pass.conf + * Register methods to D-Bus interface of PASS. By using it, + * user can turn on/off PASS */ - ret = pass_parse_resource(&g_pass, "/etc/pass/pass.conf"); + ret = register_edbus_method(PASS_PATH_CORE, edbus_methods, + ARRAY_SIZE(edbus_methods)); if (ret < 0) { - _E("cannot parse /etc/pass/pass.conf\n"); + _E("cannot register dbus methods required \ + to control the daemon (%d)\n", ret); return ret; } - ret = pass_get_resource(&g_pass); + /* + * Register a notifier for the booting-done event. The actual + * initialization of the daemon is performed by this notifier after + * booting is completely done. + */ + ret = register_notifier(DEVICE_NOTIFIER_BOOTING_DONE, + pass_init_done, NULL); if (ret < 0) { - _E("cannot get the pass resource\n"); + _E("cannot register a callback function \ + for the booting-done event (%d)\n", ret); return ret; } - return 0; + return ret; } static const struct device_ops pass_device_ops = { -- 2.7.4 From f9d097706e7f922bfd99fa81c6f298700bd7887c Mon Sep 17 00:00:00 2001 From: Wook Song Date: Fri, 3 Mar 2017 10:15:52 +0900 Subject: [PATCH 07/16] pmqos: Move registration of booting-done callback to probe This patch moves the registration of a callback for the booting-done event from the pmqos_init() function to the pmqos_probe() function, which is newly added. This booting-done callback is renamed to pmqos_init_done and slightly modified. Change-Id: If8b1de698ab03257b5fd67081cad1101c06e47d7 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pmqos/pmqos.c | 58 +++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/src/pmqos/pmqos.c b/src/pmqos/pmqos.c index baaf748..1ade574 100644 --- a/src/pmqos/pmqos.c +++ b/src/pmqos/pmqos.c @@ -418,17 +418,21 @@ static const struct edbus_method edbus_methods[] = { { "UltraPowerSaving", "i", "i", dbus_pmqos_handler }, }; -static int booting_done(void *data, void *user_data) +static int pmqos_init_done(void *data, void *user_data) { - static int done; struct edbus_method *methods = NULL; + int booting_done; int ret, size; - if (data == NULL) - goto out; - done = (int)data; - if (!done) - goto out; + /* + * As a callback function for the booting-done event, it is notified + * with the result of the booting_finished() function by using the + * first argument, void *data. When it is successfully booted up, 1 is + * passed with the first argument. + */ + booting_done = (int)data; + if (!booting_done) + return -EINVAL; /* register edbus methods */ ret = register_edbus_method(PASS_PATH_PMQOS, edbus_methods, @@ -449,13 +453,13 @@ static int booting_done(void *data, void *user_data) free(methods); } -out: - return done; + return ret; } static void pmqos_init(void *data) { - register_notifier(DEVICE_NOTIFIER_BOOTING_DONE, booting_done, NULL); + /* This is the case of daemon creation: DO NOTHING */ + return; } static void pmqos_free(void) @@ -480,14 +484,40 @@ static void pmqos_free(void) static void pmqos_exit(void *data) { - unregister_notifier(DEVICE_NOTIFIER_BOOTING_DONE, booting_done); + unregister_notifier(DEVICE_NOTIFIER_BOOTING_DONE, pmqos_init_done); pmqos_free(); } +/* + * pmqos_probe - Probe PMQOS + * + * @data: the data passed from pass_main(), currently NULL + */ +static int pmqos_probe(void *data) +{ + int ret = 0; + + /* + * Register a notifier for the booting-done event. The actual + * initialization of the daemon is performed by this notifier after + * booting is completely done. + */ + ret = register_notifier(DEVICE_NOTIFIER_BOOTING_DONE, + pmqos_init_done, NULL); + if (ret < 0) { + _E("cannot register a callback function \ + for the booting-done event (%d)\n", ret); + return ret; + } + + return ret; +} + static const struct device_ops pmqos_device_ops = { - .name = "pmqos", - .init = pmqos_init, - .exit = pmqos_exit, + .name = "pmqos", + .probe = pmqos_probe, + .init = pmqos_init, + .exit = pmqos_exit, }; DEVICE_OPS_REGISTER(&pmqos_device_ops) -- 2.7.4 From 291e69d44546211de82d089fa239002e220c16e5 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Fri, 3 Mar 2017 12:48:25 +0900 Subject: [PATCH 08/16] pmqos: Add callbacks for dbus messages This patch adds the callback functions for dbus messages that start and stop the pmqos module. Change-Id: I6a9203a766469c49ff9b9b96eb594082057288d7 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pmqos/pmqos.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/pmqos/pmqos.c b/src/pmqos/pmqos.c index 1ade574..6b7c2f1 100644 --- a/src/pmqos/pmqos.c +++ b/src/pmqos/pmqos.c @@ -51,6 +51,43 @@ static struct timespec unlock_timer_start_st; static struct timespec unlock_timer_end_st; static struct pmqos_cpu unlock_timer_owner = {"", 0}; +/****************************************************** + * PMQOS callback functions for Init/Exit * + * (Declarations) * + ******************************************************/ +static int pmqos_init_done(void *data, void *user_data); +static void pmqos_free(void); + +/****************************************************** + * PASS D-Bus interface * + ******************************************************/ +static DBusMessage* e_dbus_start_cb(E_DBus_Object *obj, DBusMessage* msg) +{ + DBusMessage *ret_dbus = NULL; + bool booting_done = true; + int ret; + + ret = pmqos_init_done(&booting_done, NULL); + if (ret < 0) { + _E("failed to initialize the daemon in dbus callback \ + for a start message\n"); + return ret_dbus; + } + + ret_dbus = dbus_message_new_method_return(msg); + return ret_dbus; +} + +static DBusMessage* e_dbus_stop_cb(E_DBus_Object *obj, DBusMessage* msg) +{ + DBusMessage *ret_dbus; + + pmqos_free(); + + ret_dbus = dbus_message_new_method_return(msg); + return ret_dbus; +} + int set_pmqos(const char *name, int val) { char scenario[100]; @@ -364,6 +401,12 @@ static int get_methods_from_conf(const char *path, struct edbus_method **edbus_m /* Add pmqos name as alphabetically */ static const struct edbus_method edbus_methods[] = { + { "start", NULL, NULL, e_dbus_start_cb }, + { "stop", NULL, NULL, e_dbus_stop_cb }, + /* + * TODO: The hard-coded scenario info is no longer required. Therefore, + * the following code will be removed. + */ { "AppLaunch", "i", "i", dbus_pmqos_handler }, { "AppLaunchHome", "i", "i", dbus_pmqos_handler }, { "BeautyShot", "i", "i", dbus_pmqos_handler }, -- 2.7.4 From 4e0609af36e78dc043e005d144a1c008bb64a7ec Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Fri, 10 Mar 2017 16:33:17 +0900 Subject: [PATCH 09/16] pass: Remove dead-code related to the number of cpu This patch just removes the dead code which is related to the number of CPU in the same cluster. The pass uses the 'num_cpus' variable of the 'struct pass_hotplug'. Change-Id: I6775124230d45f572e99bdb070b786267d6c8c68 Signed-off-by: Chanwoo Choi --- src/pass/pass.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pass/pass.c b/src/pass/pass.c index ce85a02..40ae215 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -98,7 +98,6 @@ static int pass_resource_init(struct pass_policy *policy) { struct pass_resource *pass_res = to_pass_resource(policy); int max_freq = 0; - int max_cpu = 0; int ret; int i; @@ -155,8 +154,6 @@ static int pass_resource_init(struct pass_policy *policy) for (i = 0; i < policy->num_levels; i++) { if (max_freq < policy->pass_table[i].limit_max_freq) max_freq = policy->pass_table[i].limit_max_freq; - if (max_cpu < policy->pass_table[i].limit_min_cpu) - max_cpu = policy->pass_table[i].limit_min_cpu; if (policy->pass_table[i].gov_timeout == 0) policy->pass_table[i].gov_timeout = policy->gov_timeout; } -- 2.7.4 From 58c25d188c18321dbea2c8831d18f2ca349c34cd Mon Sep 17 00:00:00 2001 From: Wook Song Date: Wed, 15 Mar 2017 11:00:17 +0900 Subject: [PATCH 10/16] pmqos: parser: Fix defects such as tainted integer and use of strcpy This patch fixes the following code-level defects according to static program analysis result: 1. PROC_USE.VULNERABLE: Use of vulnerable function 'strcpy'. For better security, using strncpy is recommended. 2. TAINTED_INT: Integer value obtained from untrusted source without checking its higher bound is used in a trusted operation by calling function 'calloc'. Change-Id: I423c8dcc6cb720673f2c754a39204e140fdb9e79 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pmqos/pmqos-parser.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/pmqos/pmqos-parser.c b/src/pmqos/pmqos-parser.c index 5caae75..9efea89 100644 --- a/src/pmqos/pmqos-parser.c +++ b/src/pmqos/pmqos-parser.c @@ -29,6 +29,8 @@ #include "pmqos.h" +#define MAX_NUM_OF_SCENARIOS 255 + static bool is_supported(const char *value) { assert(value); @@ -50,7 +52,13 @@ static int pmqos_parse_scenario(struct parse_result *result, void *user_data, un if (MATCH(result->name, "scenario_support")) scenarios->support = is_supported(result->value); else if (MATCH(result->name, "scenario_num")) { - scenarios->num = atoi(result->value); + int num = atoi(result->value); + + if (num > MAX_NUM_OF_SCENARIOS) + return -EINVAL; + + scenarios->num = num; + if (scenarios->num > 0) { scenarios->list = calloc(scenarios->num, sizeof(struct scenario)); @@ -76,7 +84,8 @@ static int pmqos_parse_scenario(struct parse_result *result, void *user_data, un /* Parse 'Scenario' section */ if (MATCH(result->name, "name")) - strcpy(scenarios->list[index].name, result->value); + snprintf(scenarios->list[index].name, strlen(result->value) + 1, + "%s", result->value); else if (MATCH(result->name, "support")) scenarios->list[index].support = is_supported(result->value); -- 2.7.4 From 6a8db45d880fc5bfbb0a57b7ccfcc07b24ae5161 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Thu, 16 Mar 2017 15:56:10 +0900 Subject: [PATCH 11/16] pass: parser: Fix various defects including nonterminated string cases This patch fixes the following code-level defects according to static program analysis result: 1. PROC_USE.VULNERABLE: Use of vulnerable function 'strcpy'. For better security, using strncpy is recommended instead of strcpy. 2. UNREACHABLE_CODE: The statement in the source code might be unreachable during program execution. 3. DEREF_AFTER_NULL.EX: After having been compared to NULL value, some pointers are dereferenced. 4. NONTERMINATED_STRING: Copying from string to a buffer without null termination by calling function 'strncpy'. 5. TAINTED_INT.LOOP.MIGHT: Integer value obtained from untrusted source without checking its higher bound is used as a loop bound. 6. TAINTED_INT/TAINTED_INT.MIGHT: Integer value obtained from untrusted source without checking its higher bound is used in a trusted operation by calling function 'calloc'. Change-Id: Ic24ac658af88c109a22664c7c03eb2c43e8e23c4 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-parser.c | 53 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index e953c88..206f428 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -28,6 +28,7 @@ #include "hal/hal.h" #define BUFF_MAX 255 +#define MAX_NUM 255 static enum pass_state is_supported(char *value) { @@ -52,7 +53,7 @@ static int pass_parse_scenario(struct parse_result *result, void *user_data, struct pass_policy *policy = user_data; struct pass_scenario_policy *scenario = &policy->scenario; - if (!policy && !scenario && !result) + if (!policy || !result) return 0; if (!result->section || !result->name || !result->value) @@ -66,7 +67,15 @@ static int pass_parse_scenario(struct parse_result *result, void *user_data, return -EINVAL; } else if (MATCH(result->name, "pass_num_scenarios")) { - scenario->num_scenarios = atoi(result->value); + unsigned int num_scenarios; + + num_scenarios = atoi(result->value); + if (num_scenarios > MAX_NUM) { + _E("cannot parse %s\n", result->name); + return -EINVAL; + } + + scenario->num_scenarios = num_scenarios; if (scenario->num_scenarios > 0 && !scenario->list) { scenario->list = calloc(scenario->num_scenarios, @@ -90,7 +99,8 @@ static int pass_parse_scenario(struct parse_result *result, void *user_data, /* Parse 'Scenario' section */ if (MATCH(result->name, "name")) { - strcpy(scenario->list[index].name, result->value); + snprintf(scenario->list[index].name, PASS_NAME_LEN, "%s", + result->value); } else if (MATCH(result->name, "support")) { scenario->list[index].state = is_supported(result->value); if (scenario->list[index].state < 0) @@ -183,9 +193,15 @@ static int pass_parse_core(struct parse_result *result, void *user_data) policy->state = atoi(result->value); else if (MATCH(result->name, "pass_gov_type")) policy->gov_type = atoi(result->value); - else if (MATCH(result->name, "pass_num_levels")) - policy->num_levels = atoi(result->value); - else if (MATCH(result->name, "pass_min_level")) + else if (MATCH(result->name, "pass_num_levels")) { + unsigned int num_levels = atoi(result->value); + + if (num_levels > MAX_NUM) { + _E("cannot parse %s\n", result->name); + return -EINVAL; + } + policy->num_levels = num_levels; + } else if (MATCH(result->name, "pass_min_level")) policy->min_level = atoi(result->value); else if (MATCH(result->name, "pass_max_level")) policy->max_level = atoi(result->value); @@ -248,7 +264,7 @@ static int pass_load_config(struct parse_result *result, void *user_data) /* Parsing 'Level' section to get pass-table */ for (level = 0; level < policy->num_levels; level++) { - ret = sprintf(section_name, "Level%d", level); + ret = snprintf(section_name, BUFF_MAX, "Level%d", level); if (MATCH(result->section, section_name)) { ret = pass_parse_level(result, user_data, level); @@ -274,7 +290,7 @@ static int pass_load_config(struct parse_result *result, void *user_data) /* Parsing 'Scenario' section */ for (index = 0; index < scenario->num_scenarios; index++) { - ret = sprintf(section_name, "Scenario%d", index); + ret = snprintf(section_name, BUFF_MAX, "Scenario%d", index); if (MATCH(result->section, section_name)) { ret = pass_parse_scenario(result, user_data, index); @@ -463,13 +479,13 @@ static int pass_parse_resource_data(struct parse_result *result, conf_data->res_type = atoi(result->value); } else if (MATCH(result->name, "pass_res_name")) { int len = strlen(result->value); - strncpy(conf_data->res_name, result->value, len); + snprintf(conf_data->res_name, len + 1, "%s", result->value); } else if (MATCH(result->name, "pass_path_conf_file")) { int len = strlen(result->value); - strncpy(conf_data->path_conf_file, result->value, len); + snprintf(conf_data->path_conf_file, len + 1, "%s", result->value); } else if (MATCH(result->name, "pass_path_load_table")) { int len = strlen(result->value); - strncpy(conf_data->path_load_table, result->value, len); + snprintf(conf_data->path_load_table, len + 1, "%s", result->value); } else if (MATCH(result->name, "pass_first_cpu")) { int pass_res_type = conf_data->res_type; @@ -547,7 +563,7 @@ static int pass_resource_config(struct parse_result *result, void *user_data) if (MATCH(result->section, "PassResource")) { if (MATCH(result->name, "pass_compatible")) { len = strlen(result->value); - strncpy(compatible, result->value, len); + snprintf(compatible, len + 1, "%s", result->value); len = strlen(path_compatible); if (len > 0) { @@ -559,7 +575,7 @@ static int pass_resource_config(struct parse_result *result, void *user_data) } } else if (MATCH(result->name, "pass_path_compatible")) { len = strlen(result->value); - strncpy(path_compatible, result->value, len); + snprintf(path_compatible, len + 1, "%s", result->value); len =strlen(compatible); if (len > 0) { @@ -570,7 +586,14 @@ static int pass_resource_config(struct parse_result *result, void *user_data) is_compatible = true; } } else if (MATCH(result->name, "pass_num_resources")) { - pass->num_resources = atoi(result->value); + unsigned int num_resources = atoi(result->value); + + if ((num_resources > MAX_NUM) || + (num_resources < 1)) { + _E("cannot parse %s\n", result->name); + return -EINVAL; + } + pass->num_resources = num_resources; pass->res = calloc(pass->num_resources, sizeof(struct pass_resource)); @@ -586,7 +609,7 @@ static int pass_resource_config(struct parse_result *result, void *user_data) /* Parsing 'PassResource[X]' section */ for (id = 0; id < pass->num_resources; id++) { - ret = sprintf(section_name, "PassResource%d", id); + ret = snprintf(section_name, BUFF_MAX, "PassResource%d", id); if (MATCH(section_name, result->section)) { if (!is_compatible) { -- 2.7.4 From ed4b4920c2cb1faf3895f862c8107807a95ead32 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Thu, 16 Mar 2017 18:25:06 +0900 Subject: [PATCH 12/16] pmqos: Fix strcpy use and memory leak defects This patch fixes the following code-level defects according to static program analysis result: 1. PROC_USE.VULNERABLE: Use of vulnerable function 'strcpy'. For better security, using strncpy is recommended instead of strcpy. 2. MEMORY_LEAK.EX: Dynamic memory was allocated by calling function 'calloc' and lost at some point. Change-Id: Ie58bb7073254935b24257f8c9db19dd04c8b25f8 Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pmqos/pmqos.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/pmqos/pmqos.c b/src/pmqos/pmqos.c index 6b7c2f1..1caa7bb 100644 --- a/src/pmqos/pmqos.c +++ b/src/pmqos/pmqos.c @@ -256,10 +256,18 @@ static int pmqos_request(const char *name, int val) ret = pmqos_unlock_timer_start(); if (ret < 0) - return ret; + goto err; + /* Set cpu lock */ set_pmqos(cpu->name, true); return 0; +err: + if (!found) { + DD_LIST_REMOVE(pmqos_head, cpu); + free(cpu); + } + + return ret; } static DBusMessage *dbus_pmqos_handler(E_DBus_Object *obj, DBusMessage *msg) @@ -371,7 +379,7 @@ static int get_methods_from_conf(const char *path, struct edbus_method **edbus_m /* allocate edbus methods structure */ methods = calloc(scenarios.num, sizeof(struct edbus_method)); if (!methods) { - _E("failed to allocate methods memory : %s", strerror(errno)); + _E("failed to allocate memory for methods"); pmqos_put_scenario(&scenarios); return -errno; } -- 2.7.4 From dc8936008dbd50311f1df0ff722ebd2f4f9f2cdc Mon Sep 17 00:00:00 2001 From: Wook Song Date: Fri, 17 Mar 2017 10:53:50 +0900 Subject: [PATCH 13/16] pass: hal: Fix defects including the case that the handle could be lost This patch fixes the following code-level defects according to static program analysis result: 1. NULL_AFTER_DEREF: A pointer which was dereferenced is compared to NULL value. 2. HANDLE_LEAK: A handle was created by calling function 'fopen' and lost at some point. 3. NO_EFFECT: the entire array is compared to 0. 4. NO_CAST.INTEGER_OVERFLOW: Values of an arithmetic expression could be subject to overflow due to a failure to cast operands to a larger data type before perfoming arithmetic overflow before widen. Change-Id: Icd25de37efc3b52aacece3476e697486a1934c3a Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/pass/pass-hal.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/pass/pass-hal.c b/src/pass/pass-hal.c index 851f93b..129d025 100644 --- a/src/pass/pass-hal.c +++ b/src/pass/pass-hal.c @@ -544,14 +544,19 @@ int pass_get_resource(struct pass *pass) /* Get the load_table of each resource to estimate the system load. */ int pass_get_cpu_stats(struct pass_policy *policy) { - struct pass_cpu_stats *stats = policy->pass_cpu_stats; - struct pass_resource *pass_res = to_pass_resource(policy); + struct pass_cpu_stats *stats; + struct pass_resource *pass_res; char str[BUFF_MAX]; FILE *fp_stats = NULL; int i, j, ret; + if (!policy) + return -EINVAL; + + stats = policy->pass_cpu_stats; + pass_res = to_pass_resource(policy); - if (!policy || !pass_res->cdata.path_load_table || !stats) { + if (!stats) { _E("invalid parameter of structure pass_cpu_stats"); return -EINVAL; } @@ -561,8 +566,10 @@ int pass_get_cpu_stats(struct pass_policy *policy) return -EIO; /* Read the title and drop the buffer because it is not used */ - if (!fgets(str, BUFF_MAX, fp_stats)) + if (!fgets(str, BUFF_MAX, fp_stats)) { + fclose(fp_stats); return -EIO; + } for (i = 0; i < policy->num_pass_cpu_stats; i++) { ret = fscanf(fp_stats, "%lld %d %d %d", @@ -570,15 +577,20 @@ int pass_get_cpu_stats(struct pass_policy *policy) &stats[i].freq, &stats[i].freq_new, &stats[i].nr_runnings); - if (ret < 0) + if (ret < 0) { + fclose(fp_stats); return -EIO; + } for (j = 0; j < pass_res->cdata.num_cpus; j++) { ret = fscanf(fp_stats, "%d", &stats[i].load[j]); - if (ret < 0) + if (ret < 0) { + fclose(fp_stats); return -EIO; + } } } + fclose(fp_stats); return 0; @@ -591,5 +603,5 @@ int64_t pass_get_time_ms(void) gettimeofday(&now, NULL); - return (int64_t)(now.tv_sec * 1000 + now.tv_usec / 1000); + return ((int64_t) now.tv_sec * 1000 + (int64_t) now.tv_usec / 1000); } -- 2.7.4 From 6c677a40ee31ffb75f728a9db90f226c9d05cdf7 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Fri, 17 Mar 2017 14:31:52 +0900 Subject: [PATCH 14/16] core: common: Fix defects including integer overflow case This patch fixes the following code-level defects according to static program analysis result: 1. NONTERMINATED_STRING: Copying from string to a buffer without null termination by calling function 'strncpy'. 2. INTEGER_OVERFLOW: Possible integer overflow. Change-Id: Iee3ed333393fa44cd0c2f4c38431fc833898b30c Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/core/common.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/core/common.c b/src/core/common.c index 0dfc10f..333c873 100644 --- a/src/core/common.c +++ b/src/core/common.c @@ -46,6 +46,9 @@ int get_cmdline_name(pid_t pid, char *cmdline, size_t cmdline_size) char buf[PATH_MAX + 1]; char *filename; + if (!cmdline_size) + return -1; + snprintf(buf, sizeof(buf), "/proc/%d/cmdline", pid); fd = open(buf, O_RDONLY); if (fd < 0) { @@ -58,15 +61,13 @@ int get_cmdline_name(pid_t pid, char *cmdline, size_t cmdline_size) if (ret < 0) return -1; - buf[PATH_MAX] = '\0'; - filename = strrchr(buf, '/'); if (filename == NULL) filename = buf; else filename = filename + 1; - if (cmdline_size < strlen(filename) + 1) { + if ((cmdline_size - 1) < strlen(filename)) { errno = EOVERFLOW; return -1; } @@ -166,7 +167,7 @@ int sys_get_str(char *fname, char *str) char buf[BUFF_MAX] = {0}; if (sys_read_buf(fname, buf) == 0) { - strncpy(str, buf, strlen(buf)); + snprintf(str, strlen(buf) + 1, "%s", buf); return 0; } -- 2.7.4 From 0c53e039b81859dab4c33b140ccaf3f10d36f089 Mon Sep 17 00:00:00 2001 From: Wook Song Date: Tue, 21 Mar 2017 15:51:12 +0900 Subject: [PATCH 15/16] shared: systemd: Fix use-after-free and double-free defects This patch fixes the following code-leve defects according to static program analysis result: 1. USE_AFTER_FREE: A pointer variable is used after the referenced memory was deallocated by calling function 'free'. 2. DOUBLE_FREE.EX: A pointer is passed to the function 'free' after the referenced memory was deallocated. Change-Id: Ieef15d3432fb72068b9186c5807fedd1685ee79a Signed-off-by: Wook Song Reviewed-by: Chanwoo Choi --- src/shared/pass-systemd.c | 62 ++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/src/shared/pass-systemd.c b/src/shared/pass-systemd.c index b3a3c12..d43f7f6 100644 --- a/src/shared/pass-systemd.c +++ b/src/shared/pass-systemd.c @@ -155,15 +155,16 @@ int pass_systemd_stop_unit(char *name) return pass_systemd_start_or_stop_unit("StopUnit", name); } -static char *pass_systemd_get_unit_dbus_path(const char *unit) +static void pass_systemd_get_unit_dbus_path(const char *unit, + char *path, int max_path_len) { - char *path = NULL; int i; size_t p, k, prefix_len, unit_len = strlen(unit); size_t path_len, len, escape; assert(unit); + memset(path, '\0', sizeof(char) * max_path_len); for (escape = 0, p = 0; p < unit_len; escape++) { k = strcspn(unit+p, SYSTEMD_UNIT_ESCAPE_CHAR); if (p+k >= unit_len) @@ -179,11 +180,11 @@ static char *pass_systemd_get_unit_dbus_path(const char *unit) * char('-') is changed to three of char("_2d"). So the total * length will be: */ /* (PREFIX) + (unit - escape + 3*escape) + NULL */ - path_len = prefix_len + (unit_len - escape) - + (escape * 3 * sizeof(char)) + 1; - path = (char *)calloc(path_len, sizeof(char)); - if (!path) - return NULL; + path_len = prefix_len + (unit_len - escape) + (escape * 3) + 1; + if (max_path_len < path_len) { + path[0] = '\0'; + return; + } strncpy(path, SYSTEMD_DBUS_UNIT_PATH, prefix_len + 1); for (i = 0, p = 0; i <= escape; i++) { @@ -197,8 +198,6 @@ static char *pass_systemd_get_unit_dbus_path(const char *unit) p += k+1; } } - free(path); - return path; } static int pass_systemd_get_property(const char *name, @@ -246,45 +245,48 @@ int pass_systemd_get_unit_property(const char *unit, const char *property, GVariant **variant) { - int r; - char *escaped; + char path[PATH_MAX]; + int ret; assert(unit); assert(property); - escaped = pass_systemd_get_unit_dbus_path(unit); + pass_systemd_get_unit_dbus_path(unit, path, PATH_MAX); + if (!strlen(path)) + return -1; - r = pass_systemd_get_property(SYSTEMD_DBUS_DEST, - escaped, - DBUS_IFACE_DBUS_PROPERTIES, - "Get", - SYSTEMD_DBUS_IFACE_UNIT, - property, - variant); - free(escaped); - return r; + ret = pass_systemd_get_property(SYSTEMD_DBUS_DEST, + path, + DBUS_IFACE_DBUS_PROPERTIES, + "Get", + SYSTEMD_DBUS_IFACE_UNIT, + property, + variant); + + return ret; } int pass_systemd_get_service_property(const char *unit, const char *property, GVariant **variant) { + char path[PATH_MAX]; int ret; - char *escaped; assert(unit); assert(property); - escaped = pass_systemd_get_unit_dbus_path(unit); + pass_systemd_get_unit_dbus_path(unit, path, PATH_MAX); + if (!strlen(path)) + return -1; ret = pass_systemd_get_property(SYSTEMD_DBUS_DEST, - escaped, - DBUS_IFACE_DBUS_PROPERTIES, - "Get", - SYSTEMD_DBUS_IFACE_SERVICE, - property, - variant); - free(escaped); + path, + DBUS_IFACE_DBUS_PROPERTIES, + "Get", + SYSTEMD_DBUS_IFACE_SERVICE, + property, + variant); return ret; } -- 2.7.4 From c2af7de627fefc711b2e62ef59033cf4a3cf3d52 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Mon, 27 Mar 2017 17:16:20 +0900 Subject: [PATCH 16/16] packaging: Remove unneeded script code This patch removes the unneeded script code in the pass.spec file and add the 'ExclusiveArch' defintion to build the pass only for both {arm} and aarch64 architecture. Change-Id: Id3cbb54b2fd4e18d9e909dbf54165ecbf2a7403d Suggested-by: Seung-Woo Kim Signed-off-by: Chanwoo Choi --- packaging/pass.spec | 39 +++------------------------------------ 1 file changed, 3 insertions(+), 36 deletions(-) diff --git a/packaging/pass.spec b/packaging/pass.spec index 3e3f798..268f252 100644 --- a/packaging/pass.spec +++ b/packaging/pass.spec @@ -3,17 +3,6 @@ %define daemon_name pass %define hal_name pass-hal-devel -%if "%{?profile}" == "mobile" -%endif -%if "%{?profile}" == "wearable" -%endif -%if "%{?profile}" == "tv" -%endif -%if "%{?profile}" == "ivi" -%if "%{?_repository}" == "x86_64" -%endif -%endif - Name: %{daemon_name} Summary: Power Aware System Service Version: 0.0.1 @@ -24,6 +13,8 @@ Source0: %{name}-%{version}.tar.gz Source1: %{name}.manifest Source2: %{hal_name}.manifest +ExclusiveArch: %{arm} aarch64 + BuildRequires: cmake BuildRequires: libattr-devel BuildRequires: gettext-devel @@ -61,35 +52,10 @@ Header files required to build pass-hal packages for specific boards. %prep %setup -q -%if %{with emulator} - %define ARCH emulator -%else - %ifarch %{arm} aarch64 - %define ARCH arm - %else - %define ARCH x86 - %endif -%endif - -%ifarch %{arm} %ix86 - %define ARCH_BIT 32 -%else - %define ARCH_BIT 64 -%endif - -%if 0%{?tizen_build_devel_mode} == 1 -%define engineer_mode on -%else -%define engineer_mode off -%endif %cmake . \ -DTZ_SYS_ETC=%TZ_SYS_ETC \ -DCMAKE_INSTALL_PREFIX=%{_prefix} \ - -DARCH=%{ARCH} \ - -DARCH_BIT=%{ARCH_BIT} \ - -DENGINEER_MODE=%{engineer_mode} \ - -DPROFILE=%{profile} \ #eol %build @@ -129,6 +95,7 @@ systemctl daemon-reload %{_unitdir}/sockets.target.wants/%{daemon_name}.socket %{_unitdir}/%{daemon_name}.service %{_unitdir}/%{daemon_name}.socket + %files -n %{hal_name} %defattr(-,root,root,-) %manifest %{hal_name}.manifest -- 2.7.4