Subject: [tip: x86/urgent] x86/mm: Avoid using set_pgd() outside of real PGD pages

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: d082d48737c75d2b3cc1f972b8c8674c25131534
Gitweb: https://git.kernel.org/tip/d082d48737c75d2b3cc1f972b8c8674c25131534
Author: Lee Jones <[email protected]>
AuthorDate: Wed, 14 Jun 2023 17:38:54 +01:00
Committer: Dave Hansen <[email protected]>
CommitterDate: Fri, 16 Jun 2023 11:46:42 -07:00

x86/mm: Avoid using set_pgd() outside of real PGD pages

KPTI keeps around two PGDs: one for userspace and another for the
kernel. Among other things, set_pgd() contains infrastructure to
ensure that updates to the kernel PGD are reflected in the user PGD
as well.

One side-effect of this is that set_pgd() expects to be passed whole
pages. Unfortunately, init_trampoline_kaslr() passes in a single entry:
'trampoline_pgd_entry'.

When KPTI is on, set_pgd() will update 'trampoline_pgd_entry' (an
8-Byte globally stored [.bss] variable) and will then proceed to
replicate that value into the non-existent neighboring user page
(located +4k away), leading to the corruption of other global [.bss]
stored variables.

Fix it by directly assigning 'trampoline_pgd_entry' and avoiding
set_pgd().

[ dhansen: tweak subject and changelog ]

Fixes: 0925dda5962e ("x86/mm/KASLR: Use only one PUD entry for real mode trampoline")
Suggested-by: Dave Hansen <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
Signed-off-by: Dave Hansen <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/g
---
arch/x86/mm/kaslr.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index 557f0fe..37db264 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -172,10 +172,10 @@ void __meminit init_trampoline_kaslr(void)
set_p4d(p4d_tramp,
__p4d(_KERNPG_TABLE | __pa(pud_page_tramp)));

- set_pgd(&trampoline_pgd_entry,
- __pgd(_KERNPG_TABLE | __pa(p4d_page_tramp)));
+ trampoline_pgd_entry =
+ __pgd(_KERNPG_TABLE | __pa(p4d_page_tramp));
} else {
- set_pgd(&trampoline_pgd_entry,
- __pgd(_KERNPG_TABLE | __pa(pud_page_tramp)));
+ trampoline_pgd_entry =
+ __pgd(_KERNPG_TABLE | __pa(pud_page_tramp));
}
}


2023-06-26 09:12:13

by Greg KH

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/mm: Avoid using set_pgd() outside of real PGD pages

On Mon, Jun 26, 2023 at 09:54:50AM +0100, Lee Jones wrote:
> Dear Stable,
>
> On Fri, 16 Jun 2023, tip-bot2 for Lee Jones wrote:
>
> > The following commit has been merged into the x86/urgent branch of tip:
> >
> > Commit-ID: d082d48737c75d2b3cc1f972b8c8674c25131534
> > Gitweb: https://git.kernel.org/tip/d082d48737c75d2b3cc1f972b8c8674c25131534
> > Author: Lee Jones <[email protected]>
> > AuthorDate: Wed, 14 Jun 2023 17:38:54 +01:00
> > Committer: Dave Hansen <[email protected]>
> > CommitterDate: Fri, 16 Jun 2023 11:46:42 -07:00
> >
> > x86/mm: Avoid using set_pgd() outside of real PGD pages
> >
> > KPTI keeps around two PGDs: one for userspace and another for the
> > kernel. Among other things, set_pgd() contains infrastructure to
> > ensure that updates to the kernel PGD are reflected in the user PGD
> > as well.
> >
> > One side-effect of this is that set_pgd() expects to be passed whole
> > pages. Unfortunately, init_trampoline_kaslr() passes in a single entry:
> > 'trampoline_pgd_entry'.
> >
> > When KPTI is on, set_pgd() will update 'trampoline_pgd_entry' (an
> > 8-Byte globally stored [.bss] variable) and will then proceed to
> > replicate that value into the non-existent neighboring user page
> > (located +4k away), leading to the corruption of other global [.bss]
> > stored variables.
> >
> > Fix it by directly assigning 'trampoline_pgd_entry' and avoiding
> > set_pgd().
> >
> > [ dhansen: tweak subject and changelog ]
> >
> > Fixes: 0925dda5962e ("x86/mm/KASLR: Use only one PUD entry for real mode trampoline")
> > Suggested-by: Dave Hansen <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > Signed-off-by: Dave Hansen <[email protected]>
> > Cc: <[email protected]>
> > Link: https://lore.kernel.org/all/[email protected]/g
> > ---
> > arch/x86/mm/kaslr.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> > index 557f0fe..37db264 100644
> > --- a/arch/x86/mm/kaslr.c
> > +++ b/arch/x86/mm/kaslr.c
> > @@ -172,10 +172,10 @@ void __meminit init_trampoline_kaslr(void)
> > set_p4d(p4d_tramp,
> > __p4d(_KERNPG_TABLE | __pa(pud_page_tramp)));
> >
> > - set_pgd(&trampoline_pgd_entry,
> > - __pgd(_KERNPG_TABLE | __pa(p4d_page_tramp)));
> > + trampoline_pgd_entry =
> > + __pgd(_KERNPG_TABLE | __pa(p4d_page_tramp));
> > } else {
> > - set_pgd(&trampoline_pgd_entry,
> > - __pgd(_KERNPG_TABLE | __pa(pud_page_tramp)));
> > + trampoline_pgd_entry =
> > + __pgd(_KERNPG_TABLE | __pa(pud_page_tramp));
> > }
> > }
>
> Could we have this expedited please? There are users waiting for it.
>
> Upstream commit is:
>
> d082d48737c75 ("x86/mm: Avoid using set_pgd() outside of real PGD pages")

