[PATCH 1/2] x86/cpu-policy: enable build of fuzzing harness by default

Jan Beulich posted 2 patches 2 weeks, 2 days ago
There is a newer version of this series
[PATCH 1/2] x86/cpu-policy: enable build of fuzzing harness by default
Posted by Jan Beulich 2 weeks, 2 days ago
... on x86, to make sure its bit-rotting can be limited at least a little.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/fuzz/Makefile
+++ b/tools/fuzz/Makefile
@@ -4,6 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
 SUBDIRS-y :=
 SUBDIRS-y += libelf
 SUBDIRS-y += x86_instruction_emulator
+SUBDIRS-$(CONFIG_X86_64) += cpu-policy
 
 .PHONY: all clean distclean install uninstall
 all clean distclean install uninstall: %: subdirs-%
Re: [PATCH 1/2] x86/cpu-policy: enable build of fuzzing harness by default
Posted by Jan Beulich 2 days, 5 hours ago
On 22.12.2025 17:53, Jan Beulich wrote:
> ... on x86, to make sure its bit-rotting can be limited at least a little.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/tools/fuzz/Makefile
> +++ b/tools/fuzz/Makefile
> @@ -4,6 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>  SUBDIRS-y :=
>  SUBDIRS-y += libelf
>  SUBDIRS-y += x86_instruction_emulator
> +SUBDIRS-$(CONFIG_X86_64) += cpu-policy
>  
>  .PHONY: all clean distclean install uninstall
>  all clean distclean install uninstall: %: subdirs-%
> 

