linux-kernel - Re: [PATCH 2/2] xen: rename wrong named pfn related variables

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: <4a5d8b43-7ffa-cbcd-943c-3c459d56e55e@suse.com>
Date:   Mon, 16 Aug 2021 07:25:24 +0200
From:   Juergen Gross <jgross@...e.com>
To:     Jan Beulich <jbeulich@...e.com>
Cc:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, xen-devel@...ts.xenproject.org,
        linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH 2/2] xen: rename wrong named pfn related variables

On 03.08.21 12:42, Jan Beulich wrote:
> On 30.07.2021 11:00, Juergen Gross wrote:
>> On 16.06.21 12:43, Juergen Gross wrote:
>>> On 16.06.21 11:56, Jan Beulich wrote:
>>>> On 16.06.2021 09:30, Juergen Gross wrote:
>>>>> --- a/arch/x86/xen/p2m.c
>>>>> +++ b/arch/x86/xen/p2m.c
>>>>> @@ -95,8 +95,8 @@ unsigned long *xen_p2m_addr __read_mostly;
>>>>>    EXPORT_SYMBOL_GPL(xen_p2m_addr);
>>>>>    unsigned long xen_p2m_size __read_mostly;
>>>>>    EXPORT_SYMBOL_GPL(xen_p2m_size);
>>>>> -unsigned long xen_max_p2m_pfn __read_mostly;
>>>>> -EXPORT_SYMBOL_GPL(xen_max_p2m_pfn);
>>>>> +unsigned long xen_p2m_max_size __read_mostly;
>>>>> +EXPORT_SYMBOL_GPL(xen_p2m_max_size);
>>>>
>>>> Instead of renaming the exported variable (which will break consumers
>>>> anyway), how about dropping the apparently unneeded export at this
>>>> occasion?
>>>
>>> Why do you think it isn't needed? It is being referenced via the inline
>>> function __pfn_to_mfn() in arch/x86/include/asm/xen/page.h. And
>>> __pfn_to_mfn() is used via lots of other inline functions and macros.
>>>
>>>> Further it looks to me as if xen_p2m_size and this variable
>>>> were actually always kept in sync, so I'd like to put up the question
>>>> of dropping one of the two.
>>>
>>> Hmm, should be possible, yes.
>>
>> Looking into this it seems this is not possible.
>>
>> xen_p2m_size always holds the number of p2m entries in the p2m table,
>> including invalid ones at the end. xen_p2m_pfn_limit however contains
>> the (rounded up) index after the last valid p2m entry.
> 
> I'm afraid I can't follow:
> 
> xen_build_dynamic_phys_to_machine() sets xen_p2m_size and then syncs
> its value to what so far has been xen_max_p2m_pfn.
> 
> xen_vmalloc_p2m_tree() sets xen_max_p2m_pfn and then syncs its value
> to xen_p2m_size.
> 
> I therefore can't see how the two values would hold different values,
> except for the brief periods between updating one and then the other.

The brief period in xen_vmalloc_p2m_tree() is the problematic one. The
different values are especially important for the calls of
__pfn_to_mfn() during xen_rebuild_p2m_list().


Juergen

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

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