Please look through your emails you got this weekend, it's already
queued up in the following stable trees:
queue-5.4 queue-5.10 queue-5.15 queue-6.1 queue-6.3

greg k-h

2023-06-26 09:15:17

by Lee Jones

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/mm: Avoid using set_pgd() outside of real PGD pages

Dear Stable,

On Fri, 16 Jun 2023, tip-bot2 for Lee Jones wrote:

> The following commit has been merged into the x86/urgent branch of tip:
>
> Commit-ID: d082d48737c75d2b3cc1f972b8c8674c25131534
> Gitweb: https://git.kernel.org/tip/d082d48737c75d2b3cc1f972b8c8674c25131534
> Author: Lee Jones <[email protected]>
> AuthorDate: Wed, 14 Jun 2023 17:38:54 +01:00
> Committer: Dave Hansen <[email protected]>
> CommitterDate: Fri, 16 Jun 2023 11:46:42 -07:00
>
> x86/mm: Avoid using set_pgd() outside of real PGD pages
>
> KPTI keeps around two PGDs: one for userspace and another for the
> kernel. Among other things, set_pgd() contains infrastructure to
> ensure that updates to the kernel PGD are reflected in the user PGD
> as well.
>
> One side-effect of this is that set_pgd() expects to be passed whole
> pages. Unfortunately, init_trampoline_kaslr() passes in a single entry:
> 'trampoline_pgd_entry'.
>
> When KPTI is on, set_pgd() will update 'trampoline_pgd_entry' (an
> 8-Byte globally stored [.bss] variable) and will then proceed to
> replicate that value into the non-existent neighboring user page
> (located +4k away), leading to the corruption of other global [.bss]
> stored variables.
>
> Fix it by directly assigning 'trampoline_pgd_entry' and avoiding
> set_pgd().
>
> [ dhansen: tweak subject and changelog ]
>
> Fixes: 0925dda5962e ("x86/mm/KASLR: Use only one PUD entry for real mode trampoline")
> Suggested-by: Dave Hansen <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> Signed-off-by: Dave Hansen <[email protected]>
> Cc: <[email protected]>
> Link: https://lore.kernel.org/all/[email protected]/g
> ---
> arch/x86/mm/kaslr.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index 557f0fe..37db264 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -172,10 +172,10 @@ void __meminit init_trampoline_kaslr(void)
> set_p4d(p4d_tramp,
> __p4d(_KERNPG_TABLE | __pa(pud_page_tramp)));
>
> - set_pgd(&trampoline_pgd_entry,
> - __pgd(_KERNPG_TABLE | __pa(p4d_page_tramp)));
> + trampoline_pgd_entry =
> + __pgd(_KERNPG_TABLE | __pa(p4d_page_tramp));
> } else {
> - set_pgd(&trampoline_pgd_entry,
> - __pgd(_KERNPG_TABLE | __pa(pud_page_tramp)));
> + trampoline_pgd_entry =
> + __pgd(_KERNPG_TABLE | __pa(pud_page_tramp));
> }
> }