As it turns out this causes build failures on Ubuntu (and only there, and only
with gcc, which I don't understand):

afl-policy-fuzzer.c: In function 'main':
afl-policy-fuzzer.c:153:9: error: ignoring return value of 'fread', declared with attribute warn_unused_result [-Werror=unused-result]
         fread(cp, sizeof(*cp), 1, fp);
         ^
cc1: all warnings being treated as errors

Given how the code uses calloc() up front I don't really see why evaluating
the return value would actually be meaningful here, so I'm inclined to add a
cast to void (provided that would make a difference, which I have yet to
check). Opinions?

Jan
Re: [PATCH 1/2] x86/cpu-policy: enable build of fuzzing harness by default
Posted by Jan Beulich 2 days, 4 hours ago
On 06.01.2026 10:36, Jan Beulich wrote:
> On 22.12.2025 17:53, Jan Beulich wrote:
>> ... on x86, to make sure its bit-rotting can be limited at least a little.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/tools/fuzz/Makefile
>> +++ b/tools/fuzz/Makefile
>> @@ -4,6 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>>  SUBDIRS-y :=
>>  SUBDIRS-y += libelf
>>  SUBDIRS-y += x86_instruction_emulator
>> +SUBDIRS-$(CONFIG_X86_64) += cpu-policy
>>  
>>  .PHONY: all clean distclean install uninstall
>>  all clean distclean install uninstall: %: subdirs-%
>>
> 
> As it turns out this causes build failures on Ubuntu (and only there, and only
> with gcc, which I don't understand):
> 
> afl-policy-fuzzer.c: In function 'main':
> afl-policy-fuzzer.c:153:9: error: ignoring return value of 'fread', declared with attribute warn_unused_result [-Werror=unused-result]
>          fread(cp, sizeof(*cp), 1, fp);
>          ^
> cc1: all warnings being treated as errors
> 
> Given how the code uses calloc() up front I don't really see why evaluating
> the return value would actually be meaningful here, so I'm inclined to add a
> cast to void (provided that would make a difference, which I have yet to
> check). Opinions?

Simply casting doesn't work. Hence what I'm intending to do is

--- a/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
+++ b/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
@@ -133,6 +133,7 @@ int main(int argc, char **argv)
 #endif
     {
         struct cpu_policy *cp = NULL;
+        size_t size;
 
         if ( fp != stdin )
         {
@@ -150,9 +151,9 @@ int main(int argc, char **argv)
         if ( !cp )
             goto skip;
 
-        fread(cp, sizeof(*cp), 1, fp);
+        size = fread(cp, sizeof(*cp), 1, fp);
 
-        if ( !feof(fp) )
+        if ( !size || !feof(fp) )
             goto skip;
 
         check_policy(cp);

along with amending the description:

"Since on Ubuntu fread()'s return value needs evaluating, adjust the code
 there to also skip the test when there's no data at all."

May I keep your ack with that adjustment?

Jan
Re: [PATCH 1/2] x86/cpu-policy: enable build of fuzzing harness by default
Posted by Andrew Cooper 2 days, 3 hours ago
On 06/01/2026 9:49 am, Jan Beulich wrote:
> On 06.01.2026 10:36, Jan Beulich wrote:
>> On 22.12.2025 17:53, Jan Beulich wrote:
>>> ... on x86, to make sure its bit-rotting can be limited at least a little.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/tools/fuzz/Makefile
>>> +++ b/tools/fuzz/Makefile
>>> @@ -4,6 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>>>  SUBDIRS-y :=
>>>  SUBDIRS-y += libelf
>>>  SUBDIRS-y += x86_instruction_emulator
>>> +SUBDIRS-$(CONFIG_X86_64) += cpu-policy
>>>  
>>>  .PHONY: all clean distclean install uninstall
>>>  all clean distclean install uninstall: %: subdirs-%
>>>
>> As it turns out this causes build failures on Ubuntu (and only there, and only
>> with gcc, which I don't understand):
>>
>> afl-policy-fuzzer.c: In function 'main':
>> afl-policy-fuzzer.c:153:9: error: ignoring return value of 'fread', declared with attribute warn_unused_result [-Werror=unused-result]
>>          fread(cp, sizeof(*cp), 1, fp);
>>          ^
>> cc1: all warnings being treated as errors
>>
>> Given how the code uses calloc() up front I don't really see why evaluating
>> the return value would actually be meaningful here, so I'm inclined to add a
>> cast to void (provided that would make a difference, which I have yet to
>> check). Opinions?
> Simply casting doesn't work. Hence what I'm intending to do is
>
> --- a/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
> +++ b/tools/fuzz/cpu-policy/afl-policy-fuzzer.c
> @@ -133,6 +133,7 @@ int main(int argc, char **argv)
>  #endif
>      {
>          struct cpu_policy *cp = NULL;
> +        size_t size;
>  
>          if ( fp != stdin )
>          {
> @@ -150,9 +151,9 @@ int main(int argc, char **argv)
>          if ( !cp )
>              goto skip;
>  
> -        fread(cp, sizeof(*cp), 1, fp);
> +        size = fread(cp, sizeof(*cp), 1, fp);
>  
> -        if ( !feof(fp) )
> +        if ( !size || !feof(fp) )
>              goto skip;
>  
>          check_policy(cp);
>
> along with amending the description:
>
> "Since on Ubuntu fread()'s return value needs evaluating, adjust the code
>  there to also skip the test when there's no data at all."
>
> May I keep your ack with that adjustment?

Yes.  This is just harness code, so whatever compiles.

~Andrew

Re: [PATCH 1/2] x86/cpu-policy: enable build of fuzzing harness by default
Posted by Andrew Cooper 1 week, 3 days ago
On 22/12/2025 4:53 pm, Jan Beulich wrote:
> ... on x86, to make sure its bit-rotting can be limited at least a little.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/tools/fuzz/Makefile
> +++ b/tools/fuzz/Makefile
> @@ -4,6 +4,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>  SUBDIRS-y :=
>  SUBDIRS-y += libelf
>  SUBDIRS-y += x86_instruction_emulator

Blank line here please.  Or it's going to get messy with extra additions.

Otherwise, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

> +SUBDIRS-$(CONFIG_X86_64) += cpu-policy
>  
>  .PHONY: all clean distclean install uninstall
>  all clean distclean install uninstall: %: subdirs-%
>