On Fri, Jul 19, 2024 at 12:41 PM Mirsad Todorovac <mtodorovac69@xxxxxxxxx> wrote: > > > > On 7/19/24 21:22, Sean Christopherson wrote: > > On Fri, Jul 19, 2024, Mirsad Todorovac wrote: > >> On 7/19/24 19:21, Sean Christopherson wrote: > >>> On Fri, Jul 19, 2024, Mirsad Todorovac wrote: > >>>> Hi, > >>>> > >>>> In the build of 6.10.0 from stable tree, the following error was detected. > >>>> > >>>> You see that the function get_fixed_pmc() can return NULL pointer as a result > >>>> if msr is outside of [base, base + pmu->nr_arch_fixed_counters) interval. > >>>> > >>>> kvm_pmu_request_counter_reprogram(pmc) is then called with that NULL pointer > >>>> as the argument, which expands to .../pmu.h > >>>> > >>>> #define pmc_to_pmu(pmc) (&(pmc)->vcpu->arch.pmu) > >>>> > >>>> which is a NULL pointer dereference in that speculative case. > >>> > >>> I'm somewhat confused. Did you actually hit a BUG() due to a NULL-pointer > >>> dereference, are you speculating that there's a bug, or did you find some speculation > >>> issue with the CPU? > >>> > >>> It should be impossible for get_fixed_pmc() to return NULL in this case. The > >>> loop iteration is fully controlled by KVM, i.e. 'i' is guaranteed to be in the > >>> ranage [0..pmu->nr_arch_fixed_counters). > >>> > >>> And the input @msr is "MSR_CORE_PERF_FIXED_CTR0 +i", so the if-statement expands to: > >>> > >>> if (MSR_CORE_PERF_FIXED_CTR0 + [0..pmu->nr_arch_fixed_counters) >= MSR_CORE_PERF_FIXED_CTR0 && > >>> MSR_CORE_PERF_FIXED_CTR0 + [0..pmu->nr_arch_fixed_counters) < MSR_CORE_PERF_FIXED_CTR0 + pmu->nr_arch_fixed_counters) > >>> > >>> i.e. is guaranteed to evaluate true. > >>> > >>> Am I missing something? > >> > >> Hi Sean, > >> > >> Thank you for replying promptly. > >> > >> Perhaps I should have provided the GCC error report in the first place. > > > > Yes, though the report itself is somewhat secondary, what matters the most is how > > you found the bug and how to reproduce the failure. Critically, IIUC, this requires > > analyzer-null-dereference, which AFAIK isn't even enabled by W=1, let alone a base > > build. > > > > Please see the 0-day bot's reports[*] for a fantastic example of how to report > > things that are found by non-standard (by kernel standards) means. > > > > In general, I suspect that analyzer-null-dereference will generate a _lot_ of > > false positives, and is probably not worth reporting unless you are absolutely > > 100% certain there's a real bug. I (and most maintainers) am happy to deal with > > false positives here and there _if_ the signal to noise ratio is high. But if > > most reports are false positives, they'll likely all end up getting ignored. > > > > [*] https://lore.kernel.org/all/202406111250.d8XtA9SC-lkp@xxxxxxxxx > > I think I understood the meaning between the lines. > > However, to repeat the obvious, reducing the global dependencies simplifies the readability > and the logical proof of the code. :-/ Comments would also help. :) > Needless to say, dividing into pure functions and const functions reduces the number of > dependencies, as it is N × (N - 1), sqr (N). > > For example, if a condition is always true, but the compiler cannot deduce it from code, > there is something odd. > > CONCLUSION: If this generated 5 out of 5 false positives, then I might be giving up on this > as a waste of your time. > > However, it was great fun analysing x86 KVM code. :-) I assure you that there are plenty of actual bugs in KVM. This tool just isn't finding them. > Sort of cool that you guys on Google consider bug report from nobody admins from the > universities ;-) > > Best regards, > Mirsad Todorovac >
