mirror of
https://github.com/RGBCube/superfreq
synced 2025-07-27 17:07:44 +00:00
{core,battery}: better input validation; path, permission and sanity checks
- battery: add path existence and writability check for threshold support - cpu: validate governor and EPP values, and add frequency validation - engine: validate turbo auto settings for proper configuration - main: add validation for minimum and maximum frequency inputs
This commit is contained in:
parent
7b6602ad39
commit
d8f609fdef
4 changed files with 303 additions and 17 deletions
|
@ -161,12 +161,29 @@ fn write_sysfs_value(path: impl AsRef<Path>, value: &str) -> Result<()> {
|
|||
})
|
||||
}
|
||||
|
||||
/// Safely check if a path exists and is writable
|
||||
fn path_exists_and_writable(path: &Path) -> bool {
|
||||
if !path.exists() {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Try to open the file with write access to verify write permission
|
||||
match fs::OpenOptions::new().write(true).open(path) {
|
||||
Ok(_) => true,
|
||||
Err(_) => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Identifies if a battery supports threshold control and which pattern it uses
|
||||
fn find_battery_with_threshold_support(ps_path: &Path) -> Option<SupportedBattery<'static>> {
|
||||
for pattern in THRESHOLD_PATTERNS {
|
||||
let start_threshold_path = ps_path.join(pattern.start_path);
|
||||
let stop_threshold_path = ps_path.join(pattern.stop_path);
|
||||
if start_threshold_path.exists() && stop_threshold_path.exists() {
|
||||
|
||||
// Ensure both paths exist and are writable before considering this battery supported
|
||||
if path_exists_and_writable(&start_threshold_path)
|
||||
&& path_exists_and_writable(&stop_threshold_path)
|
||||
{
|
||||
return Some(SupportedBattery {
|
||||
name: ps_path.file_name()?.to_string_lossy().to_string(),
|
||||
pattern,
|
||||
|
|
194
src/cpu.rs
194
src/cpu.rs
|
@ -79,6 +79,15 @@ where
|
|||
}
|
||||
|
||||
pub fn set_governor(governor: &str, core_id: Option<u32>) -> Result<()> {
|
||||
// Validate the governor is available on this system
|
||||
if !is_governor_valid(governor)? {
|
||||
return Err(ControlError::InvalidValueError(format!(
|
||||
"Governor '{}' is not available on this system. Valid governors: {}",
|
||||
governor,
|
||||
get_available_governors()?.join(", ")
|
||||
)));
|
||||
}
|
||||
|
||||
let action = |id: u32| {
|
||||
let path = format!("/sys/devices/system/cpu/cpu{id}/cpufreq/scaling_governor");
|
||||
if Path::new(&path).exists() {
|
||||
|
@ -93,6 +102,32 @@ pub fn set_governor(governor: &str, core_id: Option<u32>) -> Result<()> {
|
|||
core_id.map_or_else(|| for_each_cpu_core(action), action)
|
||||
}
|
||||
|
||||
/// Check if the provided governor is available in the system
|
||||
fn is_governor_valid(governor: &str) -> Result<bool> {
|
||||
let governors = get_available_governors()?;
|
||||
Ok(governors.contains(&governor.to_string()))
|
||||
}
|
||||
|
||||
/// Get available CPU governors from the system
|
||||
fn get_available_governors() -> Result<Vec<String>> {
|
||||
// We'll check cpu0's available governors as they're typically the same across cores
|
||||
let path = "/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_governors";
|
||||
|
||||
if !Path::new(path).exists() {
|
||||
return Err(ControlError::NotSupported(
|
||||
"Could not determine available governors".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let content = fs::read_to_string(path)
|
||||
.map_err(|e| ControlError::ReadError(format!("Failed to read available governors: {e}")))?;
|
||||
|
||||
Ok(content
|
||||
.split_whitespace()
|
||||
.map(ToString::to_string)
|
||||
.collect())
|
||||
}
|
||||
|
||||
pub fn set_turbo(setting: TurboSetting) -> Result<()> {
|
||||
let value_pstate = match setting {
|
||||
TurboSetting::Always => "0", // no_turbo = 0 means turbo is enabled
|
||||
|
@ -153,6 +188,16 @@ fn try_set_per_core_boost(value: &str) -> Result<bool> {
|
|||
}
|
||||
|
||||
pub fn set_epp(epp: &str, core_id: Option<u32>) -> Result<()> {
|
||||
// Validate the EPP value against available options
|
||||
let available_epp = get_available_epp_values()?;
|
||||
if !available_epp.contains(&epp.to_string()) {
|
||||
return Err(ControlError::InvalidValueError(format!(
|
||||
"Invalid EPP value: '{}'. Available values: {}",
|
||||
epp,
|
||||
available_epp.join(", ")
|
||||
)));
|
||||
}
|
||||
|
||||
let action = |id: u32| {
|
||||
let path = format!("/sys/devices/system/cpu/cpu{id}/cpufreq/energy_performance_preference");
|
||||
if Path::new(&path).exists() {
|
||||
|
@ -164,8 +209,37 @@ pub fn set_epp(epp: &str, core_id: Option<u32>) -> Result<()> {
|
|||
core_id.map_or_else(|| for_each_cpu_core(action), action)
|
||||
}
|
||||
|
||||
/// Get available EPP values from the system
|
||||
fn get_available_epp_values() -> Result<Vec<String>> {
|
||||
let path = "/sys/devices/system/cpu/cpu0/cpufreq/energy_performance_available_preferences";
|
||||
|
||||
if !Path::new(path).exists() {
|
||||
// If the file doesn't exist, fall back to a default set of common values
|
||||
// This is safer than failing outright, as some systems may allow these values
|
||||
// even without explicitly listing them
|
||||
return Ok(vec![
|
||||
"default".to_string(),
|
||||
"performance".to_string(),
|
||||
"balance_performance".to_string(),
|
||||
"balance_power".to_string(),
|
||||
"power".to_string(),
|
||||
]);
|
||||
}
|
||||
|
||||
let content = fs::read_to_string(path).map_err(|e| {
|
||||
ControlError::ReadError(format!("Failed to read available EPP values: {e}"))
|
||||
})?;
|
||||
|
||||
Ok(content
|
||||
.split_whitespace()
|
||||
.map(ToString::to_string)
|
||||
.collect())
|
||||
}
|
||||
|
||||
pub fn set_epb(epb: &str, core_id: Option<u32>) -> Result<()> {
|
||||
// EPB is often an integer 0-15.
|
||||
// Validate EPB value - should be a number 0-15 or a recognized string value
|
||||
validate_epb_value(epb)?;
|
||||
|
||||
let action = |id: u32| {
|
||||
let path = format!("/sys/devices/system/cpu/cpu{id}/cpufreq/energy_performance_bias");
|
||||
if Path::new(&path).exists() {
|
||||
|
@ -177,7 +251,54 @@ pub fn set_epb(epb: &str, core_id: Option<u32>) -> Result<()> {
|
|||
core_id.map_or_else(|| for_each_cpu_core(action), action)
|
||||
}
|
||||
|
||||
fn validate_epb_value(epb: &str) -> Result<()> {
|
||||
// EPB can be a number from 0-15 or a recognized string
|
||||
// Try parsing as a number first
|
||||
if let Ok(value) = epb.parse::<u8>() {
|
||||
if value <= 15 {
|
||||
return Ok(());
|
||||
} else {
|
||||
return Err(ControlError::InvalidValueError(format!(
|
||||
"EPB numeric value must be between 0 and 15, got {}",
|
||||
value
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
// If not a number, check if it's a recognized string value
|
||||
let valid_strings = [
|
||||
"performance",
|
||||
"balance-performance",
|
||||
"balance-power",
|
||||
"power",
|
||||
// Alternative forms
|
||||
"balance_performance",
|
||||
"balance_power",
|
||||
];
|
||||
|
||||
if valid_strings.contains(&epb) {
|
||||
Ok(())
|
||||
} else {
|
||||
Err(ControlError::InvalidValueError(format!(
|
||||
"Invalid EPB value: '{}'. Must be a number 0-15 or one of: {}",
|
||||
epb,
|
||||
valid_strings.join(", ")
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
pub fn set_min_frequency(freq_mhz: u32, core_id: Option<u32>) -> Result<()> {
|
||||
// Check if the new minimum frequency would be greater than current maximum
|
||||
if let Some(id) = core_id {
|
||||
validate_min_frequency(id, freq_mhz)?;
|
||||
} else {
|
||||
// Check for all cores
|
||||
let num_cores = get_logical_core_count()?;
|
||||
for id in 0..num_cores {
|
||||
validate_min_frequency(id, freq_mhz)?;
|
||||
}
|
||||
}
|
||||
|
||||
let freq_khz_str = (freq_mhz * 1000).to_string();
|
||||
let action = |id: u32| {
|
||||
let path = format!("/sys/devices/system/cpu/cpu{id}/cpufreq/scaling_min_freq");
|
||||
|
@ -191,6 +312,17 @@ pub fn set_min_frequency(freq_mhz: u32, core_id: Option<u32>) -> Result<()> {
|
|||
}
|
||||
|
||||
pub fn set_max_frequency(freq_mhz: u32, core_id: Option<u32>) -> Result<()> {
|
||||
// Check if the new maximum frequency would be less than current minimum
|
||||
if let Some(id) = core_id {
|
||||
validate_max_frequency(id, freq_mhz)?;
|
||||
} else {
|
||||
// Check for all cores
|
||||
let num_cores = get_logical_core_count()?;
|
||||
for id in 0..num_cores {
|
||||
validate_max_frequency(id, freq_mhz)?;
|
||||
}
|
||||
}
|
||||
|
||||
let freq_khz_str = (freq_mhz * 1000).to_string();
|
||||
let action = |id: u32| {
|
||||
let path = format!("/sys/devices/system/cpu/cpu{id}/cpufreq/scaling_max_freq");
|
||||
|
@ -203,6 +335,66 @@ pub fn set_max_frequency(freq_mhz: u32, core_id: Option<u32>) -> Result<()> {
|
|||
core_id.map_or_else(|| for_each_cpu_core(action), action)
|
||||
}
|
||||
|
||||
fn read_sysfs_value_as_u32(path: &str) -> Result<u32> {
|
||||
if !Path::new(path).exists() {
|
||||
return Err(ControlError::NotSupported(format!(
|
||||
"File does not exist: {path}"
|
||||
)));
|
||||
}
|
||||
|
||||
let content = fs::read_to_string(path)
|
||||
.map_err(|e| ControlError::ReadError(format!("Failed to read {path}: {e}")))?;
|
||||
|
||||
content
|
||||
.trim()
|
||||
.parse::<u32>()
|
||||
.map_err(|e| ControlError::ReadError(format!("Failed to parse value from {path}: {e}")))
|
||||
}
|
||||
|
||||
fn validate_min_frequency(core_id: u32, new_min_freq_mhz: u32) -> Result<()> {
|
||||
let max_freq_path = format!("/sys/devices/system/cpu/cpu{core_id}/cpufreq/scaling_max_freq");
|
||||
|
||||
if !Path::new(&max_freq_path).exists() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let max_freq_khz = read_sysfs_value_as_u32(&max_freq_path)?;
|
||||
let new_min_freq_khz = new_min_freq_mhz * 1000;
|
||||
|
||||
if new_min_freq_khz > max_freq_khz {
|
||||
return Err(ControlError::InvalidValueError(format!(
|
||||
"Minimum frequency ({} MHz) cannot be higher than maximum frequency ({} MHz) for core {}",
|
||||
new_min_freq_mhz,
|
||||
max_freq_khz / 1000,
|
||||
core_id
|
||||
)));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_max_frequency(core_id: u32, new_max_freq_mhz: u32) -> Result<()> {
|
||||
let min_freq_path = format!("/sys/devices/system/cpu/cpu{core_id}/cpufreq/scaling_min_freq");
|
||||
|
||||
if !Path::new(&min_freq_path).exists() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let min_freq_khz = read_sysfs_value_as_u32(&min_freq_path)?;
|
||||
let new_max_freq_khz = new_max_freq_mhz * 1000;
|
||||
|
||||
if new_max_freq_khz < min_freq_khz {
|
||||
return Err(ControlError::InvalidValueError(format!(
|
||||
"Maximum frequency ({} MHz) cannot be lower than minimum frequency ({} MHz) for core {}",
|
||||
new_max_freq_mhz,
|
||||
min_freq_khz / 1000,
|
||||
core_id
|
||||
)));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Sets the platform profile.
|
||||
/// This changes the system performance, temperature, fan, and other hardware replated characteristics.
|
||||
///
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
use crate::battery;
|
||||
use crate::config::{AppConfig, ProfileConfig};
|
||||
use crate::config::{AppConfig, ProfileConfig, TurboAutoSettings};
|
||||
use crate::core::{OperationalMode, SystemReport, TurboSetting};
|
||||
use crate::cpu::{self};
|
||||
use crate::util::error::{ControlError, EngineError};
|
||||
|
@ -177,6 +177,9 @@ fn manage_auto_turbo(report: &SystemReport, config: &ProfileConfig) -> Result<()
|
|||
// Get the auto turbo settings from the config, or use defaults
|
||||
let turbo_settings = config.turbo_auto_settings.clone().unwrap_or_default();
|
||||
|
||||
// Validate the complete configuration to ensure it's usable
|
||||
validate_turbo_auto_settings(&turbo_settings)?;
|
||||
|
||||
// Get average CPU temperature and CPU load
|
||||
let cpu_temp = report.cpu_global.average_temperature_celsius;
|
||||
|
||||
|
@ -202,14 +205,6 @@ fn manage_auto_turbo(report: &SystemReport, config: &ProfileConfig) -> Result<()
|
|||
}
|
||||
};
|
||||
|
||||
// Validate the configuration to ensure it's usable
|
||||
if turbo_settings.load_threshold_high <= turbo_settings.load_threshold_low {
|
||||
return Err(EngineError::ConfigurationError(
|
||||
"Invalid turbo auto settings: high threshold must be greater than low threshold"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
// Decision logic for enabling/disabling turbo
|
||||
let enable_turbo = match (cpu_temp, avg_cpu_usage) {
|
||||
// If temperature is too high, disable turbo regardless of load
|
||||
|
@ -262,3 +257,30 @@ fn manage_auto_turbo(report: &SystemReport, config: &ProfileConfig) -> Result<()
|
|||
Err(e) => Err(EngineError::ControlError(e)),
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_turbo_auto_settings(settings: &TurboAutoSettings) -> Result<(), EngineError> {
|
||||
// Validate load thresholds
|
||||
if settings.load_threshold_high <= settings.load_threshold_low {
|
||||
return Err(EngineError::ConfigurationError(
|
||||
"Invalid turbo auto settings: high threshold must be greater than low threshold"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
// Validate range of load thresholds (should be 0-100%)
|
||||
if settings.load_threshold_high > 100.0 || settings.load_threshold_low < 0.0 {
|
||||
return Err(EngineError::ConfigurationError(
|
||||
"Invalid turbo auto settings: load thresholds must be between 0% and 100%".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
// Validate temperature threshold (realistic range for CPU temps in Celsius)
|
||||
if settings.temp_threshold_high <= 0.0 || settings.temp_threshold_high > 110.0 {
|
||||
return Err(EngineError::ConfigurationError(
|
||||
"Invalid turbo auto settings: temperature threshold must be between 0°C and 110°C"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
|
67
src/main.rs
67
src/main.rs
|
@ -357,15 +357,70 @@ fn main() {
|
|||
cpu::set_epb(&epb, core_id).map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
|
||||
}
|
||||
Some(Commands::SetMinFreq { freq_mhz, core_id }) => {
|
||||
cpu::set_min_frequency(freq_mhz, core_id)
|
||||
.map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
|
||||
// Basic validation for reasonable CPU frequency values
|
||||
if freq_mhz == 0 {
|
||||
error!("Minimum frequency cannot be zero");
|
||||
Err(Box::new(ControlError::InvalidValueError(
|
||||
"Minimum frequency cannot be zero".to_string(),
|
||||
)) as Box<dyn std::error::Error>)
|
||||
} else if freq_mhz > 10000 {
|
||||
// Extremely high value unlikely to be valid
|
||||
error!("Minimum frequency ({freq_mhz} MHz) is unreasonably high");
|
||||
Err(Box::new(ControlError::InvalidValueError(format!(
|
||||
"Minimum frequency ({freq_mhz} MHz) is unreasonably high"
|
||||
))) as Box<dyn std::error::Error>)
|
||||
} else {
|
||||
cpu::set_min_frequency(freq_mhz, core_id)
|
||||
.map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
|
||||
}
|
||||
}
|
||||
Some(Commands::SetMaxFreq { freq_mhz, core_id }) => {
|
||||
cpu::set_max_frequency(freq_mhz, core_id)
|
||||
.map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
|
||||
// Basic validation for reasonable CPU frequency values
|
||||
if freq_mhz == 0 {
|
||||
error!("Maximum frequency cannot be zero");
|
||||
Err(Box::new(ControlError::InvalidValueError(
|
||||
"Maximum frequency cannot be zero".to_string(),
|
||||
)) as Box<dyn std::error::Error>)
|
||||
} else if freq_mhz > 10000 {
|
||||
// Extremely high value unlikely to be valid
|
||||
error!("Maximum frequency ({freq_mhz} MHz) is unreasonably high");
|
||||
Err(Box::new(ControlError::InvalidValueError(format!(
|
||||
"Maximum frequency ({freq_mhz} MHz) is unreasonably high"
|
||||
))) as Box<dyn std::error::Error>)
|
||||
} else {
|
||||
cpu::set_max_frequency(freq_mhz, core_id)
|
||||
.map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
|
||||
}
|
||||
}
|
||||
Some(Commands::SetPlatformProfile { profile }) => {
|
||||
// Get available platform profiles and validate early if possible
|
||||
match cpu::get_platform_profiles() {
|
||||
Ok(available_profiles) => {
|
||||
if !available_profiles.contains(&profile) {
|
||||
error!(
|
||||
"Invalid platform profile: '{}'. Available profiles: {}",
|
||||
profile,
|
||||
available_profiles.join(", ")
|
||||
);
|
||||
Err(Box::new(ControlError::InvalidProfile(format!(
|
||||
"Invalid platform profile: '{}'. Available profiles: {}",
|
||||
profile,
|
||||
available_profiles.join(", ")
|
||||
))) as Box<dyn std::error::Error>)
|
||||
} else {
|
||||
info!("Setting platform profile to '{}'", profile);
|
||||
cpu::set_platform_profile(&profile)
|
||||
.map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
|
||||
}
|
||||
}
|
||||
Err(_) => {
|
||||
// If we can't get profiles (e.g., feature not supported), pass through to the function
|
||||
// which will provide appropriate error
|
||||
cpu::set_platform_profile(&profile)
|
||||
.map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
|
||||
}
|
||||
}
|
||||
}
|
||||
Some(Commands::SetPlatformProfile { profile }) => cpu::set_platform_profile(&profile)
|
||||
.map_err(|e| Box::new(e) as Box<dyn std::error::Error>),
|
||||
Some(Commands::SetBatteryThresholds {
|
||||
start_threshold,
|
||||
stop_threshold,
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue