On 27.05.2025 10:48, Penny Zheng wrote:
> Accessing to perf.states[] array shall not be only guarded with user-defined
> hypercall input, so we add XEN_PX_INIT check to gain safety.
What is "guarded with user-defined hypercall input"? And what safety are we
lacking?
> --- a/xen/drivers/acpi/pmstat.c
> +++ b/xen/drivers/acpi/pmstat.c
> @@ -228,10 +228,13 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> ret = copy_to_guest(op->u.get_para.affected_cpus,
> data, op->u.get_para.cpu_num);
>
> - for ( i = 0; i < op->u.get_para.freq_num; i++ )
> - data[i] = pmpt->perf.states[i].core_frequency * 1000;
> - ret += copy_to_guest(op->u.get_para.scaling_available_frequencies,
> - data, op->u.get_para.freq_num);
> + if ( pmpt->perf.init & XEN_PX_INIT )
> + {
> + for ( i = 0; i < op->u.get_para.freq_num; i++ )
> + data[i] = pmpt->perf.states[i].core_frequency * 1000;
> + ret += copy_to_guest(op->u.get_para.scaling_available_frequencies,
> + data, op->u.get_para.freq_num);
> + }
Going from just the code change: You want to avoid copying out frequency
values when none have been reported? But when none have been reported,
isn't pmpt->perf.state_count (against which op->u.get_para.freq_num was
validated) simply going to be 0? If not, how would callers know that no
data was handed back to them?
Jan
