On 7/19/24 21:01, Sean Christopherson wrote:
> On Fri, Jul 19, 2024, Mirsad Todorovac wrote:
>> Hi, all!
>>
>> On linux-stable 6.10 vanilla tree, another NULL pointer is passed, which was detected
>> by the fortify-string.h mechanism.
>>
>> arch/x86/kvm/x86.c
>> ==================
>>
>> 13667 kvm_prepare_emulation_failure_exit(vcpu);
>>
>> calls
>>
>> 8796 __kvm_prepare_emulation_failure_exit(vcpu, NULL, 0);
>>
>> which calls
>>
>> 8790 prepare_emulation_failure_exit(vcpu, data, ndata, NULL, 0);
>>
>> Note here that data == NULL and ndata = 0.
>>
>> again data == NULL and ndata == 0, which passes unchanged all until
>>
>> 8773 memcpy(&run->internal.data[info_start + ARRAY_SIZE(info)], data, ndata * sizeof(data[0]));
>
> My reading of the C99 is that KVM's behavior is fine.
>
> Where an argument declared as size_t n specifies the length of the array for a
> function, n can have the value zero on a call to that function. Unless explicitly stated
> otherwise in the description of a particular function in this subclause, pointer arguments
> on such a call shall still have valid values, as described in 7.1.4. On such a call, a
> function that locates a character finds no occurrence, a function that compares two
> character sequences returns zero, and a function that copies characters copies zero
> characters.
>
> If the function copies zero characters, then there can't be a store to the NULL
> pointer, and if there's no store, there's no NULL pointer explosion.
>
> I suppose arguably one could argue the builtin memcpy() could deliberately fail
> on an invalid pointer, but that'd be rather ridiculous.
Thank you again, Sean, for committing so much attention to this warning.
Please don't blame it on me.
It would be for the best if I give the full compiler's warning message:
In file included from ./include/linux/string.h:374,
from ./include/linux/bitmap.h:13,
from ./include/linux/cpumask.h:13,
from ./include/linux/alloc_tag.h:13,
from ./include/linux/percpu.h:5,
from ./include/linux/context_tracking_state.h:5,
from ./include/linux/hardirq.h:5,
from ./include/linux/kvm_host.h:7,
from arch/x86/kvm/x86.c:20:
arch/x86/kvm/x86.c: In function ‘prepare_emulation_failure_exit’:
./include/linux/fortify-string.h:114:33: error: use of NULL ‘data’ where non-null expected [CWE-476] [-Werror=analyzer-null-argument]
114 | #define __underlying_memcpy __builtin_memcpy
| ^
./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
633 | __underlying_##op(p, q, __fortify_size); \
| ^~~~~~~~~~~~~
./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
| ^~~~~~~~~~~~~~~~~~~~
arch/x86/kvm/x86.c:8773:9: note: in expansion of macro ‘memcpy’
8773 | memcpy(&run->internal.data[info_start + ARRAY_SIZE(info)], data,
| ^~~~~~
‘kvm_handle_memory_failure’: events 1-4
|
|13649 | int kvm_handle_memory_failure(struct kvm_vcpu *vcpu, int r,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (1) entry to ‘kvm_handle_memory_failure’
|......
|13652 | if (r == X86EMUL_PROPAGATE_FAULT) {
| | ~
| | |
| | (2) following ‘false’ branch (when ‘r != 2’)...
|......
|13667 | kvm_prepare_emulation_failure_exit(vcpu);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (3) ...to here
| | (4) calling ‘kvm_prepare_emulation_failure_exit’ from ‘kvm_handle_memory_failure’
|
+--> ‘kvm_prepare_emulation_failure_exit’: events 5-6
|
| 8790 | prepare_emulation_failure_exit(vcpu, data, ndata, NULL, 0);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (6) calling ‘prepare_emulation_failure_exit’ from ‘kvm_prepare_emulation_failure_exit’
|......
| 8794 | void kvm_prepare_emulation_failure_exit(struct kvm_vcpu *vcpu)
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (5) entry to ‘kvm_prepare_emulation_failure_exit’
|
+--> ‘prepare_emulation_failure_exit’: event 7
|
| 8728 | static void prepare_emulation_failure_exit(struct kvm_vcpu *vcpu, u64 *data,
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (7) entry to ‘prepare_emulation_failure_exit’
|
‘prepare_emulation_failure_exit’: event 8
|
|./include/asm-generic/bug.h:112:12:
| 112 | if (unlikely(__ret_warn_on)) \
| | ^
| | |
| | (8) following ‘false’ branch...
arch/x86/kvm/x86.c:8753:13: note: in expansion of macro ‘WARN_ON_ONCE’
| 8753 | if (WARN_ON_ONCE(ndata > 4))
| | ^~~~~~~~~~~~
|
‘prepare_emulation_failure_exit’: events 9-10
|
| 8757 | info_start = 1;
| | ^~~~~~~~~~
| | |
| | (9) ...to here
|......
| 8760 | if (insn_size) {
| | ~
| | |
| | (10) following ‘false’ branch (when ‘insn_size == 0’)...
|
‘prepare_emulation_failure_exit’: event 11
|
|./include/linux/fortify-string.h:620:62:
| 620 | p_size_field, q_size_field, op) ({ \
| | ^
| | |
| | (11) ...to here
./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
| 678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
| | ^~~~~~~~~~~~~~~~~~~~
arch/x86/kvm/x86.c:8772:9: note: in expansion of macro ‘memcpy’
| 8772 | memcpy(&run->internal.data[info_start], info, sizeof(info));
| | ^~~~~~
|
‘prepare_emulation_failure_exit’: events 12-17
|
|./include/linux/fortify-string.h:596:12:
| 596 | if (p_size != SIZE_MAX && p_size < size)
| | ^
| | |
| | (12) following ‘false’ branch (when ‘__p_size > 39’)...
| | (14) following ‘false’ branch (when ‘__fortify_size <= __p_size’)...
| 597 | fortify_panic(func, FORTIFY_WRITE, p_size, size, true);
| 598 | else if (q_size != SIZE_MAX && q_size < size)
| | ~~ ~
| | | |
| | | (16) following ‘false’ branch (when ‘__fortify_size <= __q_size’)...
| | (13) ...to here
| | (15) ...to here
|......
| 612 | if (p_size_field != SIZE_MAX &&
| | ~~
| | |
| | (17) ...to here
|
‘prepare_emulation_failure_exit’: event 18
|
| 114 | #define __underlying_memcpy __builtin_memcpy
| | ^
| | |
| | (18) argument 2 (‘data’) NULL where non-null expected
./include/linux/fortify-string.h:633:9: note: in expansion of macro ‘__underlying_memcpy’
| 633 | __underlying_##op(p, q, __fortify_size); \
| | ^~~~~~~~~~~~~
./include/linux/fortify-string.h:678:26: note: in expansion of macro ‘__fortify_memcpy_chk’
| 678 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
| | ^~~~~~~~~~~~~~~~~~~~
arch/x86/kvm/x86.c:8773:9: note: in expansion of macro ‘memcpy’
| 8773 | memcpy(&run->internal.data[info_start + ARRAY_SIZE(info)], data,
| | ^~~~~~
|
<built-in>: note: argument 2 of ‘__builtin_memcpy’ must be non-null
CC [M] kernel/kheaders.o
cc1: all warnings being treated as errors
Hope this helps.
Best regards,
Mirsad Todorovac
