linux-kernel - Re: [PATCH v10 16/50] x86/sev: Introduce snp leaked pages list

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: <b1b0decf-dc0b-b1bb-db9d-2a00a8c81b0d@suse.cz>
Date:   Mon, 11 Dec 2023 14:08:37 +0100
From:   Vlastimil Babka <vbabka@...e.cz>
To:     "Kalra, Ashish" <ashish.kalra@....com>,
        Michael Roth <michael.roth@....com>, kvm@...r.kernel.org
Cc:     linux-coco@...ts.linux.dev, linux-mm@...ck.org,
        linux-crypto@...r.kernel.org, x86@...nel.org,
        linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
        jroedel@...e.de, thomas.lendacky@....com, hpa@...or.com,
        ardb@...nel.org, pbonzini@...hat.com, seanjc@...gle.com,
        vkuznets@...hat.com, jmattson@...gle.com, luto@...nel.org,
        dave.hansen@...ux.intel.com, slp@...hat.com, pgonda@...gle.com,
        peterz@...radead.org, srinivas.pandruvada@...ux.intel.com,
        rientjes@...gle.com, dovmurik@...ux.ibm.com, tobin@....com,
        bp@...en8.de, kirill@...temov.name, ak@...ux.intel.com,
        tony.luck@...el.com, marcorr@...gle.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com, alpergun@...gle.com,
        jarkko@...nel.org, nikunj.dadhania@....com, pankaj.gupta@....com,
        liam.merwick@...cle.com, zhi.a.wang@...el.com
Subject: Re: [PATCH v10 16/50] x86/sev: Introduce snp leaked pages list



On 12/8/23 23:10, Kalra, Ashish wrote:
> Hello Vlastimil,
> 
> On 12/7/2023 10:20 AM, Vlastimil Babka wrote:
> 
>>> +
>>> +void snp_leak_pages(u64 pfn, unsigned int npages)
>>> +{
>>> +    struct page *page = pfn_to_page(pfn);
>>> +
>>> +    pr_debug("%s: leaking PFN range 0x%llx-0x%llx\n", __func__, pfn,
>>> pfn + npages);
>>> +
>>> +    spin_lock(&snp_leaked_pages_list_lock);
>>> +    while (npages--) {
>>> +        /*
>>> +         * Reuse the page's buddy list for chaining into the leaked
>>> +         * pages list. This page should not be on a free list currently
>>> +         * and is also unsafe to be added to a free list.
>>> +         */
>>> +        list_add_tail(&page->buddy_list, &snp_leaked_pages_list);
>>> +        sev_dump_rmpentry(pfn);
>>> +        pfn++;
>>
>> You increment pfn, but not page, which is always pointing to the page
>> of the
>> initial pfn, so need to do page++ too.
> 
> Yes, that is a bug and needs to be fixed.
> 
>> But that assumes it's all order-0 pages (hard to tell for me whether
>> that's
>> true as we start with a pfn), if there can be compound pages, it would be
>> best to only add the head page and skip the tail pages - it's not
>> expected
>> to use page->buddy_list of tail pages.
> 
> Can't we use PageCompound() to check if the page is a compound page and
> then use page->compound_head to get and add the head page to leaked
> pages list. I understand the tail pages for compound pages are really
> limited for usage.

Yeah that should work. Need to be careful though, should probably only
process head pages and check if the whole compound_order() is within the
range we are to leak, and then leak the head page and advance the loop
by compound_order(). And if we encounter a tail page, it should probably
be just skipped. I'm looking at snp_reclaim_pages() which seems to
process a number of pages with SEV_CMD_SNP_PAGE_RECLAIM and once any
fails, call snp_leak_pages() on the rest. Could that invoke
snp_leak_pages with the first pfn being a tail page?

> Thanks,
> Ashish