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: <4fb4b087-cae2-4516-a34e-cb4c72be13eb@intel.com>
Date: Fri, 21 Jun 2024 07:51:26 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Peter Xu <peterx@...hat.com>, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org
Cc: x86@...nel.org, Borislav Petkov <bp@...en8.de>,
 Dave Jiang <dave.jiang@...el.com>, "Kirill A . Shutemov"
 <kirill@...temov.name>, Ingo Molnar <mingo@...hat.com>,
 Oscar Salvador <osalvador@...e.de>, Matthew Wilcox <willy@...radead.org>,
 Vlastimil Babka <vbabka@...e.cz>, Dan Williams <dan.j.williams@...el.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Hugh Dickins <hughd@...gle.com>,
 Michael Ellerman <mpe@...erman.id.au>,
 Dave Hansen <dave.hansen@...ux.intel.com>,
 Thomas Gleixner <tglx@...utronix.de>, linuxppc-dev@...ts.ozlabs.org,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 Rik van Riel <riel@...riel.com>, Mel Gorman <mgorman@...hsingularity.net>,
 "Aneesh Kumar K . V" <aneesh.kumar@...ux.ibm.com>,
 Nicholas Piggin <npiggin@...il.com>, Huang Ying <ying.huang@...el.com>,
 "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
Subject: Re: [PATCH 6/7] mm/x86: Add missing pud helpers

On 6/21/24 07:25, Peter Xu wrote:
> These new helpers will be needed for pud entry updates soon.  Namely:
> 
> - pudp_invalidate()
> - pud_modify()

I think it's also definitely worth noting where you got this code from.
Presumably you copied, pasted and modified the PMD code.  That's fine,
but it should be called out.

...
> +static inline pud_t pud_modify(pud_t pud, pgprot_t newprot)
> +{
> +	pudval_t val = pud_val(pud), oldval = val;
> +
> +	/*
> +	 * NOTE: no need to consider shadow stack complexities because it
> +	 * doesn't support 1G mappings.
> +	 */
> +	val &= _HPAGE_CHG_MASK;
> +	val |= check_pgprot(newprot) & ~_HPAGE_CHG_MASK;
> +	val = flip_protnone_guard(oldval, val, PHYSICAL_PUD_PAGE_MASK);
> +
> +	return __pud(val);
> +}

First of all, the comment to explain what you didn't do here is as many
lines as the code to _actually_ implement it.

Second, I believe this might have missed the purpose of the "shadow
stack complexities".  The pmd/pte code is there not to support modifying
shadow stack mappings, it's there to avoid inadvertent shadow stack
mapping creation.

That "NOTE:" is ambiguous as to whether the shadow stacks aren't
supported on 1G mappings in Linux or the hardware (I just checked the
hardware docs and don't see anything making 1G mappings special, btw).

But, still, what if you take a Dirty=1,Write=1 pud and pud_modify() it
to make it Dirty=1,Write=0?  What prevents that from being
misinterpreted by the hardware as being a valid 1G shadow stack mapping?

>  /*
>   * mprotect needs to preserve PAT and encryption bits when updating
>   * vm_page_prot
> @@ -1377,10 +1398,25 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
>  }
>  #endif
>  
> +static inline pud_t pudp_establish(struct vm_area_struct *vma,
> +		unsigned long address, pud_t *pudp, pud_t pud)
> +{
> +	if (IS_ENABLED(CONFIG_SMP)) {
> +		return xchg(pudp, pud);
> +	} else {
> +		pud_t old = *pudp;
> +		WRITE_ONCE(*pudp, pud);
> +		return old;
> +	}
> +}

Why is there no:

	page_table_check_pud_set(vma->vm_mm, pudp, pud);

?  Sure, it doesn't _do_ anything today.  But the PMD code has it today.
 So leaving it out creates a divergence that honestly can only serve to
bite us in the future and will create a head-scratching delta for anyone
that is comparing PUD and PMD implementations in the future.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