linux-kernel - Re: [PATCH] x86/xen: silence smatch warning in pmu_msr_chk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <69f4f7dd-e630-709d-5aa9-334e349109d9@suse.com>
Date:   Thu, 20 Oct 2022 16:33:00 +0200
From:   Juergen Gross <jgross@...e.com>
To:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Jan Beulich <jbeulich@...e.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>, xen-devel@...ts.xenproject.org,
        Dan Carpenter <dan.carpenter@...cle.com>,
        linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH] x86/xen: silence smatch warning in pmu_msr_chk_emulated()

On 20.10.22 16:22, Boris Ostrovsky wrote:
> 
> On 10/20/22 9:34 AM, Juergen Gross wrote:
>> On 20.10.22 15:16, Jan Beulich wrote:
>>> On 20.10.2022 13:37, Juergen Gross wrote:
>>>> Commit 8714f7bcd3c2 ("xen/pv: add fault recovery control to pmu msr
>>>> accesses") introduced code resulting in a warning issued by the smatch
>>>> static checker, claiming to use an uninitialized variable.
>>>>
>>>> This is a false positive, but work around the warning nevertheless.
>>>
>>> The risk of introducing a problem might be quite low here, but in general
>>> it exists: With the adjustment you remove any chance of the compiler
>>> spotting a missing initialization before use. And I'm not convinced using
>>> 0 in such a case would actually be ending up sufficiently benign.
>>
>> Hmm, an alternative would be to initialize it to -1 and add a test for the
>> index to be >= 0 before using it.
>>
>> Or to live with the smash warning with the chance, that a compiler might be
>> warning for the same reason in the future.
> 
> 
> Is smatch complaining about both variables or just index? There are two cases in 
> is_intel_pmu_msr() where it returns true but index is not set so perhaps that's 
> what bothers smatch? It shold not complain if is_intel_pmu_msr() returns false.

I didn't test it myself, so I can only speculate.

I guess the problem is when is_intel_pmu_msr() returns true.

In the end I don't think we expect much code churn in this area in the future.
Its not as if the pmu handling for PV guests is expected to be extended.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)