From 5483fc2223a9c46eafe501dd6b69d58ecd45c879 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Thu, 1 Sep 2022 17:32:01 +0900 Subject: [PATCH 01/16] tests: resource-monitor-tests: Fix test fail by changing the return value commit b5d308519f73("util: resource: Change error code from -EPERM to -EACCES") changed the return value from -EPERM to -EACCESS. In order to pass the test, change the return value according to commit b5d308519f73. Change-Id: I2e3261fe80ceea0fd70645484778916a928e8872 Signed-off-by: Chanwoo Choi --- tests/integration-test/resource-monitor-tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration-test/resource-monitor-tests.cpp b/tests/integration-test/resource-monitor-tests.cpp index 8d62db8..f588989 100644 --- a/tests/integration-test/resource-monitor-tests.cpp +++ b/tests/integration-test/resource-monitor-tests.cpp @@ -184,7 +184,7 @@ TEST_F(ResourceMonitorTest, EXPECT_EQ(ret, 0); ret = pass_resource_monitor_set_resource_attr(id, res_id, res_attr_id); - EXPECT_EQ(ret, -EPERM); + EXPECT_EQ(ret, -EACCES); ret = pass_resource_monitor_delete_resource(id, res_id); EXPECT_EQ(ret, 0); -- 2.7.4 From a8b342aa318d95d043cdff721e80033b82d0b201 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Thu, 1 Sep 2022 19:10:50 +0900 Subject: [PATCH 02/16] lib: resource-monitor: Remove unneeded privilege check commit fccfb02d128c("lib: resource-monitor: Check systemmonitor privilege") added the unneeded privilege check code. In result, cynara is running by preempting CPU resource. So that remove unneeded privilege check code. Only check the privilege when pass_resource_monitor_init() call. Change-Id: I0a9c7b2ca76a781dcf88ab19ddf0ec597c98b6a5 Signed-off-by: Chanwoo Choi --- lib/resource-monitor/resource-monitor.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/resource-monitor/resource-monitor.c b/lib/resource-monitor/resource-monitor.c index e7b567e..a7ab07e 100644 --- a/lib/resource-monitor/resource-monitor.c +++ b/lib/resource-monitor/resource-monitor.c @@ -128,9 +128,6 @@ static inline int handle_request(struct request_data *data) if (!data) return TIZEN_ERROR_INVALID_PARAMETER; - if (!is_privilege_supported(PRIVILEGE_SYSTEMMONITOR)) - return TIZEN_ERROR_PERMISSION_DENIED; - /* Make buffer with struct request_data according to request */ switch (data->request) { case REQUEST_UPDATE_RESOURCE_ALL: @@ -343,9 +340,6 @@ int pass_resource_monitor_exit(int id) { struct pass_resource_monitor_client *client; - if (!is_privilege_supported(PRIVILEGE_SYSTEMMONITOR)) - return TIZEN_ERROR_PERMISSION_DENIED; - client = find_client_by_id(id); if (!client) { _E("[libpass] cannot find client-%d", id); @@ -518,9 +512,6 @@ static int pass_resource_monitor_get_json(int id, char *json_string, int request char *buffer; va_list args; - if (!is_privilege_supported(PRIVILEGE_SYSTEMMONITOR)) - return TIZEN_ERROR_PERMISSION_DENIED; - buffer = malloc(HUGE_BUFF_MAX + 1); if (!buffer) return TIZEN_ERROR_OUT_OF_MEMORY; @@ -720,9 +711,6 @@ pass_resource_monitor_get_array(int id, int res_id, u_int64_t attr_id, int data_ int response_req; int ret, i; - if (!is_privilege_supported(PRIVILEGE_SYSTEMMONITOR)) - return TIZEN_ERROR_PERMISSION_DENIED; - buffer = malloc(HUGE_BUFF_MAX + 1); if (!buffer) return TIZEN_ERROR_OUT_OF_MEMORY; -- 2.7.4 From 1bbc88af8380214499fdeac39f34ff1f8c0322b1 Mon Sep 17 00:00:00 2001 From: Sung-hun Kim Date: Fri, 2 Sep 2022 14:41:47 +0900 Subject: [PATCH 03/16] monitor: lib: Support Unix domain socket (UDS) and TCP/IP socket concurrently Previous TCP/IP socket-based IPC has a security issue that anyone can access the opened port without credential. Since UDS is exported as a file, we can apply file-based access control to it. So, we use UDS instead of TCP/IP socket by default. At the same time, we also support TCP/IP socket for resource-monitor-tool which is a web-based monitoring tool in user space. In the future, we will support TCP/IP socket only in debugging mode which is turned on/off by root- privileged command line tool. Change-Id: Id46a5e7560cf95d09f31222e5022ce3ea209753a Signed-off-by: Sung-hun Kim --- CMakeLists.txt | 6 ++ lib/resource-monitor/resource-monitor.c | 18 ++-- packaging/pass.spec | 3 + src/monitor/request-handler.c | 146 ++++++++++++++++++++++++++------ systemd/pass-resource-monitor.socket.in | 10 +++ 5 files changed, 146 insertions(+), 37 deletions(-) create mode 100644 systemd/pass-resource-monitor.socket.in diff --git a/CMakeLists.txt b/CMakeLists.txt index 4ea606a..18f595b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -123,6 +123,7 @@ INSTALL(FILES ${CMAKE_CURRENT_SOURCE_DIR}/scripts/${PROJECT_NAME}.conf DESTINATI INSTALL(FILES ${CMAKE_CURRENT_SOURCE_DIR}/scripts/pass-pmqos.json DESTINATION /etc/pass) INSTALL(FILES ${CMAKE_CURRENT_SOURCE_DIR}/scripts/pass-thermal.json DESTINATION /etc/pass) CONFIGURE_FILE(${CMAKE_SOURCE_DIR}/systemd/${PROJECT_NAME}.service.in ${CMAKE_SOURCE_DIR}/systemd/${PROJECT_NAME}.service @ONLY) +CONFIGURE_FILE(${CMAKE_SOURCE_DIR}/systemd/${PROJECT_NAME}-resource-monitor.socket.in ${CMAKE_SOURCE_DIR}/systemd/${PROJECT_NAME}-resource-monitor.socket @ONLY) INSTALL(FILES ${CMAKE_SOURCE_DIR}/systemd/org.tizen.system.pass.service DESTINATION /usr/share/dbus-1/system-services) INSTALL(FILES ${CMAKE_SOURCE_DIR}/systemd/org.tizen.system.thermal.service DESTINATION /usr/share/dbus-1/system-services) @@ -131,6 +132,11 @@ INSTALL(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/systemd/ DESTINATION lib/systemd/s PATTERN "${PROJECT_NAME}.service" ) +INSTALL(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/systemd/ DESTINATION lib/systemd/system + FILES_MATCHING + PATTERN "${PROJECT_NAME}-resource-monitor.socket" + ) + ADD_SUBDIRECTORY(tests/integration-test) ADD_SUBDIRECTORY(tests/haltest) ADD_SUBDIRECTORY(tests/unittest) diff --git a/lib/resource-monitor/resource-monitor.c b/lib/resource-monitor/resource-monitor.c index a7ab07e..7a08699 100644 --- a/lib/resource-monitor/resource-monitor.c +++ b/lib/resource-monitor/resource-monitor.c @@ -30,11 +30,11 @@ #include #include #include +#include #include #include #include #include - #include #include "resource-monitor.h" @@ -289,8 +289,9 @@ static inline int handle_request(struct request_data *data) EXPORT int pass_resource_monitor_init(void) { + const char *server_socket_path = "/run/.pass-resource-monitor.socket"; struct pass_resource_monitor_client *client; - struct sockaddr_in server_addr; + struct sockaddr_un server_addr; int ret = TIZEN_ERROR_NO_DATA; if (!is_privilege_supported(PRIVILEGE_SYSTEMMONITOR)) @@ -303,19 +304,16 @@ int pass_resource_monitor_init(void) } /* open socket to server */ - client->id = socket(AF_INET, SOCK_STREAM, 0); + client->id = socket(AF_UNIX, SOCK_STREAM, 0); if (client->id < 0) { _E("[libpass] socket creation error"); goto err_out_free; } - server_addr.sin_family = AF_INET; - server_addr.sin_port = htons(REQUEST_SERVER_PORT); - - if (inet_pton(AF_INET, REQUEST_SERVER_IP, &server_addr.sin_addr) <= 0) { - _E("[libpass] invalid address"); - goto err_out_close; - } + bzero(&server_addr, sizeof(server_addr)); + server_addr.sun_family = AF_UNIX; + strncpy(server_addr.sun_path, server_socket_path, sizeof(server_addr.sun_path)); + server_addr.sun_path[sizeof(server_addr.sun_path) - 1] = '\0'; if (connect(client->id, (struct sockaddr *)&server_addr, sizeof(server_addr)) < 0) { diff --git a/packaging/pass.spec b/packaging/pass.spec index 4444495..dac786f 100644 --- a/packaging/pass.spec +++ b/packaging/pass.spec @@ -94,6 +94,7 @@ rm -rf %{buildroot} %make_install %install_service delayed.target.wants %{daemon_name}.service +%install_service sockets.target.wants %{daemon_name}-resource-monitor.socket %post @@ -119,7 +120,9 @@ systemctl daemon-reload %{_bindir}/%{daemon_name} %{_bindir}/resource-monitor %{_unitdir}/delayed.target.wants/%{daemon_name}.service +%{_unitdir}/sockets.target.wants/%{daemon_name}-resource-monitor.socket %{_unitdir}/%{daemon_name}.service +%{_unitdir}/%{daemon_name}-resource-monitor.socket %{_datadir}/dbus-1/system-services/org.tizen.system.pass.service %{_datadir}/dbus-1/system-services/org.tizen.system.thermal.service diff --git a/src/monitor/request-handler.c b/src/monitor/request-handler.c index 5d30833..324d0fc 100644 --- a/src/monitor/request-handler.c +++ b/src/monitor/request-handler.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -1078,53 +1079,129 @@ static int create_request_client(int socket_fd) return 0; } -static int request_server_func(void *ctx, void **result) +static int init_unix_socket(int *sock, struct sockaddr_un *address, int *addrlen) { - struct sockaddr_in address; - struct timeval wait; + const char *server_unix_socket_path = "/run/.pass-resource-monitor.socket"; int opt = true; - int server_socket; - int addrlen; - int ret; - fd_set fds; - if (!g_request_server_run) - return THREAD_RETURN_DONE; - - init_resource_id(); + if (!sock || !address || !addrlen) + return -EINVAL; - /* 0. initialize server socket */ - server_socket = socket(AF_INET, SOCK_STREAM, 0); - if (server_socket < 0) { + *sock = socket(AF_UNIX, SOCK_STREAM, 0); + if (*sock < 0) { _E("Failed to initialize socket"); goto error_out; } - if (setsockopt(server_socket, SOL_SOCKET, SO_REUSEADDR, + if (setsockopt(*sock, SOL_SOCKET, SO_REUSEADDR, (char *)&opt, sizeof(opt)) < 0) { _E("Failed to setsockopt"); goto error_out_close; } - address.sin_family = AF_INET; - address.sin_addr.s_addr = INADDR_ANY; - address.sin_port = htons(REQUEST_SERVER_PORT); + bzero(address, sizeof(*address)); + address->sun_family = AF_UNIX; + strncpy(address->sun_path, server_unix_socket_path, sizeof(address->sun_path)); + address->sun_path[sizeof(address->sun_path) - 1] = '\0'; + + if (!access(server_unix_socket_path, F_OK)) + unlink(server_unix_socket_path); - if (bind(server_socket, (struct sockaddr *)&address, sizeof(address)) < 0) { + if (bind(*sock, (struct sockaddr *)address, sizeof(*address)) < 0) { _E("Failed to bind"); goto error_out_close; } - if (listen(server_socket, PENDING_MAX) < 0) { + if (listen(*sock, PENDING_MAX) < 0) { _E("Failed to begin listenning"); goto error_out_close; } - addrlen = sizeof(address); + *addrlen = sizeof(*address); + + return 0; + +error_out_close: + close(*sock); +error_out: + return -EIO; +} + +int init_ip_socket(int *sock, struct sockaddr_in *address, int *addrlen) +{ + int opt = true; + + if (!sock || !address || !addrlen) + return -EINVAL; + + *sock = socket(AF_INET, SOCK_STREAM, 0); + if (*sock < 0) { + _E("Failed to initialize TCP/IP socket"); + goto error_out; + } + + if (setsockopt(*sock, SOL_SOCKET, SO_REUSEADDR, + (char *)&opt, sizeof(opt)) < 0) { + _E("Failed to setsockopt for TCP/IP socket"); + goto error_out_close; + } + + bzero(address, sizeof(*address)); + address->sin_family = AF_INET; + address->sin_addr.s_addr = INADDR_ANY; + address->sin_port = htons(REQUEST_SERVER_PORT); + + if (bind(*sock, (struct sockaddr *)address, sizeof(*address)) < 0) { + _E("Failed to bind for TCP/IP socket"); + goto error_out_close; + } + + if (listen(*sock, PENDING_MAX) < 0) { + _E("Failed to begin listenning for TCP/IP socket"); + goto error_out_close; + } + + *addrlen = sizeof(*address); + + return 0; + +error_out_close: + close(*sock); +error_out: + return -EIO; +} + +static int request_server_func(void *ctx, void **result) +{ + struct sockaddr_un unix_address; + struct sockaddr_in ip_address; + struct timeval wait; + int server_unix_socket; + int server_ip_socket; + int unix_addrlen; + int ip_addrlen; + int ret; + fd_set fds; + + if (!g_request_server_run) + return THREAD_RETURN_DONE; + + init_resource_id(); + + /* 0. initialize server socket */ + ret = init_unix_socket(&server_unix_socket, &unix_address, &unix_addrlen); + if (ret < 0) + goto error_out; + + /* initialize TCP socket */ + ret = init_ip_socket(&server_ip_socket, &ip_address, &ip_addrlen); + if (ret < 0) + goto error_out_close_server_unix_socket; while (g_request_server_run) { FD_ZERO(&fds); - FD_SET(server_socket, &fds); + FD_SET(server_unix_socket, &fds); + FD_SET(server_ip_socket, &fds); wait.tv_sec = 1; wait.tv_usec = 0; @@ -1135,9 +1212,21 @@ static int request_server_func(void *ctx, void **result) goto error_out_close; } - if (FD_ISSET(server_socket, &fds)) { - int new_socket = accept(server_socket, (struct sockaddr *)&address, - (socklen_t *)&addrlen); + if (FD_ISSET(server_unix_socket, &fds)) { + int new_socket = accept(server_unix_socket, (struct sockaddr *)&unix_address, + (socklen_t *)&unix_addrlen); + + if (new_socket < 0) { + _E("Failed to accept"); + goto error_out_close; + } + + create_request_client(new_socket); + } + + if (FD_ISSET(server_ip_socket, &fds)) { + int new_socket = accept(server_ip_socket, (struct sockaddr *)&ip_address, + (socklen_t *)&ip_addrlen); if (new_socket < 0) { _E("Failed to accept"); @@ -1148,12 +1237,15 @@ static int request_server_func(void *ctx, void **result) } } - close(server_socket); + close(server_ip_socket); + close(server_unix_socket); return THREAD_RETURN_DONE; error_out_close: - close(server_socket); + close(server_ip_socket); +error_out_close_server_unix_socket: + close(server_unix_socket); error_out: return THREAD_RETURN_ERROR; diff --git a/systemd/pass-resource-monitor.socket.in b/systemd/pass-resource-monitor.socket.in new file mode 100644 index 0000000..406f96f --- /dev/null +++ b/systemd/pass-resource-monitor.socket.in @@ -0,0 +1,10 @@ +[Unit] +Description=Pass resource-monitor socket + +[Socket] +SocketUser=system_fw +SocketGroup=system_fw +ListenStream=/run/.pass.socket +SocketMode=0777 +SmackLabelIPIn=* +SmackLabelIPOut=@ -- 2.7.4 From 47bcf74f5789b3db5afc2842d1b2c08244aed7a5 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Wed, 7 Sep 2022 12:05:52 +0900 Subject: [PATCH 04/16] util: kernel: Fix possible vulnerability Change-Id: I14a31746037d78670418b2b3b615e042b9a6aab4 Signed-off-by: Dongwoo Lee --- src/util/kernel.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/kernel.c b/src/util/kernel.c index 996c83d..6259968 100644 --- a/src/util/kernel.c +++ b/src/util/kernel.c @@ -481,7 +481,10 @@ int kernel_get_thread_group_map_info(struct proc_map_info *map_info, smaps_fd = fopen(smap_file_path, "r"); if (!smaps_fd) { - _E("failed to open smap: %s", strerror(errno)); + char errstr[BUFF_MAX]; + + strerror_r(errno, errstr, BUFF_MAX); + _E("failed to open smap: %s", errstr); return -ENOENT; } -- 2.7.4 From 873f5cb6eb8c6c0e3c154918fe936f940b245980 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Mon, 27 Jun 2022 18:30:13 +0900 Subject: [PATCH 05/16] tests: unittest: Add pass-parser invalid test Add pass-parser invalid unittests for each section - Test for invalid level section - Test for invalid pmqos section - Test for invalid thermal section - Test for invalid header section - Test for invalid battery h/w resource type - Test for invalid memory h/w resource type Change-Id: Ibdf109751560fc130ce3d8680867c9ac81be1c56 Signed-off-by: Chanwoo Choi --- tests/unittest/pass-unittests.cc | 43 +++++++++++++- .../scripts/invalid-battery/pass-battery.json | 51 ++++++++++++++++ tests/unittest/scripts/invalid-battery/pass.json | 12 ++++ .../unittest/scripts/invalid-header/pass-bus0.json | 46 +++++++++++++++ tests/unittest/scripts/invalid-header/pass.json | 11 ++++ .../unittest/scripts/invalid-level/pass-cpu0.json | 64 +++++++++++++++++++++ tests/unittest/scripts/invalid-level/pass.json | 13 +++++ .../scripts/invalid-memory/pass-memory.json | 33 +++++++++++ tests/unittest/scripts/invalid-memory/pass.json | 10 ++++ .../unittest/scripts/invalid-pmqos/pass-cpu0.json | 64 +++++++++++++++++++++ tests/unittest/scripts/invalid-pmqos/pass.json | 13 +++++ .../unittest/scripts/invalid-thermal/pass-cpu.json | 64 +++++++++++++++++++++ .../scripts/invalid-thermal/pass-cpu1.json | 67 ++++++++++++++++++++++ tests/unittest/scripts/invalid-thermal/pass.json | 13 +++++ tests/unittest/scripts/invalid-thermal/pass1.json | 13 +++++ 15 files changed, 516 insertions(+), 1 deletion(-) create mode 100644 tests/unittest/scripts/invalid-battery/pass-battery.json create mode 100644 tests/unittest/scripts/invalid-battery/pass.json create mode 100644 tests/unittest/scripts/invalid-header/pass-bus0.json create mode 100644 tests/unittest/scripts/invalid-header/pass.json create mode 100644 tests/unittest/scripts/invalid-level/pass-cpu0.json create mode 100644 tests/unittest/scripts/invalid-level/pass.json create mode 100644 tests/unittest/scripts/invalid-memory/pass-memory.json create mode 100644 tests/unittest/scripts/invalid-memory/pass.json create mode 100644 tests/unittest/scripts/invalid-pmqos/pass-cpu0.json create mode 100644 tests/unittest/scripts/invalid-pmqos/pass.json create mode 100644 tests/unittest/scripts/invalid-thermal/pass-cpu.json create mode 100644 tests/unittest/scripts/invalid-thermal/pass-cpu1.json create mode 100644 tests/unittest/scripts/invalid-thermal/pass.json create mode 100644 tests/unittest/scripts/invalid-thermal/pass1.json diff --git a/tests/unittest/pass-unittests.cc b/tests/unittest/pass-unittests.cc index 6408468..cda27aa 100644 --- a/tests/unittest/pass-unittests.cc +++ b/tests/unittest/pass-unittests.cc @@ -526,7 +526,6 @@ TEST(PassParserTest, pass_parser_get_each_resource_config_valid) { for (i = 0; i < (int)pass.num_resources; i++) { struct pass_resource *res = &pass.res[i]; - ret = pass_parser_get_each_resource_config(res, res->config_data.path_conf_file); EXPECT_EQ(ret, 0); @@ -539,6 +538,48 @@ TEST(PassParserTest, pass_parser_get_each_resource_config_valid) { pass_parser_put_resource_config(&pass); } +class ResourceConfigInvalid { +public: + const char *path_json; + + ResourceConfigInvalid( + const char *path_json_) : + path_json(path_json_) {} +}; + +class PassParserInvalidTest : public ::testing::TestWithParam {}; + +INSTANTIATE_TEST_CASE_P (PassParserTest, PassParserInvalidTest, + ::testing::Values ( + ResourceConfigInvalid ("./scripts/invalid-level/pass.json"), + ResourceConfigInvalid ("./scripts/invalid-pmqos/pass.json"), + ResourceConfigInvalid ("./scripts/invalid-thermal/pass.json"), + ResourceConfigInvalid ("./scripts/invalid-thermal/pass1.json"), + ResourceConfigInvalid ("./scripts/invalid-header/pass.json"), + ResourceConfigInvalid ("./scripts/invalid-battery/pass.json"), + ResourceConfigInvalid ("./scripts/invalid-memory/pass.json") +)); + +TEST_P(PassParserInvalidTest, pass_parser_get_each_resource_config_invalid) { + auto param = GetParam(); + struct pass pass; + unsigned int i; + + int ret = pass_parser_get_resource_config(&pass, param.path_json); + EXPECT_EQ(ret, 0); + + for (i = 0; i < pass.num_resources; i++) { + struct pass_resource *res = &pass.res[i]; + ret = pass_parser_get_each_resource_config(res, + res->config_data.path_conf_file); + EXPECT_NE(ret, 0); + } + + for (i = 0; i < pass.num_resources; i++) + pass_parser_put_each_resource_config(&pass.res[i]); + pass_parser_put_resource_config(&pass); +} + int main(int argc, char *argv[]) { try { diff --git a/tests/unittest/scripts/invalid-battery/pass-battery.json b/tests/unittest/scripts/invalid-battery/pass-battery.json new file mode 100644 index 0000000..01b822a --- /dev/null +++ b/tests/unittest/scripts/invalid-battery/pass-battery.json @@ -0,0 +1,51 @@ +{ + "support" : true, + "init_level" : 0, + "level_list" : + [ + { + "level" : 0, + //"battery,charging_status" : 0, // POWER_SUPPLY_STATUS_UNKNOWN + //"battery_charging_currnt_uA" : 2048000 + }, { + "level" : 1, + //"battery,charging_status" : 0, // POWER_SUPPLY_STATUS_UNKNOWN + //"battery,charging_currnt_uA" : 1024000 + }, { + //"level" : 2, + //"battery,charging_status" : 3, // POWER_SUPPLY_STATUS_NOT_CHARGING + //"battery,charging_currnt_uA" : 1024000 + } + ], + + "pmqos_support" : false, + + "thermal_support" : true, + "thermal_timer_interval_ms" : 5000, + "thermal_scenario_list": + [ + { + "name" : "Release", + "temperature" : { "start": 0, "end": 45, "threshold": 42}, + "timer_interval_ms" : 5000, + "target_level" : 0 + }, { + "name" : "Warning", + "temperature" : { "start": 45, "end": 50, "threshold": 47}, + "timer_interval_ms" : 3000, + "target_level" : 1 + }, { + "name" : "LimitAction", + "temperature" : { "start": 50, "end": 55, "threshold": 52}, + "timer_interval_ms" : 1000, + "target_level" : 2 + }, { + "name" : "Shutdown", + "temperature" : { "start": 55, "end": 100 }, + "timer_interval_ms" : 1000, + "target_level" : 2 + } + ], + + "cpuhp_support" : false +} diff --git a/tests/unittest/scripts/invalid-battery/pass.json b/tests/unittest/scripts/invalid-battery/pass.json new file mode 100644 index 0000000..4857561 --- /dev/null +++ b/tests/unittest/scripts/invalid-battery/pass.json @@ -0,0 +1,12 @@ +{ + "device_list" : + [ + { + "device_type" : "battery", + "device_name" : "battery", + "device_config_path" : "./scripts/invalid-battery/pass-battery.json", + "thermal_device_name" : "thermal_zone5", + "cooling_device_name" : null + } + ] +} diff --git a/tests/unittest/scripts/invalid-header/pass-bus0.json b/tests/unittest/scripts/invalid-header/pass-bus0.json new file mode 100644 index 0000000..bcc94fd --- /dev/null +++ b/tests/unittest/scripts/invalid-header/pass-bus0.json @@ -0,0 +1,46 @@ +{ + "support" : true, + //"init_level" : 0, + //"level_list" : + [ + { + "level" : 0, + "dvfs,minimum_frequency_khz" : 138000000, + "dvfs,maximum_frequency_khz" : 413000000 + }, { + "level" : 1, + "dvfs,minimum_frequency_khz" : 138000000, + "dvfs,maximum_frequency_khz" : 138000000 + }, { + "level" : 2, + "dvfs,minimum_frequency_khz" : 138000000, + "dvfs,maximum_frequency_khz" : 275000000 + }, { + "level" : 3, + "dvfs,minimum_frequency_khz" : 413000000, + "dvfs,maximum_frequency_khz" : 413000000 + } + ], + + "pmqos_support" : true, + "pmqos_scenario_list": + [ + { + "name" : "AppLaunch", + "target_level" : 3 + }, { + "name" : "UltraPowerSaving", + "target_level" : 1 + }, { + "name" : "Doze", + "target_level" : 2 + }, { + "name" : "Performance", + "target_level" : 3 + } + ], + + "thermal_support" : false, + + "cpuhp_support" : false +} diff --git a/tests/unittest/scripts/invalid-header/pass.json b/tests/unittest/scripts/invalid-header/pass.json new file mode 100644 index 0000000..1591e61 --- /dev/null +++ b/tests/unittest/scripts/invalid-header/pass.json @@ -0,0 +1,11 @@ +{ + "device_list" : + [ + { + "device_type" : "bus", + "device_name" : "devfreq0", + "device_config_path" : "./scripts/invalid-bus/pass-bus0.json", + "thermal_device_name" : null + } + ] +} diff --git a/tests/unittest/scripts/invalid-level/pass-cpu0.json b/tests/unittest/scripts/invalid-level/pass-cpu0.json new file mode 100644 index 0000000..ae42205 --- /dev/null +++ b/tests/unittest/scripts/invalid-level/pass-cpu0.json @@ -0,0 +1,64 @@ +{ + "support" : true, + "init_level" : 0, + "level_list" : + [ + { + "level" : 0, + "dvfs,minimum_frequency_khz" : 600000, + "dvfs,maximum_frequency_khz" : 1500000 + }, { + "level" : 1, + "dvfs,minimum_frequency_khz" : 1500000, + "dvfs,maximum_frequency_khz" : 1500000 + }, { + //"level" : 2, + "dvfs,minimum_frequency_khz" : 600000, + "dvfs,maximum_frequency_khz" : 600000 + } + ], + + "pmqos_support" : true, + "pmqos_scenario_list": + [ + { + "name" : "AppLaunch", + "target_level" : 1 + }, { + "name" : "UltraPowerSaving", + "target_level" : 2 + }, { + "name" : "Performance", + "target_level" : 1 + } + ], + + "thermal_support" : true, + "thermal_timer_interval_ms" : 5000, + "thermal_scenario_list": + [ + { + "name" : "Release", + "temperature" : { "start": 0, "end": 75, "threshold": 72}, + "timer_interval_ms" : 5000, + "target_level" : 0 + }, { + "name" : "Warning", + "temperature" : { "start": 75, "end": 80, "threshold": 77 }, + "timer_interval_ms" : 3000, + "target_level" : 0 + }, { + "name" : "LimitAction", + "temperature" : { "start": 80, "end": 85, "threshold": 82 }, + "timer_interval_ms" : 1000, + "target_level" : 2 + }, { + "name" : "Shutdown", + "temperature" : { "start": 85, "end": 100 }, + "timer_interval_ms" : 1000, + "target_level" : 2 + } + ], + + "cpuhp_support" : false +} diff --git a/tests/unittest/scripts/invalid-level/pass.json b/tests/unittest/scripts/invalid-level/pass.json new file mode 100644 index 0000000..e26f60d --- /dev/null +++ b/tests/unittest/scripts/invalid-level/pass.json @@ -0,0 +1,13 @@ +{ + "device_list" : + [ + { + "device_type" : "cpu", + "device_name" : "cpu0", + "device_config_path" : "./scripts/invalid-level/pass-cpu0.json", + "thermal_device_name" : "thermal_zone0", + "cpu,number_of_cpus" : 4, + "cpu,first_cpu" : 0 + } + ] +} diff --git a/tests/unittest/scripts/invalid-memory/pass-memory.json b/tests/unittest/scripts/invalid-memory/pass-memory.json new file mode 100644 index 0000000..1038fad --- /dev/null +++ b/tests/unittest/scripts/invalid-memory/pass-memory.json @@ -0,0 +1,33 @@ +{ + "support" : true, + "init_level" : 0, + "level_list" : + [ + { + "level" : 0, + "memory,fault_around_bytes" : 4096 + }, { + //"level" : 1, + //"memory,fault_around_bytes" : 65536 + } + ], + + "pmqos_support" : true, + "pmqos_scenario_list" : + [ + { + "name" : "AppLaunch", + "target_level" : 0 + }, { + "name" : "UltraPowerSaving", + "target_level" : 0 + }, { + "name" : "Performance", + "target_level" : 1 + } + ], + + "thermal_support" : false, + + "cpuhp_support" : false +} diff --git a/tests/unittest/scripts/invalid-memory/pass.json b/tests/unittest/scripts/invalid-memory/pass.json new file mode 100644 index 0000000..df8ed06 --- /dev/null +++ b/tests/unittest/scripts/invalid-memory/pass.json @@ -0,0 +1,10 @@ +{ + "device_list" : + [ + { + "device_type" : "memory", + "device_name" : "memory", + "device_config_path" : "./scripts/invalid-memory/pass-memory.json" + } + ] +} diff --git a/tests/unittest/scripts/invalid-pmqos/pass-cpu0.json b/tests/unittest/scripts/invalid-pmqos/pass-cpu0.json new file mode 100644 index 0000000..13fe285 --- /dev/null +++ b/tests/unittest/scripts/invalid-pmqos/pass-cpu0.json @@ -0,0 +1,64 @@ +{ + "support" : true, + "init_level" : 0, + "level_list" : + [ + { + "level" : 0, + "dvfs,minimum_frequency_khz" : 600000, + "dvfs,maximum_frequency_khz" : 1500000 + }, { + "level" : 1, + "dvfs,minimum_frequency_khz" : 1500000, + "dvfs,maximum_frequency_khz" : 1500000 + }, { + "level" : 2, + "dvfs,minimum_frequency_khz" : 600000, + "dvfs,maximum_frequency_khz" : 600000 + } + ], + + "pmqos_support" : true, + "pmqos_scenario_list": + [ + { + "name" : "AppLaunch", + "target_level" : 1 + }, { + //"name" : "UltraPowerSaving", + //"target_level" : 2 + }, { + "name" : "Performance", + "target_level" : 1 + } + ], + + "thermal_support" : true, + "thermal_timer_interval_ms" : 5000, + "thermal_scenario_list": + [ + { + "name" : "Release", + "temperature" : { "start": 0, "end": 75, "threshold": 72}, + "timer_interval_ms" : 5000, + "target_level" : 0 + }, { + "name" : "Warning", + "temperature" : { "start": 75, "end": 80, "threshold": 77 }, + "timer_interval_ms" : 3000, + "target_level" : 0 + }, { + "name" : "LimitAction", + "temperature" : { "start": 80, "end": 85, "threshold": 82 }, + "timer_interval_ms" : 1000, + "target_level" : 2 + }, { + "name" : "Shutdown", + "temperature" : { "start": 85, "end": 100 }, + "timer_interval_ms" : 1000, + "target_level" : 2 + } + ], + + "cpuhp_support" : false +} diff --git a/tests/unittest/scripts/invalid-pmqos/pass.json b/tests/unittest/scripts/invalid-pmqos/pass.json new file mode 100644 index 0000000..e35f722 --- /dev/null +++ b/tests/unittest/scripts/invalid-pmqos/pass.json @@ -0,0 +1,13 @@ +{ + "device_list" : + [ + { + "device_type" : "cpu", + "device_name" : "cpu0", + "device_config_path" : "./scripts/invalid-pmqos/pass-cpu0.json", + "thermal_device_name" : "thermal_zone0", + "cpu,number_of_cpus" : 4, + "cpu,first_cpu" : 0 + } + ] +} diff --git a/tests/unittest/scripts/invalid-thermal/pass-cpu.json b/tests/unittest/scripts/invalid-thermal/pass-cpu.json new file mode 100644 index 0000000..51d546b --- /dev/null +++ b/tests/unittest/scripts/invalid-thermal/pass-cpu.json @@ -0,0 +1,64 @@ +{ + "support" : true, + "init_level" : 0, + "level_list" : + [ + { + "level" : 0, + "dvfs,minimum_frequency_khz" : 600000, + "dvfs,maximum_frequency_khz" : 1500000 + }, { + "level" : 1, + "dvfs,minimum_frequency_khz" : 1500000, + "dvfs,maximum_frequency_khz" : 1500000 + }, { + "level" : 2, + "dvfs,minimum_frequency_khz" : 600000, + "dvfs,maximum_frequency_khz" : 600000 + } + ], + + "pmqos_support" : true, + "pmqos_scenario_list": + [ + { + "name" : "AppLaunch", + "target_level" : 1 + }, { + "name" : "UltraPowerSaving", + "target_level" : 2 + }, { + "name" : "Performance", + "target_level" : 1 + } + ], + + "thermal_support" : true, + "thermal_timer_interval_ms" : 5000, + "thermal_scenario_list": + [ + { + "name" : "Release", + "temperature" : { "start": 0, "end": 75, "threshold": 72}, + "timer_interval_ms" : 5000, + "target_level" : 0 + }, { + //"name" : "Warning", + //"temperature" : { "start": 75, "end": 80, "threshold": 77 }, + //"timer_interval_ms" : 3000, + //"target_level" : 0 + }, { + "name" : "LimitAction", + "temperature" : { "start": 80, "end": 85, "threshold": 82 }, + "timer_interval_ms" : 1000, + "target_level" : 2 + }, { + "name" : "Shutdown", + "temperature" : { "start": 85, "end": 100 }, + "timer_interval_ms" : 1000, + "target_level" : 2 + } + ], + + "cpuhp_support" : false +} diff --git a/tests/unittest/scripts/invalid-thermal/pass-cpu1.json b/tests/unittest/scripts/invalid-thermal/pass-cpu1.json new file mode 100644 index 0000000..1e5521f --- /dev/null +++ b/tests/unittest/scripts/invalid-thermal/pass-cpu1.json @@ -0,0 +1,67 @@ +{ + "support" : true, + "init_level" : 0, + "level_list" : + [ + { + "level" : 0, + "dvfs,minimum_frequency_khz" : 600000, + "dvfs,maximum_frequency_khz" : 1500000 + }, { + "level" : 1, + "dvfs,minimum_frequency_khz" : 1500000, + "dvfs,maximum_frequency_khz" : 1500000 + }, { + "level" : 2, + "dvfs,minimum_frequency_khz" : 600000, + "dvfs,maximum_frequency_khz" : 600000 + } + ], + + "pmqos_support" : true, + "pmqos_scenario_list": + [ + { + "name" : "AppLaunch", + "target_level" : 1 + }, { + "name" : "UltraPowerSaving", + "target_level" : 2 + }, { + "name" : "Performance", + "target_level" : 1 + } + ], + + "thermal_support" : true, + "thermal_timer_interval_ms" : 5000, + "thermal_scenario_list": + [ + { + "name" : "Release", + "temperature" : { "start": 0, "end": 75, "threshold": 72}, + "timer_interval_ms" : 5000, + "target_level" : 0 + }, { + "name" : "Warning", + "temperature" : { "start": 75, "end": 80, "threshold": 77 }, + "timer_interval_ms" : 3000, + "target_level" : 0 + }, { + "name" : "LimitAction", + "temperature" : + { + //"start": 80, "end": 85, "threshold": 82 + }, + "timer_interval_ms" : 1000, + "target_level" : 2 + }, { + "name" : "Shutdown", + "temperature" : { "start": 85, "end": 100 }, + "timer_interval_ms" : 1000, + "target_level" : 2 + } + ], + + "cpuhp_support" : false +} diff --git a/tests/unittest/scripts/invalid-thermal/pass.json b/tests/unittest/scripts/invalid-thermal/pass.json new file mode 100644 index 0000000..044b17b --- /dev/null +++ b/tests/unittest/scripts/invalid-thermal/pass.json @@ -0,0 +1,13 @@ +{ + "device_list" : + [ + { + "device_type" : "cpu", + "device_name" : "cpu0", + "device_config_path" : "./scripts/invalid-thermal/pass-cpu.json", + "thermal_device_name" : "thermal_zone0", + "cpu,number_of_cpus" : 4, + "cpu,first_cpu" : 0 + } + ] +} diff --git a/tests/unittest/scripts/invalid-thermal/pass1.json b/tests/unittest/scripts/invalid-thermal/pass1.json new file mode 100644 index 0000000..212862f --- /dev/null +++ b/tests/unittest/scripts/invalid-thermal/pass1.json @@ -0,0 +1,13 @@ +{ + "device_list" : + [ + { + "device_type" : "cpu", + "device_name" : "cpu0", + "device_config_path" : "./scripts/invalid-thermal/pass-cpu1.json", + "thermal_device_name" : "thermal_zone0", + "cpu,number_of_cpus" : 4, + "cpu,first_cpu" : 0 + } + ] +} -- 2.7.4 From 782dd36d42a7c1eb47428e6908235d9447a39962 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Wed, 31 Aug 2022 23:28:33 +0900 Subject: [PATCH 06/16] pass: parser: Add get_property_set to simplify getting properties And get_property_set function to reduce the get_property function call. It makes the getting propertis more simpler than before Also, get the property value from json configuration file and then stored them to local variables. And fill out the allocated memory of structure with local variables. But, local variables are not necessary. In order to reduce the LOC (line of code), get property value directly without local variables. Change-Id: Ib993d11d6ad22d3fd12fca54a591ae30cd3b9b20 Signed-off-by: Chanwoo Choi --- include/util/common.h | 9 + src/pass/pass-parser.c | 514 +++++++++++++++---------------------------- src/pmqos/pmqos-parser.c | 22 +- src/thermal/thermal-parser.c | 17 +- src/util/common.c | 15 +- 5 files changed, 219 insertions(+), 358 deletions(-) diff --git a/include/util/common.h b/include/util/common.h index a11475e..213ed70 100644 --- a/include/util/common.h +++ b/include/util/common.h @@ -123,6 +123,13 @@ enum data_type { (type *)((char *)__mptr - offsetof(type, member)); }) #endif +struct config_property { + const char *name; + int type; + bool mandatory; + void *value; +}; + int sys_get_str(const char *fname, char *str); int sysfs_get_int(char *path, int *val); @@ -134,4 +141,6 @@ json_object *get_object_from_object(json_object *obj, const char *key); int get_property(json_object *obj, const char *key, int type, bool mandatory, void *data); +int get_property_set(json_object *obj, struct config_property *properties, + unsigned int num_properties); #endif /* __CORE_COMMON_H__ */ diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index 67e3f51..8bff029 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -107,53 +107,45 @@ static void init_scenario(struct pass_scenario *scenario) static int parse_scenario(struct pass_resource *res, json_object *obj, struct pass_scenario *scenario) { - char name[BUFF_MAX] = {}; - bool support; - int target_level; - int cpuhp_min_level; - int cpuhp_max_level; json_object *temperature; - int temp_start; - int temp_end; - int threshold; - int timer_interval_ms; + bool support; bool overridable; int ret = 0; + if (!res || !obj || !scenario) + return -EINVAL; + /* Get property values */ - ret += get_property(obj, "name", DATA_TYPE_STRING, - true, (void *)name); - ret += get_property(obj, "target_level", DATA_TYPE_INT, - true, (void *)&target_level); - - ret += get_property(obj, "support", DATA_TYPE_BOOLEAN, - false, (void *)&support); - ret += get_property(obj, "timer_interval_ms", DATA_TYPE_INT, - false, (void *)&timer_interval_ms); - ret += get_property(obj, "overridable", DATA_TYPE_BOOLEAN, - false, (void *)&overridable); - - /* - property for only pmqos module */ - ret += get_property(obj, "cpuhp_min_level", DATA_TYPE_INT, - false, (void *)&cpuhp_min_level); - ret += get_property(obj, "cpuhp_max_level", DATA_TYPE_INT, - false, (void *)&cpuhp_max_level); - - /* - property for only thermal module */ + struct config_property scenario_properties[] = { + { "name", DATA_TYPE_STRING, true, (void *)&scenario->name }, + { "target_level", DATA_TYPE_INT, true, (void *)&scenario->level }, + { "support", DATA_TYPE_BOOLEAN, false, (void *)&support }, + { "overridable", DATA_TYPE_BOOLEAN, false, (void *)&overridable }, + /* - property for only pmqos module */ + { "cpuhp_min_level", DATA_TYPE_INT, false, (void *)&scenario->pmqos.min_level }, + { "cpuhp_max_level", DATA_TYPE_INT, false, (void *)&scenario->pmqos.max_level }, + /* - property for only thermal module */ + { "timer_interval_ms", DATA_TYPE_INT, false, (void *)&scenario->thermal.timer_interval }, + }; + + ret += get_property_set(obj, scenario_properties, ARRAY_SIZE(scenario_properties)); + temperature = get_object_from_object(obj, "temperature"); if (temperature) { ret += get_property(temperature, "start", DATA_TYPE_INT, - true, (void *)&temp_start); + true, (void *)&scenario->thermal.temp_start); ret += get_property(temperature, "end", DATA_TYPE_INT, - true, (void *)&temp_end); + true, (void *)&scenario->thermal.temp_end); ret += get_property(temperature, "threshold", DATA_TYPE_INT, - false, (void *)&threshold); + false, (void *)&scenario->thermal.threshold); /* if threshold is not presented, using temp_end as default */ - if (threshold < 0) - threshold = temp_end; + if (scenario->thermal.threshold < 0) + scenario->thermal.threshold = scenario->thermal.temp_end; } else { - temp_start = temp_end = threshold = 0; + scenario->thermal.temp_start = 0; + scenario->thermal.temp_end = 0; + scenario->thermal.threshold = 0; } /* Check whether the mandatory properties are included or not */ @@ -164,29 +156,19 @@ static int parse_scenario(struct pass_resource *res, json_object *obj, } /* Check the validation of property value */ - if (temp_start < 0 || temp_end < 0 || temp_end < temp_start) { + if (scenario->thermal.temp_start < 0 || scenario->thermal.temp_end < 0 + || scenario->thermal.temp_end < scenario->thermal.temp_start) { _E("Invalid temperature range in scenario section\n"); return -EINVAL; - } else if (threshold < temp_start || threshold > temp_end) { + } else if (scenario->thermal.threshold < scenario->thermal.temp_start + || scenario->thermal.threshold > scenario->thermal.temp_end) { _W("Invalid thermal 'threshold', using default as range 'end'\n"); - threshold = temp_end; + scenario->thermal.threshold = scenario->thermal.temp_end; } /* Initialize config_data from property values of confiugartion file */ - snprintf(scenario->name, NAME_MAX, "%s", name); - scenario->state = support; - scenario->level = target_level; - - /* - property for only pmqos module */ - scenario->pmqos.min_level = cpuhp_min_level; - scenario->pmqos.max_level = cpuhp_max_level; - - /* - property for only thermal module */ - scenario->thermal.temp_start = temp_start; - scenario->thermal.temp_end = temp_end; - scenario->thermal.threshold = threshold; - scenario->thermal.timer_interval = timer_interval_ms; - scenario->thermal.overridable = overridable; + scenario->state = !!support; + scenario->thermal.overridable = !!overridable; return 0; } @@ -259,93 +241,49 @@ static int parse_level(struct pass_resource *res, json_object *obj, struct pass_level *target_level, int level_idx) { int level; - int minimum_frequency_khz; - int maximum_frequency_khz; - int number_of_minimum_cpu; - int number_of_maximum_cpu; - int num_down_cond; - int num_down_cond_freq; - int num_down_cond_nr_running; - int num_down_cond_busy_cpu; - int num_up_cond; - int num_up_cond_freq; - int num_up_cond_nr_running; - int num_up_cond_busy_cpu; - int num_right_cond; - int num_right_cond_freq; - int num_right_cond_nr_running; - int num_right_cond_busy_cpu; - int num_left_cond; - int num_left_cond_freq; - int num_left_cond_nr_running; - int num_left_cond_busy_cpu; - int governor_timeout_ms; - int fault_around_bytes; - int cooling_device_state; - int charging_status; - int charging_current_uA; int ret = 0; + if(!res || !obj || !target_level || level_idx < 0) + return -EINVAL; + /* Get property values */ - ret += get_property(obj, "level", DATA_TYPE_INT, - true, (void *)&level); - ret += get_property(obj, "dvfs,minimum_frequency_khz", DATA_TYPE_INT, - false, (void *)&minimum_frequency_khz); - ret += get_property(obj, "dvfs,maximum_frequency_khz", DATA_TYPE_INT, - false, (void *)&maximum_frequency_khz); - ret += get_property(obj, "hotplug,number_of_minimum_cpu", DATA_TYPE_INT, - false, (void *)&number_of_minimum_cpu); - ret += get_property(obj, "hotplug,number_of_maximum_cpu", DATA_TYPE_INT, - false, (void *)&number_of_maximum_cpu); - - ret += get_property(obj, "hotplug,num_down_cond", DATA_TYPE_INT, - false, (void *)&num_down_cond); - ret += get_property(obj, "hotplug,num_down_cond_freq", DATA_TYPE_INT, - false, (void *)&num_down_cond_freq); - ret += get_property(obj, "hotplug,num_down_cond_nr_running", DATA_TYPE_INT, - false, (void *)&num_down_cond_nr_running); - ret += get_property(obj, "hotplug,num_down_cond_busy_cpu", DATA_TYPE_INT, - false, (void *)&num_down_cond_busy_cpu); - - ret += get_property(obj, "hotplug,num_up_cond", DATA_TYPE_INT, - false, (void *)&num_up_cond); - ret += get_property(obj, "hotplug,num_up_cond_freq", DATA_TYPE_INT, - false, (void *)&num_up_cond_freq); - ret += get_property(obj, "hotplug,num_up_cond_nr_running", DATA_TYPE_INT, - false, (void *)&num_up_cond_nr_running); - ret += get_property(obj, "hotplug,num_up_cond_busy_cpu", DATA_TYPE_INT, - false, (void *)&num_up_cond_busy_cpu); - - ret += get_property(obj, "hotplug,num_right_cond", DATA_TYPE_INT, - false, (void *)&num_right_cond); - ret += get_property(obj, "hotplug,num_right_cond_freq", DATA_TYPE_INT, - false, (void *)&num_right_cond_freq); - ret += get_property(obj, "hotplug,num_right_cond_nr_running", DATA_TYPE_INT, - false, (void *)&num_right_cond_nr_running); - ret += get_property(obj, "hotplug,num_right_cond_busy_cpu", DATA_TYPE_INT, - false, (void *)&num_right_cond_busy_cpu); - - ret += get_property(obj, "hotplug,num_left_cond", DATA_TYPE_INT, - false, (void *)&num_left_cond); - ret += get_property(obj, "hotplug,num_left_cond_freq", DATA_TYPE_INT, - false, (void *)&num_left_cond_freq); - ret += get_property(obj, "hotplug,num_left_cond_nr_running", DATA_TYPE_INT, - false, (void *)&num_left_cond_nr_running); - ret += get_property(obj, "hotplug,num_left_cond_busy_cpu", DATA_TYPE_INT, - false, (void *)&num_left_cond_busy_cpu); - - ret += get_property(obj, "hotplug,governor_timeout_ms", DATA_TYPE_INT, - false, (void *)&governor_timeout_ms); - - ret += get_property(obj, "memory,fault_around_bytes", DATA_TYPE_INT, - false, (void *)&fault_around_bytes); - - ret += get_property(obj, "thermal,cooling_device_state", DATA_TYPE_INT, - false, (void *)&cooling_device_state); - ret += get_property(obj, "battery,charging_status", DATA_TYPE_INT, - false, (void *)&charging_status); - ret += get_property(obj, "battery,charging_current_uA", DATA_TYPE_INT, - false, (void *)&charging_current_uA); + struct config_property level_properties[] = { + { "level", DATA_TYPE_INT, true, (void *)&level }, + { "dvfs,minimum_frequency_khz", DATA_TYPE_INT, false, (void *)&target_level->limit_min_freq }, + { "dvfs,maximum_frequency_khz", DATA_TYPE_INT, false, (void *)&target_level->limit_max_freq }, + { "hotplug,number_of_minimum_cpu", DATA_TYPE_INT, false, (void *)&target_level->limit_min_cpu }, + { "hotplug,number_of_maximum_cpu", DATA_TYPE_INT, false, (void *)&target_level->limit_max_cpu }, + + { "hotplug,num_down_cond", DATA_TYPE_INT, false, (void *)&target_level->num_down_cond }, + { "hotplug,num_down_cond_freq", DATA_TYPE_INT, false, (void *)&target_level->down_cond[0].freq }, + { "hotplug,num_down_cond_nr_running", DATA_TYPE_INT, false, (void *)&target_level->down_cond[0].nr_running }, + { "hotplug,num_down_cond_busy_cpu", DATA_TYPE_INT, false, (void *)&target_level->down_cond[0].busy_cpu }, + + { "hotplug,num_up_cond", DATA_TYPE_INT, false, (void *)&target_level->num_up_cond }, + { "hotplug,num_up_cond_freq", DATA_TYPE_INT, false, (void *)&target_level->up_cond[0].freq }, + { "hotplug,num_up_cond_nr_running", DATA_TYPE_INT, false, (void *)&target_level->up_cond[0].nr_running }, + { "hotplug,num_up_cond_busy_cpu", DATA_TYPE_INT, false, (void *)&target_level->up_cond[0].busy_cpu }, + + { "hotplug,num_right_cond", DATA_TYPE_INT, false, (void *)&target_level->num_right_cond }, + { "hotplug,num_right_cond_freq", DATA_TYPE_INT, false, (void *)&target_level->right_cond[0].freq }, + { "hotplug,num_right_cond_nr_running", DATA_TYPE_INT, false, (void *)&target_level->right_cond[0].nr_running }, + { "hotplug,num_right_cond_busy_cpu", DATA_TYPE_INT, false, (void *)&target_level->right_cond[0].busy_cpu }, + + { "hotplug,num_left_cond", DATA_TYPE_INT, false, (void *)&target_level->num_left_cond }, + { "hotplug,num_left_cond_freq", DATA_TYPE_INT, false, (void *)&target_level->left_cond[0].freq }, + { "hotplug,num_left_cond_nr_running", DATA_TYPE_INT, false, (void *)&target_level->left_cond[0].nr_running }, + { "hotplug,num_left_cond_busy_cpu", DATA_TYPE_INT, false, (void *)&target_level->left_cond[0].busy_cpu }, + + { "hotplug,governor_timeout_sec", DATA_TYPE_DOUBLE, false, (void *)&target_level->gov_timeout }, + + { "memory,fault_around_bytes", DATA_TYPE_INT, false, (void *)&target_level->fault_around_bytes }, + + { "thermal,cooling_device_state", DATA_TYPE_INT, false, (void *)&target_level->cooling_device_state }, + { "battery,charging_status", DATA_TYPE_INT, false, (void *)&target_level->charging_status }, + { "battery,charging_current_uA", DATA_TYPE_INT, false, (void *)&target_level->charging_current_uA }, + }; + + ret += get_property_set(obj, level_properties, ARRAY_SIZE(level_properties)); /* Check whether the mandatory properties are included or not */ if (ret < 0) { @@ -361,97 +299,9 @@ static int parse_level(struct pass_resource *res, json_object *obj, } /* Initialize config_data from property values of confiugartion file */ - - /* - * Properties for the following h/w resources: - * - PASS_RESOURCE_CPU_ID - * - PASS_RESOURCE_BUS_ID - * - PASS_RESOURCE_GPU_ID - */ - if (minimum_frequency_khz >= 0) - target_level->limit_min_freq = minimum_frequency_khz; - if (maximum_frequency_khz >= 0) - target_level->limit_max_freq = maximum_frequency_khz; - - /* - * Properties for the following h/w resources: - * - PASS_RESOURCE_CPU_ID - */ - if (number_of_minimum_cpu >= 0) - target_level->limit_min_cpu = number_of_minimum_cpu; - if (number_of_maximum_cpu >= 0) - target_level->limit_max_cpu = number_of_maximum_cpu; - - if (num_down_cond >= 0) - target_level->num_down_cond = num_down_cond; - if (num_down_cond_freq >= 0) - target_level->down_cond[0].freq = num_down_cond_freq; - if (num_down_cond_nr_running >= 0) - target_level->down_cond[0].nr_running = num_down_cond_nr_running; - if (num_down_cond_busy_cpu >= 0) - target_level->down_cond[0].busy_cpu = num_down_cond_busy_cpu; - - if (num_up_cond >= 0) - target_level->num_up_cond = num_up_cond; - if (num_up_cond_freq >= 0) - target_level->up_cond[0].freq = num_up_cond_freq; - if (num_up_cond_nr_running >= 0) - target_level->up_cond[0].nr_running = num_up_cond_nr_running; - if (num_up_cond_busy_cpu >= 0) - target_level->up_cond[0].busy_cpu = num_up_cond_busy_cpu; - - if (num_left_cond >= 0) - target_level->num_left_cond = num_left_cond; - if (num_left_cond_freq >= 0) - target_level->left_cond[0].freq = num_left_cond_freq; - if (num_left_cond_nr_running >= 0) - target_level->left_cond[0].nr_running = num_left_cond_nr_running; - if (num_left_cond_busy_cpu >= 0) - target_level->left_cond[0].busy_cpu = num_left_cond_busy_cpu; - - if (num_right_cond >= 0) - target_level->num_right_cond = num_right_cond; - if (num_right_cond_freq >= 0) - target_level->right_cond[0].freq = num_right_cond_freq; - if (num_right_cond_nr_running >= 0) - target_level->right_cond[0].nr_running = num_right_cond_nr_running; - if (num_right_cond_busy_cpu >= 0) - target_level->right_cond[0].busy_cpu = num_right_cond_busy_cpu; - - if (governor_timeout_ms >= 0) { - if (governor_timeout_ms < MIN_TIMEOUT_MS - || governor_timeout_ms > MAX_TIMEOUT_MS) - governor_timeout_ms = MIN_TIMEOUT_MS; - target_level->gov_timeout = governor_timeout_ms; - } - - /* - * Properties for the following h/w resources: - * - PASS_RESOURCE_MEMORY_ID - */ - if (fault_around_bytes >= 0) - target_level->fault_around_bytes = fault_around_bytes; - - /* - * Properties for the following h/w resources: - * - PASS_RESOURCE_CPU_ID - * - PASS_RESOURCE_BUS_ID - * - PASS_RESOURCE_GPU_ID - * - PASS_RESOURCE_BATTERY_ID - * - PASS_RESOURCE_NONSTANDARD_ID - */ - if (cooling_device_state >= 0) - target_level->cooling_device_state = cooling_device_state; - - /* - * Properties for the following h/w resources: - * - PASS_RESOURCE_BATTERY_ID - * - PASS_RESOURCE_NONSTANDARD_ID - */ - if (charging_status >= 0) - target_level->charging_status = charging_status; - if (charging_current_uA >= 0) - target_level->charging_current_uA = charging_current_uA; + if (target_level->gov_timeout < MIN_TIMEOUT_MS + || target_level->gov_timeout > MAX_TIMEOUT_MS) + target_level->gov_timeout = MIN_TIMEOUT_MS; return 0; } @@ -464,13 +314,19 @@ static int parse_level(struct pass_resource *res, json_object *obj, */ static int parse_pmqos(struct pass_resource *res, json_object *obj) { - int pmqos_support; json_object *pmqos_scenario_list = NULL; int num_scenarios = 0, ret = 0, i; + if (!res || !obj) + return -EINVAL; + /* Get property values */ - ret = get_property(obj, "pmqos_support", DATA_TYPE_INT, - true, (void *)&pmqos_support); + struct config_property pmqos_scenario_properties[] = { + { "pmqos_support", DATA_TYPE_INT, true, (void *)&res->pmqos.state }, + }; + + ret += get_property_set(obj, pmqos_scenario_properties, + ARRAY_SIZE(pmqos_scenario_properties)); if (json_object_object_get_ex(obj, "pmqos_scenario_list", &pmqos_scenario_list)) num_scenarios = json_object_array_length(pmqos_scenario_list); @@ -483,11 +339,10 @@ static int parse_pmqos(struct pass_resource *res, json_object *obj) } /* Check the validation of property value */ - if (pmqos_support <= 0) + if (res->pmqos.state <= 0) return 0; /* Initialize config_data from property values of confiugartion file */ - res->pmqos.state = pmqos_support; res->pmqos.scenarios = calloc(num_scenarios, sizeof(struct pass_scenario)); if (!res->pmqos.scenarios) { @@ -520,16 +375,20 @@ static int parse_pmqos(struct pass_resource *res, json_object *obj) */ static int parse_thermal(struct pass_resource *res, json_object *obj) { - int thermal_support; - int thermal_timer_interval_ms; json_object *thermal_scenario_list = NULL; int num_scenarios = 0, ret = 0, i; + if (!res || !obj) + return -EINVAL; + /* Get property values */ - ret += get_property(obj, "thermal_support",DATA_TYPE_INT, - true, (void *)&thermal_support); - ret += get_property(obj, "thermal_timer_interval_ms", DATA_TYPE_INT, - false, (void *)&thermal_timer_interval_ms); + struct config_property thermal_properties[] = { + { "thermal_support", DATA_TYPE_INT, true, (void *)&res->thermal.state }, + { "thermal_timer_interval_ms", DATA_TYPE_INT, false, (void *)&res->thermal.timer_interval }, + }; + + ret += get_property_set(obj, thermal_properties, ARRAY_SIZE(thermal_properties)); + if (json_object_object_get_ex(obj, "thermal_scenario_list", &thermal_scenario_list)) num_scenarios = json_object_array_length(thermal_scenario_list); @@ -542,13 +401,10 @@ static int parse_thermal(struct pass_resource *res, json_object *obj) } /* Check the validation of property value */ - if (thermal_support <= 0) + if (res->thermal.state <= 0) return 0; /* Initialize config_data from property values of confiugartion file */ - res->thermal.state = thermal_support; - res->thermal.timer_interval = thermal_timer_interval_ms; - res->thermal.scenarios = calloc(num_scenarios, sizeof(struct pass_scenario)); if (!res->thermal.scenarios) { @@ -581,40 +437,32 @@ static int parse_thermal(struct pass_resource *res, json_object *obj) */ static int parse_cpuhp(struct pass_resource *res, json_object *obj) { - int cpuhp_support; - int cpuhp_governor; - int cpuhp_timer_interval_ms; int cpuhp_min_level; int cpuhp_max_level; int cpuhp_init_level; - int cpuhp_cpu_threshold; - int cpuhp_up_threshold; - int cpuhp_down_threshold; json_object *cpuhp_level_list = NULL; - int cpuhp_num_levels = 0, ret = 0, i; + int ret = 0, i; + + if (!res || !obj) + return -EINVAL; /* Get property values */ - ret += get_property(obj, "cpuhp_support", DATA_TYPE_INT, - true, (void *)&cpuhp_support); - ret += get_property(obj, "cpuhp_governor", DATA_TYPE_INT, - false, (void *)&cpuhp_governor); - ret += get_property(obj, "cpuhp_timer_interval_ms", DATA_TYPE_INT, - false, (void *)&cpuhp_timer_interval_ms); - ret += get_property(obj, "cpuhp_min_level", DATA_TYPE_INT, - false, (void *)&cpuhp_min_level); - ret += get_property(obj, "cpuhp_max_level", DATA_TYPE_INT, - false, (void *)&cpuhp_max_level); - ret += get_property(obj, "cpuhp_init_level", DATA_TYPE_INT, - false, (void *)&cpuhp_init_level); - ret += get_property(obj, "cpuhp_cpu_threshold", DATA_TYPE_INT, - false, (void *)&cpuhp_cpu_threshold); - ret += get_property(obj, "cpuhp_up_threshold", DATA_TYPE_INT, - false, (void *)&cpuhp_up_threshold); - ret += get_property(obj, "cpuhp_down_threshold", DATA_TYPE_INT, - false, (void *)&cpuhp_down_threshold); + struct config_property cpuhp_properties[] = { + { "cpuhp_support", DATA_TYPE_INT, true, (void *)&res->config_data.state }, + { "cpuhp_governor", DATA_TYPE_INT, false, (void *)&res->config_data.gov_type }, + { "cpuhp_timer_interval_ms", DATA_TYPE_INT, false, (void *)&res->config_data.gov_timeout }, + { "cpuhp_min_level", DATA_TYPE_INT, false, (void *)&cpuhp_min_level }, + { "cpuhp_max_level", DATA_TYPE_INT, false, (void *)&cpuhp_max_level }, + { "cpuhp_init_level", DATA_TYPE_INT, false, (void *)&cpuhp_init_level }, + { "cpuhp_cpu_threshold", DATA_TYPE_INT, false, (void *)&res->cpuhp.pass_cpu_threshold }, + { "cpuhp_up_threshold", DATA_TYPE_INT, false, (void *)&res->cpuhp.up_threshold }, + { "cpuhp_down_threshold", DATA_TYPE_INT, false, (void *)&res->cpuhp.down_threshold }, + }; + + ret += get_property_set(obj, cpuhp_properties, ARRAY_SIZE(cpuhp_properties)); if (json_object_object_get_ex(obj, "cpuhp_level_list", &cpuhp_level_list)) - cpuhp_num_levels = json_object_array_length(cpuhp_level_list); + res->config_data.num_levels = json_object_array_length(cpuhp_level_list); /* Check whether the mandatory properties are included or not */ if (ret < 0) { @@ -624,36 +472,25 @@ static int parse_cpuhp(struct pass_resource *res, json_object *obj) } /* Check the validation of property value */ - if (cpuhp_support <= 0) { + if (res->config_data.state <= 0) { return 0; } else if (!cpuhp_level_list) { _E("Failed to get 'cpuhp_level_list' property\n"); return -EINVAL; - } else if (cpuhp_num_levels <= 0) { + } else if (res->config_data.num_levels <= 0) { _E("Must need to add at least one level in 'cpuhp_level_list' section\n"); return -EINVAL; } /* Initialize config_data from property values of confiugartion file */ - /* - core */ - res->config_data.state = cpuhp_support; - res->config_data.gov_type = cpuhp_governor; - res->config_data.gov_timeout = cpuhp_timer_interval_ms; - if (res->config_data.gov_timeout < MIN_TIMEOUT_MS - || res->config_data.gov_timeout > MAX_TIMEOUT_MS) - res->config_data.gov_timeout = MIN_TIMEOUT_MS; - - /* - properties for rescon module */ res->rescon.min_level = cpuhp_min_level; res->rescon.max_level = cpuhp_max_level; res->rescon.init_level = cpuhp_init_level; - /* - properties for cpuhp module */ - res->cpuhp.pass_cpu_threshold = cpuhp_cpu_threshold; - res->cpuhp.up_threshold = cpuhp_up_threshold; - res->cpuhp.down_threshold = cpuhp_down_threshold; + if (res->config_data.gov_timeout < MIN_TIMEOUT_MS + || res->config_data.gov_timeout > MAX_TIMEOUT_MS) + res->config_data.gov_timeout = MIN_TIMEOUT_MS; - res->config_data.num_levels = cpuhp_num_levels; res->config_data.levels = calloc(res->config_data.num_levels, sizeof(struct pass_level)); if (!res->config_data.levels) { @@ -688,15 +525,21 @@ static int parse_header(struct pass_resource *res, json_object *obj) bool support; int init_level; json_object *level_list = NULL; - int num_levels = 0, ret = 0, i; + int ret = 0, i; + + if (!res || !obj) + return -EINVAL; /* Get property values */ - ret += get_property(obj, "support", DATA_TYPE_BOOLEAN, - true, (void *)&support); - ret += get_property(obj, "init_level", DATA_TYPE_INT, - true, (void *)&init_level); + struct config_property header_properties[] = { + { "support", DATA_TYPE_BOOLEAN, true, (void *)&support }, + { "init_level", DATA_TYPE_INT, true, (void *)&init_level }, + }; + + ret += get_property_set(obj, header_properties, ARRAY_SIZE(header_properties)); + if (json_object_object_get_ex(obj, "level_list", &level_list)) - num_levels = json_object_array_length(level_list); + res->config_data.num_scenario_levels = json_object_array_length(level_list); /* Check whether the mandatory properties are included or not */ if (ret < 0) { @@ -709,21 +552,21 @@ static int parse_header(struct pass_resource *res, json_object *obj) if (!level_list) { _E("Failed to get 'level_list' property\n"); return -EINVAL; - } else if (num_levels <= 0) { + } else if (res->config_data.num_scenario_levels <= 0) { _E("Must need to add at least one level in 'level_list' section\n"); return -EINVAL; - } else if (init_level > num_levels) { + } else if (init_level > res->config_data.num_scenario_levels) { _E("Invalid 'init_level' in core section\n"); return -EINVAL; } + /* Initialize config_data from property values of confiugartion file */ + res->config_data.state = !!support; + if (init_level < 0) init_level = 0; - - /* Initialize config_data from property values of confiugartion file */ - res->config_data.state = support; - res->config_data.num_scenario_levels = num_levels; res->rescon.init_scenario_level = init_level; + res->config_data.scenario_levels = calloc( res->config_data.num_scenario_levels, sizeof(struct pass_level)); @@ -799,37 +642,27 @@ static int parse_resource_data(struct pass *pass, int id, json_object *obj) struct pass_resource_config_data *config_data = &(pass->res[id].config_data); char device_type[BUFF_MAX] = {}; - char device_name[BUFF_MAX] = {}; - char thermal_device_name[BUFF_MAX] = {}; - char cooling_device_name[BUFF_MAX] = {}; - char device_config_path[BUFF_MAX] = {}; - char cpu_load_path[BUFF_MAX] = {}; - int number_of_cpus; - int first_cpu; - int thermal_priority; int ret = 0; + if(!pass || id < 0 || !obj) + return -EINVAL; + /* Get property values */ - ret += get_property(obj, "device_type", DATA_TYPE_STRING, - true, (void *)device_type); - ret += get_property(obj, "device_name", DATA_TYPE_STRING, - true, (void *)device_name); - ret += get_property(obj, "device_config_path", DATA_TYPE_STRING, - true, (void *)device_config_path); - - ret += get_property(obj, "thermal_device_name", DATA_TYPE_STRING, - false, (void *)thermal_device_name); - ret += get_property(obj, "cooling_device_name", DATA_TYPE_STRING, - false, (void *)cooling_device_name); - ret += get_property(obj, "thermal_priority", DATA_TYPE_INT, - false, (void *)&thermal_priority); - - ret += get_property(obj, "cpu,cpu_load_path", DATA_TYPE_STRING, - false, (void *)cpu_load_path); - ret += get_property(obj, "cpu,number_of_cpus", DATA_TYPE_INT, - false, (void *)&number_of_cpus); - ret += get_property(obj, "cpu,first_cpu", DATA_TYPE_INT, - false, (void *)&first_cpu); + struct config_property resource_properties[] = { + { "device_type", DATA_TYPE_STRING, true, (void *)device_type }, + { "device_name", DATA_TYPE_STRING, true, (void *)config_data->res_name }, + { "device_config_path", DATA_TYPE_STRING, true, (void *)config_data->path_conf_file }, + + { "thermal_device_name", DATA_TYPE_STRING, false, (void *)config_data->res_thermal_name }, + { "cooling_device_name", DATA_TYPE_STRING, false, (void *)config_data->res_cooling_name }, + { "thermal_priority", DATA_TYPE_INT, false, (void *)&config_data->res_thermal_priority }, + + { "cpu,cpu_load_path", DATA_TYPE_STRING, false, (void *)config_data->path_load_table }, + { "cpu,number_of_cpus", DATA_TYPE_INT, false, (void *)&config_data->num_cpus }, + { "cpu,first_cpu", DATA_TYPE_INT, false, (void *)&config_data->cpu }, + }; + + ret += get_property_set(obj, resource_properties, ARRAY_SIZE(resource_properties)); /* Check whether the mandatory properties are included or not */ if (ret < 0) { @@ -838,14 +671,14 @@ static int parse_resource_data(struct pass *pass, int id, json_object *obj) } /* Check the validation of property value */ - if (thermal_priority < 0) - thermal_priority = INT_MAX; + if (config_data->res_thermal_priority < 0) + config_data->res_thermal_priority = INT_MAX; switch (config_data->res_type) { case PASS_RESOURCE_CPU_ID: - if (number_of_cpus < 0 || first_cpu < 0) { + if (config_data->num_cpus < 0 || config_data->cpu < 0) { _E("Invalid 'number_of_cpus'(%d) or 'first_cpu'(%d) property\n", - number_of_cpus, first_cpu); + config_data->num_cpus, config_data->cpu); return -EINVAL; } break; @@ -853,7 +686,8 @@ static int parse_resource_data(struct pass *pass, int id, json_object *obj) break; } - number_of_cpus = number_of_cpus < 0 ? 0 : number_of_cpus; + if (config_data->num_cpus < 0) + config_data->num_cpus = 0; /* Initialize config_data from property values of confiugartion file */ if (!strncmp(device_type, PASS_RESOURCE_CPU_NAME, strlen(device_type))) @@ -879,16 +713,6 @@ static int parse_resource_data(struct pass *pass, int id, json_object *obj) return -EINVAL; } - snprintf(config_data->res_name, BUFF_MAX, "%s", device_name); - snprintf(config_data->res_thermal_name, BUFF_MAX, "%s", thermal_device_name); - snprintf(config_data->res_cooling_name, BUFF_MAX, "%s", cooling_device_name); - config_data->res_thermal_priority = thermal_priority; - - snprintf(config_data->path_conf_file, BUFF_MAX, "%s", device_config_path); - snprintf(config_data->path_load_table, BUFF_MAX, "%s", cpu_load_path); - config_data->num_cpus = number_of_cpus; - config_data->cpu = first_cpu; - return 0; } @@ -906,11 +730,17 @@ static int parse_resource(struct pass *pass, json_object *obj) char board_name_path[BUFF_MAX] = {}; int num_resources = 0, ret = 0, i; + if (!pass || !obj) + return -EINVAL; + /* Get property values */ - ret += get_property(obj, "board_name", DATA_TYPE_STRING, - false, (void *)board_name); - ret += get_property(obj, "board_name_path", DATA_TYPE_STRING, - false, (void *)board_name_path); + struct config_property board_properties[] = { + { "board_name", DATA_TYPE_STRING, false, (void *)board_name }, + { "board_name_path", DATA_TYPE_STRING, false, (void *)board_name_path }, + }; + + ret += get_property_set(obj, board_properties, ARRAY_SIZE(board_properties)); + if (json_object_object_get_ex(obj, "device_list", &device_list)) num_resources = json_object_array_length(device_list); diff --git a/src/pmqos/pmqos-parser.c b/src/pmqos/pmqos-parser.c index 845f441..6c16c4f 100644 --- a/src/pmqos/pmqos-parser.c +++ b/src/pmqos/pmqos-parser.c @@ -55,14 +55,15 @@ static int pmqos_parse_scenario(json_object *obj, struct scenario *scenario) int max_duration_ms; bool support; int ret = 0; + struct config_property pmqos_scenario_properties[] = { + { "name", DATA_TYPE_STRING, true, (void *)name }, + { "support", DATA_TYPE_BOOLEAN, false, (void *)&support }, + { "max_duration_ms", DATA_TYPE_INT, false, (void *)&max_duration_ms }, + }; /* Get property values */ - ret += get_property(obj, "name", DATA_TYPE_STRING, true, - (void *)name); - ret += get_property(obj, "support", DATA_TYPE_BOOLEAN, false, - (void *)&support); - ret += get_property(obj, "max_duration_ms", DATA_TYPE_INT, false, - (void *)&max_duration_ms); + ret += get_property_set(obj, pmqos_scenario_properties, + ARRAY_SIZE(pmqos_scenario_properties)); /* Check whether the mandatory properties are included or not */ if (ret < 0) { @@ -90,11 +91,14 @@ static int pmqos_load_config(json_object *obj, struct pmqos_scenario *scenarios) { bool pmqos_support; json_object *pmqos_scenario_list = NULL; - int num_scenarios = 0, ret, i; + int num_scenarios = 0, ret = 0, i; + struct config_property pmqos_header_properties[] = { + {"pmqos_support", DATA_TYPE_BOOLEAN, false, (void *)&pmqos_support }, + }; /* Get property values */ - ret = get_property(obj, "pmqos_support", DATA_TYPE_BOOLEAN, false, - (void *)&pmqos_support); + ret += get_property_set(obj, pmqos_header_properties, + ARRAY_SIZE(pmqos_header_properties)); if (json_object_object_get_ex(obj, "pmqos_scenario_list", &pmqos_scenario_list)) num_scenarios = json_object_array_length(pmqos_scenario_list); diff --git a/src/thermal/thermal-parser.c b/src/thermal/thermal-parser.c index 09edc5b..781a31d 100644 --- a/src/thermal/thermal-parser.c +++ b/src/thermal/thermal-parser.c @@ -52,12 +52,14 @@ static int thermal_parse_scenario(json_object *obj, struct scenario *scenario) char name[BUFF_MAX] = {}; bool support; int ret = 0; + struct config_property thermal_scenario_properties[] = { + { "name", DATA_TYPE_STRING, true, (void *)name }, + {"support", DATA_TYPE_BOOLEAN, false, (void *)&support }, + }; /* Get property values */ - ret += get_property(obj, "name", DATA_TYPE_STRING, true, - (void *)name); - ret += get_property(obj, "support", DATA_TYPE_BOOLEAN, false, - (void *)&support); + ret += get_property_set(obj, thermal_scenario_properties, + ARRAY_SIZE(thermal_scenario_properties)); /* Check whether the mandatory properties are included or not */ if (ret < 0) { @@ -84,10 +86,13 @@ static int thermal_load_config(json_object *obj, struct thermal_scenario *scenar bool thermal_support; json_object *thermal_scenario_list = NULL; int num_scenarios = 0, ret = 0, i; + struct config_property thermal_header_properties[] = { + {"thermal_support", DATA_TYPE_BOOLEAN, false, (void *)&thermal_support }, + }; /* Get property values */ - ret = get_property(obj, "thermal_support", DATA_TYPE_BOOLEAN, false, - (void *)&thermal_support); + ret += get_property_set(obj, thermal_header_properties, + ARRAY_SIZE(thermal_header_properties)); if (json_object_object_get_ex(obj, "thermal_scenario_list", &thermal_scenario_list)) num_scenarios = json_object_array_length(thermal_scenario_list); diff --git a/src/util/common.c b/src/util/common.c index 055c169..d58c2e0 100644 --- a/src/util/common.c +++ b/src/util/common.c @@ -16,7 +16,6 @@ * limitations under the License. */ - #include #include #include @@ -193,3 +192,17 @@ int get_property(json_object *obj, const char *key, return ret; } + +int get_property_set(json_object *obj, struct config_property *properties, + unsigned int num_properties) +{ + int i, ret = 0; + + if (!obj || !properties || num_properties >= INT_MAX) + return 0; + + for (i = 0; i < num_properties; i++) + ret += get_property(obj, properties[i].name, properties[i].type, + properties[i].mandatory, properties[i].value); + return ret; +} -- 2.7.4 From 152061441eb8353b4569ad2db473571b1eb43b10 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Tue, 13 Sep 2022 17:08:54 +0900 Subject: [PATCH 07/16] monitor: providing ip socket server when debug mode is enabled To protect against attacks using network socket vulnerabilities, open the TCP/IP based socket interface only when debug mode is enabled. Change-Id: Ie8b3d392b783c63225f30f6dc611524ccb7c6a09 Signed-off-by: Dongwoo Lee --- include/monitor/monitor.h | 3 +++ src/monitor/monitor.c | 11 +++++++++++ src/monitor/request-handler.c | 35 +++++++++++++++++++++++++---------- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index 38b4e62..25413f9 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -41,6 +41,8 @@ struct monitor { }; struct monitor *monitor_get(void); +gboolean monitor_get_debug_mode(void); +void monitor_set_debug_mode(gboolean on); int monitor_thread_init(struct monitor *monitor); void monitor_thread_exit(struct monitor *monitor); @@ -55,4 +57,5 @@ void monitor_command_exit(struct monitor_command *cmd); int request_server_init(void); void request_server_exit(void); + #endif diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index dd0e818..b412f93 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -24,12 +24,23 @@ #include static struct monitor g_monitor; +static gboolean g_debug_mode; struct monitor *monitor_get(void) { return &g_monitor; } +gboolean monitor_get_debug_mode(void) +{ + return g_debug_mode; +} + +void monitor_set_debug_mode(gboolean on) +{ + g_debug_mode = on; +} + static int monitor_setup(void *data, void *user_data) { int ret; diff --git a/src/monitor/request-handler.c b/src/monitor/request-handler.c index 324d0fc..5d2d8bd 100644 --- a/src/monitor/request-handler.c +++ b/src/monitor/request-handler.c @@ -1177,7 +1177,7 @@ static int request_server_func(void *ctx, void **result) struct sockaddr_in ip_address; struct timeval wait; int server_unix_socket; - int server_ip_socket; + int server_ip_socket = 0; int unix_addrlen; int ip_addrlen; int ret; @@ -1193,15 +1193,11 @@ static int request_server_func(void *ctx, void **result) if (ret < 0) goto error_out; - /* initialize TCP socket */ - ret = init_ip_socket(&server_ip_socket, &ip_address, &ip_addrlen); - if (ret < 0) - goto error_out_close_server_unix_socket; - while (g_request_server_run) { FD_ZERO(&fds); FD_SET(server_unix_socket, &fds); - FD_SET(server_ip_socket, &fds); + if (server_ip_socket) + FD_SET(server_ip_socket, &fds); wait.tv_sec = 1; wait.tv_usec = 0; @@ -1224,7 +1220,7 @@ static int request_server_func(void *ctx, void **result) create_request_client(new_socket); } - if (FD_ISSET(server_ip_socket, &fds)) { + if (server_ip_socket && FD_ISSET(server_ip_socket, &fds)) { int new_socket = accept(server_ip_socket, (struct sockaddr *)&ip_address, (socklen_t *)&ip_addrlen); @@ -1235,15 +1231,34 @@ static int request_server_func(void *ctx, void **result) create_request_client(new_socket); } + + if (!server_ip_socket && (monitor_get_debug_mode() == TRUE)) { + /* + * FIXME: + * server_ip_socket activation can be deferred since it + * will be done after timeout of select is expired. So, + * when the timeout is extended, this activation should be + * done by other ways in order to prevent unacceptable + * delays. + */ + ret = init_ip_socket(&server_ip_socket, &ip_address, &ip_addrlen); + if (ret < 0) + goto error_out_close_server_unix_socket; + } else if (server_ip_socket && (monitor_get_debug_mode() == FALSE)) { + close(server_ip_socket); + server_ip_socket = 0; + } } - close(server_ip_socket); close(server_unix_socket); + if (server_ip_socket) + close(server_ip_socket); return THREAD_RETURN_DONE; error_out_close: - close(server_ip_socket); + if (server_ip_socket) + close(server_ip_socket); error_out_close_server_unix_socket: close(server_unix_socket); error_out: -- 2.7.4 From d3df00fdb645b074c86c2888270eb660a39f947e Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Tue, 13 Sep 2022 15:00:28 +0900 Subject: [PATCH 08/16] monitor: Add dbus interface for turning on/off debug mode In order to provide the way to change debug mode dynamically, the new dbus interface is now applied as below: - dbus path: /Org/Tizen/System/Pass/Monitor - dbus interface: org.tizen.system.pass.monitor - dbus method: org.tizen.system.pass.monitor.SetDebug - parameter: boolean value for representing debug mode is enabled Change-Id: I431182809d23b1b7ee795c392bdeb84f9ef741bf Signed-off-by: Dongwoo Lee --- CMakeLists.txt | 9 ++++++ include/util/gdbus-definition.h | 4 +++ include/util/gdbus-util.h | 3 ++ scripts/monitor-dbus.xml | 9 ++++++ scripts/pass.conf | 2 ++ src/monitor/monitor.c | 63 ++++++++++++++++++++++++++++++++++++++++- src/util/gdbus-util.c | 10 +++++++ 7 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 scripts/monitor-dbus.xml diff --git a/CMakeLists.txt b/CMakeLists.txt index 18f595b..8b44fa6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -55,6 +55,8 @@ SET(SRCS src/monitor/monitor-thread.c src/monitor/monitor-command.c src/monitor/request-handler.c + #Generated by a custom command 'gdbus-codegen' below + src/monitor/monitor-dbus-stub.c ) INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}) @@ -115,6 +117,13 @@ ADD_CUSTOM_COMMAND( --output-directory src/thermal/ scripts/thermal-dbus.xml COMMENT "Generating GDBus stub for thermal interface") +ADD_CUSTOM_COMMAND( + OUTPUT src/monitor/monitor-dbus-stub.c + COMMAND gdbus-codegen --interface-prefix org.tizen. + --generate-c-code monitor-dbus-stub + --output-directory src/monitor/ + scripts/monitor-dbus.xml + COMMENT "Generating GDBus stub for monitor interface") ADD_EXECUTABLE(${PROJECT_NAME} ${SRCS}) TARGET_LINK_LIBRARIES(${PROJECT_NAME} ${pkgs_LDFLAGS} -ldl -lm) diff --git a/include/util/gdbus-definition.h b/include/util/gdbus-definition.h index 5072341..86ea5cb 100644 --- a/include/util/gdbus-definition.h +++ b/include/util/gdbus-definition.h @@ -42,6 +42,10 @@ typedef enum { #define DBUS_PMQOS_I_ULTRAPOWERSAVING_HANDLER "handle_ultra_power_saving" #define DBUS_PMQOS_I_SET_SCENARIO_HANDLER "handle_set_scenario" +#define DBUS_MONITOR_INTERFACE "org.tizen.system.pass.monitor" +#define DBUS_MONITOR_PATH "/Org/Tizen/System/Pass/Monitor" +#define DBUS_MONITOR_I_SET_DEBUG_HANDLER "handle_set_debug" + #define DBUS_THERMAL_BUS_NAME "org.tizen.system.thermal" #define DBUS_THERMAL_INTERFACE "org.tizen.system.thermal" #define DBUS_THERMAL_PATH "/Org/Tizen/System/Thermal" diff --git a/include/util/gdbus-util.h b/include/util/gdbus-util.h index a5dbc44..dcb1bd5 100644 --- a/include/util/gdbus-util.h +++ b/include/util/gdbus-util.h @@ -26,6 +26,7 @@ #include "pass/pass-dbus-stub.h" #include "pmqos/pmqos-dbus-stub.h" #include "thermal/thermal-dbus-stub.h" +#include "monitor/monitor-dbus-stub.h" #include "gdbus-definition.h" @@ -54,6 +55,8 @@ SystemPassCore *gdbus_get_instance_core(void); void gdbus_put_instance_core(SystemPassCore **instance); SystemPassPmqos *gdbus_get_instance_pmqos(void); void gdbus_put_instance_pmqos(SystemPassPmqos **instance); +SystemPassMonitor *gdbus_get_instance_monitor(void); +void gdbus_put_instance_monitor(SystemPassMonitor **instance); SystemThermal *gdbus_get_instance_thermal(void); void gdbus_put_instance_thermal(SystemThermal **instance); diff --git a/scripts/monitor-dbus.xml b/scripts/monitor-dbus.xml new file mode 100644 index 0000000..39606ef --- /dev/null +++ b/scripts/monitor-dbus.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/scripts/pass.conf b/scripts/pass.conf index abfdfdd..a2f7e72 100644 --- a/scripts/pass.conf +++ b/scripts/pass.conf @@ -6,6 +6,8 @@ send_interface="org.tizen.system.pass.core"/> + diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index b412f93..0fc9a1a 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -21,11 +21,14 @@ #include #include #include +#include #include static struct monitor g_monitor; static gboolean g_debug_mode; +static SystemPassMonitor *g_gdbus_instance; + struct monitor *monitor_get(void) { return &g_monitor; @@ -41,6 +44,30 @@ void monitor_set_debug_mode(gboolean on) g_debug_mode = on; } +static gboolean dbus_cb_monitor_set_debug(SystemPassMonitor *obj, + GDBusMethodInvocation *invoc, + gboolean on, + gpointer user_data) +{ + if (monitor_get_debug_mode() != on) { + monitor_set_debug_mode(on); + _I("PASS Resource Monitor debug mode is set to %s", on ? "on" : "off"); + } + + g_dbus_method_invocation_return_value(invoc, g_variant_new("(i)", 0)); + + return TRUE; +} + +static struct gdbus_signal_info g_gdbus_signal_infos[] = { + { + .handler = DBUS_MONITOR_I_SET_DEBUG_HANDLER, + .cb = G_CALLBACK(dbus_cb_monitor_set_debug), + .cb_data = NULL, + .ret_id = 0, + }, +}; + static int monitor_setup(void *data, void *user_data) { int ret; @@ -80,6 +107,31 @@ static int monitor_probe(void *data) { int ret = 0; + g_gdbus_instance = gdbus_get_instance_monitor(); + if (g_gdbus_instance == NULL) { + _E("failed to get instance for the %s interface\n", + DBUS_MONITOR_INTERFACE); + return -EINVAL; + } + + ret = gdbus_connect_signal(g_gdbus_instance, + ARRAY_SIZE(g_gdbus_signal_infos), g_gdbus_signal_infos); + if (ret < 0) { + _E("failed to register dbus methods\n"); + ret = -EINVAL; + goto out; + } + + ret = gdbus_export_interface(PASS_DBUS_CORE, + g_gdbus_instance, DBUS_MONITOR_PATH); + if (ret < 0) { + _E("cannot export the dbus interface '%s' at the object path '%s'\n", + DBUS_MONITOR_INTERFACE, + DBUS_MONITOR_PATH); + ret = -EINVAL; + goto out_disconnect; + } + /* * Register a notifier for the booting-done event. The actual * initialization of the daemon is performed by this notifier after @@ -90,10 +142,19 @@ static int monitor_probe(void *data) if (ret < 0) { _E("cannot register a callback function \ for the booting-done event (%d)\n", ret); - return ret; + goto out_disconnect; } return 0; + +out_disconnect: + gdbus_disconnect_signal(g_gdbus_instance, + ARRAY_SIZE(g_gdbus_signal_infos), g_gdbus_signal_infos); +out: + gdbus_put_instance_monitor(&g_gdbus_instance); + + return ret; + } static const struct device_ops monitor_device_ops = { diff --git a/src/util/gdbus-util.c b/src/util/gdbus-util.c index 17c25c2..b0db944 100644 --- a/src/util/gdbus-util.c +++ b/src/util/gdbus-util.c @@ -193,6 +193,16 @@ void gdbus_put_instance_pmqos(SystemPassPmqos **instance) put_instance((gpointer *)instance); } +SystemPassMonitor *gdbus_get_instance_monitor(void) +{ + return system_pass_monitor_skeleton_new(); +} + +void gdbus_put_instance_monitor(SystemPassMonitor **instance) +{ + put_instance((gpointer *)instance); +} + SystemThermal *gdbus_get_instance_thermal(void) { return system_thermal_skeleton_new(); -- 2.7.4 From 77fb6033be76b54f2c47c75c7fdaf82855ac4a20 Mon Sep 17 00:00:00 2001 From: Sung-hun Kim Date: Thu, 15 Sep 2022 14:56:02 +0900 Subject: [PATCH 09/16] systemd: Fix socket file path Change from a invalid socket file path /run/.pass.socket to a valid socket file path /run/.pass-resource-monitor.socket Change-Id: I197c24710ba76ffc5b8fe195129d6057cd0e2035 Signed-off-by: Sung-hun Kim --- systemd/pass-resource-monitor.socket.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/systemd/pass-resource-monitor.socket.in b/systemd/pass-resource-monitor.socket.in index 406f96f..88f021a 100644 --- a/systemd/pass-resource-monitor.socket.in +++ b/systemd/pass-resource-monitor.socket.in @@ -4,7 +4,7 @@ Description=Pass resource-monitor socket [Socket] SocketUser=system_fw SocketGroup=system_fw -ListenStream=/run/.pass.socket +ListenStream=/run/.pass-resource-monitor.socket SocketMode=0777 SmackLabelIPIn=* SmackLabelIPOut=@ -- 2.7.4 From c709c78da0af3807289cd74fe148572f8d39ff2c Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Thu, 15 Sep 2022 11:15:27 +0900 Subject: [PATCH 10/16] resource: Pass error result directly from resource creation Previously, it was lost by returning NULL instead of the error code that occurred during the resource creation process, but now the error code is passed directly so that the caller can know the exact reason. Change-Id: Iffc25c266186195d23ef0f61bba989ff5e36b4d7 Signed-off-by: Dongwoo Lee --- include/util/resource.h | 2 +- src/monitor/request-handler.c | 8 ++++---- src/util/resource.c | 14 ++++++++------ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/include/util/resource.h b/include/util/resource.h index 028e7c7..0855c80 100644 --- a/include/util/resource.h +++ b/include/util/resource.h @@ -151,7 +151,7 @@ int add_resource_device(struct resource_device *resource_device); void remove_resource_device(struct resource_device *resource_device); /* Create/delete resource instance */ -struct resource *create_resource(int resource_type); +int create_resource(struct resource **res, int resource_type); void delete_resource(struct resource *resource); /* Set flag of the resource to given flag mask */ diff --git a/src/monitor/request-handler.c b/src/monitor/request-handler.c index 5d2d8bd..f5a2138 100644 --- a/src/monitor/request-handler.c +++ b/src/monitor/request-handler.c @@ -88,7 +88,7 @@ get_resource_by_id(struct request_client *client, int resource_id) static int handle_request_create_resource(struct request_client *client, char *args) { struct resource *res; - int resource_type; + int resource_type, ret; if (!client || !args) { _E("Invalid parameter\n"); @@ -101,10 +101,10 @@ static int handle_request_create_resource(struct request_client *client, char *a */ resource_type = atoi(args); - res = create_resource(resource_type); - if (!res) { + ret = create_resource(&res, resource_type); + if (ret < 0) { _E("failed to create resource, res:type(%d)\n", resource_type); - return -EINVAL; + return ret; } register_resource_to_client(client, res); diff --git a/src/util/resource.c b/src/util/resource.c index cfb9e94..2def817 100644 --- a/src/util/resource.c +++ b/src/util/resource.c @@ -227,7 +227,7 @@ void init_resource_id(void) g_resource_id = clear_sign_bit(val); } -struct resource *create_resource(int resource_type) +int create_resource(struct resource **res, int resource_type) { const struct resource_driver *driver = NULL; struct resource *resource = NULL; @@ -237,12 +237,12 @@ struct resource *create_resource(int resource_type) if (!driver) { _E("failed to find resource driver, res:type(%d)\n", resource_type); - return NULL; + return -EINVAL; } resource = calloc(1, sizeof(*resource)); if (!resource) - return NULL; + return -ENOMEM; resource->id = clear_sign_bit( (unsigned int)__sync_fetch_and_add(&g_resource_id, 1)); @@ -255,7 +255,7 @@ struct resource *create_resource(int resource_type) sizeof(*resource->attrs_value)); if (!resource->attrs_value) { do_delete_resource(resource); - return NULL; + return -ENOMEM; } for (i = 0; i < resource->num_attrs; i++) @@ -271,11 +271,13 @@ struct resource *create_resource(int resource_type) _E("failed to initialize resource driver, res:type(%s)id(%d)\n", resource->name, resource->id); do_delete_resource(resource); - return NULL; + return ret; } } - return resource; + *res = resource; + + return 0; } int set_resource_flag(struct resource *resource, u_int64_t flag_mask) -- 2.7.4 From e561e82bd928cb24c0e44ff9f12ad5882476acc0 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Thu, 15 Sep 2022 19:51:18 +0900 Subject: [PATCH 11/16] resource: Fix missing NULL check Change-Id: I8a9f2d5139ad6298788d373485d24110e9d85979 Signed-off-by: Dongwoo Lee --- src/util/resource.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/resource.c b/src/util/resource.c index 2def817..39ccc68 100644 --- a/src/util/resource.c +++ b/src/util/resource.c @@ -233,6 +233,9 @@ int create_resource(struct resource **res, int resource_type) struct resource *resource = NULL; int i, ret; + if (!res) + return -EINVAL; + driver = find_resource_driver(resource_type); if (!driver) { _E("failed to find resource driver, res:type(%d)\n", -- 2.7.4 From 39b88d4e37ffe12b775f7932a35cb3819486c717 Mon Sep 17 00:00:00 2001 From: Sung-hun Kim Date: Mon, 19 Sep 2022 13:42:34 +0900 Subject: [PATCH 12/16] resource-monitor-tests: Prevent possible memory leaks Change-Id: I9ff9c4bd7ef970ca1541c756fd1b11c5506980f9 Signed-off-by: Sung-hun Kim --- tests/integration-test/resource-monitor-tests.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/integration-test/resource-monitor-tests.cpp b/tests/integration-test/resource-monitor-tests.cpp index f588989..f10237a 100644 --- a/tests/integration-test/resource-monitor-tests.cpp +++ b/tests/integration-test/resource-monitor-tests.cpp @@ -785,6 +785,7 @@ static int __pass_resource_monitor_get_value(int mon_id, int res_id, int num_att int32_t *array = NULL; ret = pass_resource_monitor_get_array_int(mon_id, res_id, attrs[i].attr_id, &array, &length); + free(array); EXPECT_EQ(ret, 0); break; } @@ -793,6 +794,7 @@ static int __pass_resource_monitor_get_value(int mon_id, int res_id, int num_att int64_t *array = NULL;; ret = pass_resource_monitor_get_array_int64(mon_id, res_id, attrs[i].attr_id, &array, &length); + free(array); EXPECT_EQ(ret, 0); break; } @@ -801,6 +803,7 @@ static int __pass_resource_monitor_get_value(int mon_id, int res_id, int num_att u_int32_t *array = NULL; ret = pass_resource_monitor_get_array_uint(mon_id, res_id, attrs[i].attr_id, &array, &length); + free(array); EXPECT_EQ(ret, 0); break; } @@ -809,6 +812,7 @@ static int __pass_resource_monitor_get_value(int mon_id, int res_id, int num_att u_int64_t *array = NULL; ret = pass_resource_monitor_get_array_uint64(mon_id, res_id, attrs[i].attr_id, &array, &length); + free(array); EXPECT_EQ(ret, 0); break; } @@ -817,6 +821,7 @@ static int __pass_resource_monitor_get_value(int mon_id, int res_id, int num_att double *array = NULL; ret = pass_resource_monitor_get_array_double(mon_id, res_id, attrs[i].attr_id, &array, &length); + free(array); EXPECT_EQ(ret, 0); break; } @@ -825,6 +830,9 @@ static int __pass_resource_monitor_get_value(int mon_id, int res_id, int num_att char **array = NULL; ret = pass_resource_monitor_get_array_string(mon_id, res_id, attrs[i].attr_id, &array, &length); + for (int i = 0; i < length; i++) + g_free(array[i]); + free(array); EXPECT_EQ(ret, 0); break; } -- 2.7.4 From 5641d5d99c307ab05eb93a2697f025866754e6ce Mon Sep 17 00:00:00 2001 From: Sung-hun Kim Date: Mon, 19 Sep 2022 19:51:54 +0900 Subject: [PATCH 13/16] monitor: Use the systemd Unix socket Systemd creates an Unix socket when the system is booted on by using existing configuration files in /usr/lib/systemd. Previously, the resource monitor always creates a new Unix socket whether the socket exists or not. From now on, the resource monitor checks that the Unix socket is existed on the given path. If so, it uses the Unix socket instead of open a newer one. By doing so, the permission of the Unix socket can follow the given configuration denoted in the Systemd socket configuration file. Change-Id: I1eab46cd238340cc9700e1a2d52481270c06b8b1 Signed-off-by: Sung-hun Kim --- src/monitor/request-handler.c | 36 ++++++++++++++++++++++++++++----- systemd/pass-resource-monitor.socket.in | 4 ++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/monitor/request-handler.c b/src/monitor/request-handler.c index f5a2138..3ffc258 100644 --- a/src/monitor/request-handler.c +++ b/src/monitor/request-handler.c @@ -39,6 +39,9 @@ #include #include #include +#include + +#include #define PENDING_MAX 3 #define REQUEST_SERVER_PORT 10001 @@ -1083,10 +1086,30 @@ static int init_unix_socket(int *sock, struct sockaddr_un *address, int *addrlen { const char *server_unix_socket_path = "/run/.pass-resource-monitor.socket"; int opt = true; + int n = sd_listen_fds(0); + int fd; + int file_mode; if (!sock || !address || !addrlen) return -EINVAL; + bzero(address, sizeof(*address)); + address->sun_family = AF_UNIX; + strncpy(address->sun_path, server_unix_socket_path, sizeof(address->sun_path)); + address->sun_path[sizeof(address->sun_path) - 1] = '\0'; + + /* use the existing systemd unix socket */ + if (n > 0) { + for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) { + if (sd_is_socket_unix(fd, SOCK_STREAM, 1, server_unix_socket_path, 0) > 0) { + *sock = fd; + *addrlen = sizeof(*address); + return 0; + } + } + } + + /* make a new unix socket */ *sock = socket(AF_UNIX, SOCK_STREAM, 0); if (*sock < 0) { _E("Failed to initialize socket"); @@ -1099,11 +1122,6 @@ static int init_unix_socket(int *sock, struct sockaddr_un *address, int *addrlen goto error_out_close; } - bzero(address, sizeof(*address)); - address->sun_family = AF_UNIX; - strncpy(address->sun_path, server_unix_socket_path, sizeof(address->sun_path)); - address->sun_path[sizeof(address->sun_path) - 1] = '\0'; - if (!access(server_unix_socket_path, F_OK)) unlink(server_unix_socket_path); @@ -1112,6 +1130,12 @@ static int init_unix_socket(int *sock, struct sockaddr_un *address, int *addrlen goto error_out_close; } + file_mode = (S_IRWXU | S_IRWXG | S_IRWXO); + if (chmod(server_unix_socket_path, file_mode) < 0) { + _E("Failed to change the file mode of %s", server_unix_socket_path); + goto error_out_close; + } + if (listen(*sock, PENDING_MAX) < 0) { _E("Failed to begin listenning"); goto error_out_close; @@ -1124,6 +1148,8 @@ static int init_unix_socket(int *sock, struct sockaddr_un *address, int *addrlen error_out_close: close(*sock); error_out: + bzero(address, sizeof(*address)); + return -EIO; } diff --git a/systemd/pass-resource-monitor.socket.in b/systemd/pass-resource-monitor.socket.in index 88f021a..533a720 100644 --- a/systemd/pass-resource-monitor.socket.in +++ b/systemd/pass-resource-monitor.socket.in @@ -8,3 +8,7 @@ ListenStream=/run/.pass-resource-monitor.socket SocketMode=0777 SmackLabelIPIn=* SmackLabelIPOut=@ +Service=pass.service + +[Install] +WantedBy=sockets.target -- 2.7.4 From 588af681c5c6e7d194b3c479c8e98d2d7b1d17ea Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Tue, 20 Sep 2022 12:21:01 +0900 Subject: [PATCH 14/16] pass: parser: Check NULL before reference Change-Id: Ifae8c9ae87a9f79bae5b79450b504d3025ecee24 Signed-off-by: Dongwoo Lee --- src/pass/pass-parser.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pass/pass-parser.c b/src/pass/pass-parser.c index 8bff029..4e16a43 100644 --- a/src/pass/pass-parser.c +++ b/src/pass/pass-parser.c @@ -639,14 +639,15 @@ static int parse_each_resource(struct pass_resource *res, json_object *obj) */ static int parse_resource_data(struct pass *pass, int id, json_object *obj) { - struct pass_resource_config_data *config_data - = &(pass->res[id].config_data); + struct pass_resource_config_data *config_data; char device_type[BUFF_MAX] = {}; int ret = 0; if(!pass || id < 0 || !obj) return -EINVAL; + config_data = &(pass->res[id].config_data); + /* Get property values */ struct config_property resource_properties[] = { { "device_type", DATA_TYPE_STRING, true, (void *)device_type }, -- 2.7.4 From 3b9904360a97fb5d2adcef5b0305120858d52150 Mon Sep 17 00:00:00 2001 From: Dongwoo Lee Date: Tue, 20 Sep 2022 12:22:01 +0900 Subject: [PATCH 15/16] pass: parser: Change num_of_cpus and first_cpu into signed type Since two variable is unsigned so it can be never be less than zero, but in order to check error case they are needed to be negative. Change-Id: Ib60fafeee7c1352d6cf3d63e8e4f9c6e6e7f68ce Signed-off-by: Dongwoo Lee --- src/pass/pass.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pass/pass.h b/src/pass/pass.h index 7167688..009eed4 100644 --- a/src/pass/pass.h +++ b/src/pass/pass.h @@ -501,12 +501,12 @@ struct pass_resource_config_data { * The number of supported CPU in the same cluster. * If res_type is PASS_RESOURCE_CPU_ID, it is mandatory. */ - unsigned int num_cpus; + int num_cpus; /** * [optional] Index of first cpu in the same cluster. * If res_type is PASS_RESOURCE_CPU_ID, it is mandatory. */ - unsigned int cpu; + int cpu; /* * Parsed data from each resource info configuration -- 2.7.4 From df2b14364f4bf23fddffc98947dc02ab1eb1a562 Mon Sep 17 00:00:00 2001 From: Seung-Woo Kim Date: Fri, 23 Sep 2022 11:03:57 +0900 Subject: [PATCH 16/16] pass: Fix to return proper value for dbus signal handler From the dbus-stub generated code, signal handler should return TRUE if it handles invocation. Even ret value is error, fix to return TRUE after handling invocation. Below is dbus-stub comment about signal handler: If a signal handler returns %TRUE, it means the signal handler will handle the invocation and no other signal handlers will run. If no signal handler handles the invocation, the %G_DBUS_ERROR_UNKNOWN_METHOD error is returned. Returns: %G_DBUS_METHOD_INVOCATION_HANDLED or %TRUE if the invocation was handled, %G_DBUS_METHOD_INVOCATION_UNHANDLED or %FALSE to let other signal handlers run. NOTE: GLIB related asan heap-buffer-overflow issue is from the wrong return. Change-Id: I11e684101efb68e625a742b4d0655056299fd106 Suggested-by: Sangjung Woo Signed-off-by: Seung-Woo Kim --- src/pass/pass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pass/pass.c b/src/pass/pass.c index 112fa81..295e23f 100644 --- a/src/pass/pass.c +++ b/src/pass/pass.c @@ -148,7 +148,7 @@ static gboolean dbus_cb_core_start(SystemPassCore *obj, _E("failed to initialize PASS Core of the daemon " "in dbus callback for a start message\n"); - return FALSE; + return TRUE; } /** -- 2.7.4