From bbde1d035731370cb9e00914ba9fc3667b0541a5 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Thu, 21 Mar 2019 09:55:51 +0100 Subject: [PATCH] Ensure strncpy'ed strings are all null terminated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If there is no null byte among the first n bytes of the source the resulting string will not be properly null terminated. Ensure that all strings that are copied via strncpy are properly terminated copy "sizeof (dest) - 1" bytes and manually terminate the string in the cases the array was not initialized. Example compiler warning: ../daemon/gamemode-tests.c: In function ‘run_cpu_governor_tests’: ../daemon/gamemode-tests.c:326:4: warning: ‘strncpy’ specified bound 256 equals destination size [-Wstringop-truncation] strncpy(defaultgov, currentgov, CONFIG_VALUE_MAX); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ --- daemon/gamemode-gpu.c | 3 ++- daemon/gamemode-tests.c | 10 +++++----- daemon/governors-query.c | 2 +- daemon/gpuclockctl.c | 8 ++++---- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/daemon/gamemode-gpu.c b/daemon/gamemode-gpu.c index 9c2d707..675920e 100644 --- a/daemon/gamemode-gpu.c +++ b/daemon/gamemode-gpu.c @@ -224,7 +224,8 @@ int game_mode_get_gpu(GameModeGPUInfo *info) } break; case Vendor_AMD: - strncpy(info->amd_performance_level, buffer, CONFIG_VALUE_MAX); + strncpy(info->amd_performance_level, buffer, sizeof(info->amd_performance_level) - 1); + info->amd_performance_level[sizeof(info->amd_performance_level) - 1] = '\0'; break; } diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c index 9fa073e..b8549ae 100644 --- a/daemon/gamemode-tests.c +++ b/daemon/gamemode-tests.c @@ -323,7 +323,7 @@ static int run_cpu_governor_tests(struct GameModeConfig *config) if (defaultgov[0] == '\0') { const char *currentgov = get_gov_state(); if (currentgov) { - strncpy(defaultgov, currentgov, CONFIG_VALUE_MAX); + strncpy(defaultgov, currentgov, CONFIG_VALUE_MAX - 1); } else { LOG_ERROR( "Could not get current CPU governor state, this indicates an error! See rest " @@ -445,16 +445,16 @@ int run_gpu_optimisation_tests(struct GameModeConfig *config) long expected_core = gpuinfo->nv_core; long expected_mem = gpuinfo->nv_mem; long expected_nv_powermizer_mode = gpuinfo->nv_powermizer_mode; - char expected_amd_performance_level[CONFIG_VALUE_MAX]; - strncpy(expected_amd_performance_level, gpuinfo->amd_performance_level, CONFIG_VALUE_MAX); + char expected_amd_performance_level[CONFIG_VALUE_MAX] = { 0 }; + strncpy(expected_amd_performance_level, gpuinfo->amd_performance_level, CONFIG_VALUE_MAX - 1); /* Get current stats */ game_mode_get_gpu(gpuinfo); long original_nv_core = gpuinfo->nv_core; long original_nv_mem = gpuinfo->nv_mem; long original_nv_powermizer_mode = gpuinfo->nv_powermizer_mode; - char original_amd_performance_level[CONFIG_VALUE_MAX]; - strncpy(original_amd_performance_level, gpuinfo->amd_performance_level, CONFIG_VALUE_MAX); + char original_amd_performance_level[CONFIG_VALUE_MAX] = { 0 }; + strncpy(original_amd_performance_level, gpuinfo->amd_performance_level, CONFIG_VALUE_MAX - 1); /* account for when settings are not set */ if (expected_nv_powermizer_mode == -1) diff --git a/daemon/governors-query.c b/daemon/governors-query.c index 50ab711..d357678 100644 --- a/daemon/governors-query.c +++ b/daemon/governors-query.c @@ -134,7 +134,7 @@ const char *get_gov_state(void) return "malformed"; } - strncpy(governor, contents, sizeof(governor)); + strncpy(governor, contents, sizeof(governor) - 1); } else { LOG_ERROR("Failed to read contents of %s\n", gov); } diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c index 400327a..b59340c 100644 --- a/daemon/gpuclockctl.c +++ b/daemon/gpuclockctl.c @@ -289,7 +289,7 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) return -1; } - char buff[CONFIG_VALUE_MAX]; + char buff[CONFIG_VALUE_MAX] = { 0 }; if (!fgets(buff, CONFIG_VALUE_MAX, file)) { LOG_ERROR("Could not read file %s (%s)!\n", path, strerror(errno)); return -1; @@ -301,8 +301,8 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) } /* Copy in the value from the file */ - strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX); - + strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX - 1); + info->amd_performance_level[CONFIG_VALUE_MAX - 1] = '\0'; return info == NULL; } @@ -449,7 +449,7 @@ int main(int argc, char *argv[]) LOG_ERROR("Must pass performance level for AMD gpu!\n"); print_usage_and_exit(); } - strncpy(info.amd_performance_level, argv[3], CONFIG_VALUE_MAX); + strncpy(info.amd_performance_level, argv[3], CONFIG_VALUE_MAX - 1); return set_gpu_state_amd(&info); break; default: