intel / thermal_daemon

Thermal daemon for IA
GNU General Public License v2.0
540 stars 117 forks source link

static analysis report #369

Closed benzea closed 7 months ago

benzea commented 2 years ago

Hi,

I got a few new reports, I think most can be safely ignored, but there are some oddities (like swapped warnings). Anyway, posting it here, as I am leaving Red Hat and I am not sure who is taking over Red Hat.

Error: CLANG_WARNING:
thermal_daemon-2.5/src/thd_cdev.cpp:142:3: warning[deadcode.DeadStores]: Value stored to '_curr_state' is never read
#  140|     } else {
#  141|         // Get the latest state, which is not the latest from the hardware but last set state to the device
#  142|->       _curr_state = get_curr_state();
#  143|         _curr_state = thd_clamp_state_max(curr_state);
#  144|   

Error: CLANG_WARNING:
thermal_daemon-2.5/src/thd_cdev.cpp:329:3: warning[deadcode.DeadStores]: Value stored to 'target_value' is never read
#  327|   
#  328|         limit = zone_trip_limits[zone_trip_limits.size() - 1];
#  329|->       target_value = limit.target_value;
#  330|         target_state_valid = limit.target_state_valid;
#  331|   

Error: CLANG_WARNING:
thermal_daemon-2.5/src/thd_cdev.cpp:333:3: warning[deadcode.DeadStores]: Value stored to 'min_max_valid' is never read
#  331|   
#  332|         target_value = limit.target_value;
#  333|->       min_max_valid = limit._min_max_valid;
#  334|         _max_state = limit._max_state;
#  335|         _min_state = limit._min_state;

Error: SWAPPED_ARGUMENTS (CWE-683):
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:303: swapped_arguments: The positions of arguments in the call to "thd_trip_point_add_cdev" do not match the ordering of the parameters:
* "max_state" is passed to "min_state".
* "min_state" is passed to "max_state".

#  301|             zone->get_zone_index(), sensor->get_index(), SEQUENTIAL);
#  302|   
#  303|->   trip_pt.thd_trip_point_add_cdev(*cdev, cthd_trip_point::default_influence,
#  304|     DEFAULT_SAMPLE_TIME_SEC, 0, 0,
#  305|     NULL, 1, max_state, min_state);