Could we have this expedited please? There are users waiting for it.

Upstream commit is:

d082d48737c75 ("x86/mm: Avoid using set_pgd() outside of real PGD pages")

Thanks muchly.

--
Lee Jones [李琼斯]

2023-06-26 09:21:32

by Lee Jones

[permalink] [raw]
Subject: Re: [tip: x86/urgent] x86/mm: Avoid using set_pgd() outside of real PGD pages

On Mon, 26 Jun 2023, Greg KH wrote:

> On Mon, Jun 26, 2023 at 09:54:50AM +0100, Lee Jones wrote:
> > Dear Stable,
> >
> > On Fri, 16 Jun 2023, tip-bot2 for Lee Jones wrote:
> >
> > > The following commit has been merged into the x86/urgent branch of tip:
> > >
> > > Commit-ID: d082d48737c75d2b3cc1f972b8c8674c25131534
> > > Gitweb: https://git.kernel.org/tip/d082d48737c75d2b3cc1f972b8c8674c25131534
> > > Author: Lee Jones <[email protected]>
> > > AuthorDate: Wed, 14 Jun 2023 17:38:54 +01:00
> > > Committer: Dave Hansen <[email protected]>
> > > CommitterDate: Fri, 16 Jun 2023 11:46:42 -07:00
> > >
> > > x86/mm: Avoid using set_pgd() outside of real PGD pages
> > >
> > > KPTI keeps around two PGDs: one for userspace and another for the
> > > kernel. Among other things, set_pgd() contains infrastructure to
> > > ensure that updates to the kernel PGD are reflected in the user PGD
> > > as well.
> > >
> > > One side-effect of this is that set_pgd() expects to be passed whole
> > > pages. Unfortunately, init_trampoline_kaslr() passes in a single entry:
> > > 'trampoline_pgd_entry'.
> > >
> > > When KPTI is on, set_pgd() will update 'trampoline_pgd_entry' (an
> > > 8-Byte globally stored [.bss] variable) and will then proceed to
> > > replicate that value into the non-existent neighboring user page
> > > (located +4k away), leading to the corruption of other global [.bss]
> > > stored variables.
> > >
> > > Fix it by directly assigning 'trampoline_pgd_entry' and avoiding
> > > set_pgd().
> > >
> > > [ dhansen: tweak subject and changelog ]
> > >
> > > Fixes: 0925dda5962e ("x86/mm/KASLR: Use only one PUD entry for real mode trampoline")
> > > Suggested-by: Dave Hansen <[email protected]>
> > > Signed-off-by: Lee Jones <[email protected]>
> > > Signed-off-by: Dave Hansen <[email protected]>
> > > Cc: <[email protected]>
> > > Link: https://lore.kernel.org/all/[email protected]/g
> > > ---
> > > arch/x86/mm/kaslr.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> > > index 557f0fe..37db264 100644
> > > --- a/arch/x86/mm/kaslr.c
> > > +++ b/arch/x86/mm/kaslr.c
> > > @@ -172,10 +172,10 @@ void __meminit init_trampoline_kaslr(void)
> > > set_p4d(p4d_tramp,
> > > __p4d(_KERNPG_TABLE | __pa(pud_page_tramp)));
> > >
> > > - set_pgd(&trampoline_pgd_entry,
> > > - __pgd(_KERNPG_TABLE | __pa(p4d_page_tramp)));
> > > + trampoline_pgd_entry =
> > > + __pgd(_KERNPG_TABLE | __pa(p4d_page_tramp));
> > > } else {
> > > - set_pgd(&trampoline_pgd_entry,
> > > - __pgd(_KERNPG_TABLE | __pa(pud_page_tramp)));
> > > + trampoline_pgd_entry =
> > > + __pgd(_KERNPG_TABLE | __pa(pud_page_tramp));
> > > }
> > > }
> >
> > Could we have this expedited please? There are users waiting for it.
> >
> > Upstream commit is:
> >
> > d082d48737c75 ("x86/mm: Avoid using set_pgd() outside of real PGD pages")
>
> Please look through your emails you got this weekend, it's already
> queued up in the following stable trees:
> queue-5.4 queue-5.10 queue-5.15 queue-6.1 queue-6.3

Haven't reached those ones yet - only 160 to go!

Thank you though, very much appreciated.

--
Lee Jones [李琼斯]