On 2025-02-11 07:08, Jan Beulich wrote:
On 06.02.2025 09:32, Penny Zheng wrote:
--- /dev/null
+++ b/xen/arch/x86/acpi/cpufreq/amd-cppc.c
@@ -0,0 +1,64 @@
+static bool __init amd_cppc_handle_option(const char *s, const char *end)
+{
+ int ret;
+
+ ret = parse_boolean("verbose", s, end);
+ if ( ret >= 0 )
+ {
+ cpufreq_verbose = ret;
+ return true;
+ }
+
+ return false;
+}
+
+int __init amd_cppc_cmdline_parse(const char *s, const char *e)
+{
+ do
+ {
+ const char *end = strpbrk(s, ",;");
+
+ if ( !amd_cppc_handle_option(s, end) )
You have an incoming "e" here. What if the comma / semicolon you find
is past where that points? (I understand you've copied that from
hwp_cmdline_parse(), and should have raised the question already there
when reviewing the respective patch. It also looks as if behavior-
wise all would be okay here. It's just that this very much looks like
a buffer overrun on the first and second glance.)
It's been a while since I worked on HWP. I think my assumption was that
you are inside a nul terminated string, and command line option parsing
functions can handle end == NULL, so it would all work out. A too long
string crossing " " or ";" would not match.
+ {
+ printk(XENLOG_WARNING
+ "cpufreq/amd-cppc: option '%.*s' not recognized\n",
+ (int)((end ?: e) - s), s);
+
+ return -EINVAL;
+ }
+
+ s = end ? ++end : end;
The increment is odd here (again inherited from the HWP function), as
"end" is about to go out of scope.
For HWP, I copied from setup_cpufreq_option() which does similar.
You'd prefer:
s = end ? end + 1 : NULL;
That is more explicit which is good. I considered using NULL back when
working on that. I think I when with the current form to match
setup_cpufreq_option().
Regards,
Jason