Error: PASS_BY_VALUE (CWE-628):
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:450: pass_by_value: Passing parameter target of type "adaptive_target" (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.
#  448|   }
#  449|   
#  450|-> void cthd_engine_adaptive::execute_target(struct adaptive_target target) {
#  451|     cthd_cdev *cdev;
#  452|     std::string name;

Error: RESOURCE_LEAK (CWE-772):
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:582: alloc_fn: Storage is returned from allocation function "operator new[]".
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:582: var_assign: Assigning: "buf" = storage returned from "new char[size]".
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:588: noescape: Resource "buf" is not freed or pointed-to in "read".
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:608: leaked_storage: Variable "buf" going out of scope leaks the storage it points to.
#  606|             passive_def_only = 1;
#  607|         }
#  608|->       return THD_SUCCESS;
#  609|     }
#  610|   

Error: RESOURCE_LEAK (CWE-772):
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:582: alloc_fn: Storage is returned from allocation function "operator new[]".
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:582: var_assign: Assigning: "buf" = storage returned from "new char[size]".
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:588: noescape: Resource "buf" is not freed or pointed-to in "read".
thermal_daemon-2.5/src/thd_engine_adaptive.cpp:616: leaked_storage: Variable "buf" going out of scope leaks the storage it points to.
#  614|         return res;
#  615|   
#  616|->   return THD_SUCCESS;
#  617|   }
#  618|   

Error: CPPCHECK_WARNING (CWE-909):
thermal_daemon-2.5/src/thd_gddv.cpp:338: error[uninitStructMember]: Uninitialized struct member: condition.ignore_condition
#  336|                     }
#  337|                 }
#  338|->               condition_set.push_back(condition);
#  339|             }
#  340|             conditions.push_back(condition_set);

Error: UNINIT (CWE-457):
thermal_daemon-2.5/src/thd_gddv.cpp:298: var_decl: Declaring variable "condition".
thermal_daemon-2.5/src/thd_gddv.cpp:338: uninit_use_in_call: Using uninitialized value "condition". Field "condition.ignore_condition" is uninitialized when calling "push_back". [Note: The source code implementation of the function has been overridden by a builtin model.]
#  336|                     }
#  337|                 }
#  338|->               condition_set.push_back(condition);
#  339|             }
#  340|             conditions.push_back(condition_set);

Error: TAINTED_SCALAR (CWE-20):
thermal_daemon-2.5/src/thd_gddv.cpp:1487: tainted_argument: Calling function "read" taints argument "*buf".
thermal_daemon-2.5/src/thd_gddv.cpp:1495: tainted_data: Passing tainted expression "*buf" to "parse_gddv", which uses it as an allocation size.
thermal_daemon-2.5/src/thd_gddv.cpp:1495: remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
# 1493|   
# 1494|     try {
# 1495|->       if (parse_gddv(buf, size, NULL)) {
# 1496|             thd_log_debug("Unable to parse GDDV");
# 1497|             delete[] buf;

Error: UNINIT_CTOR (CWE-456):
thermal_daemon-2.5/src/thd_gddv.h:186: member_decl: Class member declaration for "power_profiles_daemon".
thermal_daemon-2.5/src/thd_gddv.h:240: uninit_member: Non-static class member "power_profiles_daemon" is not initialized in this constructor nor in any functions that it calls.
thermal_daemon-2.5/src/thd_gddv.h:188: member_decl: Class member declaration for "lid_dev".
thermal_daemon-2.5/src/thd_gddv.h:240: uninit_member: Non-static class member "lid_dev" is not initialized in this constructor nor in any functions that it calls.
#  238|             NULL), tablet_dev(NULL), int3400_base_path(""), power_slider(75), current_condition_set(
#  239|                     0xffff) {
#  240|->   }
#  241|   
#  242|     ~cthd_gddv();

Error: UNINIT (CWE-457):
thermal_daemon-2.5/src/thd_trip_point.cpp:335: var_decl: Declaring variable "thd_cdev".
thermal_daemon-2.5/src/thd_trip_point.cpp:358: uninit_use_in_call: Using uninitialized value "thd_cdev". Field "thd_cdev.min_state" is uninitialized when calling "trip_cdev_add".
#  356|         memset(&thd_cdev.pid_param, 0, sizeof(pid_param_t));
#  357|     }
#  358|->   trip_cdev_add(thd_cdev);
#  359|   }
#  360|   

Error: CPPCHECK_WARNING (CWE-909):
thermal_daemon-2.5/src/thd_trip_point.cpp:370: error[uninitStructMember]: Uninitialized struct member: thd_cdev.max_state
#  368|         thd_cdev.last_op_time = 0;
#  369|         thd_cdev.target_state_valid = 0;
#  370|->       trip_cdev_add(thd_cdev);
#  371|         return THD_SUCCESS;
#  372|     } else {

Error: CPPCHECK_WARNING (CWE-909):
thermal_daemon-2.5/src/thd_trip_point.cpp:370: error[uninitStructMember]: Uninitialized struct member: thd_cdev.min_max_valid
#  368|         thd_cdev.last_op_time = 0;
#  369|         thd_cdev.target_state_valid = 0;
#  370|->       trip_cdev_add(thd_cdev);
#  371|         return THD_SUCCESS;
#  372|     } else {

Error: CPPCHECK_WARNING (CWE-909):
thermal_daemon-2.5/src/thd_trip_point.cpp:370: error[uninitStructMember]: Uninitialized struct member: thd_cdev.min_state
#  368|         thd_cdev.last_op_time = 0;
#  369|         thd_cdev.target_state_valid = 0;
#  370|->       trip_cdev_add(thd_cdev);
#  371|         return THD_SUCCESS;
#  372|     } else {

Error: PASS_BY_VALUE (CWE-628):
thermal_daemon-2.5/src/thd_trip_point.h:72: pass_by_value: Passing parameter cdev1 of type "trip_pt_cdev_t" (size 136 bytes) by value, which exceeds the low threshold of 128 bytes.
thermal_daemon-2.5/src/thd_trip_point.h:72: pass_by_value: Passing parameter cdev2 of type "trip_pt_cdev_t" (size 136 bytes) by value, which exceeds the low threshold of 128 bytes.
#   70|   #define DEFAULT_SENSOR_ID 0xFFFF
#   71|   
#   72|-> static bool trip_cdev_sort(trip_pt_cdev_t cdev1, trip_pt_cdev_t cdev2) {
#   73|     return (cdev1.influence > cdev2.influence);
#   74|   }

Error: PASS_BY_VALUE (CWE-628):
thermal_daemon-2.5/src/thd_trip_point.h:216: pass_by_value: Passing parameter trip_cdev of type "trip_pt_cdev_t" (size 136 bytes) by value, which exceeds the low threshold of 128 bytes.
#  214|   #endif
#  215|   
#  216|->   void trip_cdev_add(trip_pt_cdev_t trip_cdev) {
#  217|         int index;
#  218|         if (check_duplicate(trip_cdev.cdev, &index)) {
spandruvada commented 1 year ago

Good Luck, wherever you are heading. I will take a look.