* [PATCH] arm64: mm: mark tramp_pg_dir read-only @ 2018-05-30 4:48 YaoJun 2018-05-30 9:14 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: YaoJun @ 2018-05-30 4:48 UTC (permalink / raw) To: linux-arm-kernel To protect against KSMA(Kernel Space Mirroring Attack), make tramp_pg_dir read-only. The principle of KSMA is to insert a carefully constructed PGD entry into the translation table. The type of this entry is block, which maps the kernel text and its access permissions bits are 01. The user process can then modify kernel text directly through this mapping. In this way, an arbitrary write can be converted to multiple arbitrary writes. Signed-off-by: YaoJun <yaojun8558363@gmail.com> --- arch/arm64/mm/mmu.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index 2dbb2c9f1ec1..ac4b22c7e435 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, prot, pgd_pgtable_alloc, 0); + update_mapping_prot(__pa_symbol(tramp_pg_dir), + (unsigned long)tramp_pg_dir, + PGD_SIZE, PAGE_KERNEL_RO); + /* Map both the text and data into the kernel page table */ __set_fixmap(FIX_ENTRY_TRAMP_TEXT, pa_start, prot); if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) { -- 2.17.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-05-30 4:48 [PATCH] arm64: mm: mark tramp_pg_dir read-only YaoJun @ 2018-05-30 9:14 ` Will Deacon 2018-05-30 9:53 ` Ard Biesheuvel 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2018-05-30 9:14 UTC (permalink / raw) To: linux-arm-kernel On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: > To protect against KSMA(Kernel Space Mirroring Attack), make > tramp_pg_dir read-only. The principle of KSMA is to insert a > carefully constructed PGD entry into the translation table. > The type of this entry is block, which maps the kernel text > and its access permissions bits are 01. The user process can > then modify kernel text directly through this mapping. In this > way, an arbitrary write can be converted to multiple arbitrary > writes. > > Signed-off-by: YaoJun <yaojun8558363@gmail.com> > --- > arch/arm64/mm/mmu.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index 2dbb2c9f1ec1..ac4b22c7e435 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) > __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, > prot, pgd_pgtable_alloc, 0); > > + update_mapping_prot(__pa_symbol(tramp_pg_dir), > + (unsigned long)tramp_pg_dir, > + PGD_SIZE, PAGE_KERNEL_RO); Hmm, I like the idea but is there a risk that the page table has been mapped as part of a block entry, which we can't safely split at this point (i.e. we'll run into one of the BUG_ONs in the mapping code)? Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-05-30 9:14 ` Will Deacon @ 2018-05-30 9:53 ` Ard Biesheuvel 2018-06-19 15:20 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Ard Biesheuvel @ 2018-05-30 9:53 UTC (permalink / raw) To: linux-arm-kernel On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: >> To protect against KSMA(Kernel Space Mirroring Attack), make >> tramp_pg_dir read-only. The principle of KSMA is to insert a >> carefully constructed PGD entry into the translation table. >> The type of this entry is block, which maps the kernel text >> and its access permissions bits are 01. The user process can >> then modify kernel text directly through this mapping. In this >> way, an arbitrary write can be converted to multiple arbitrary >> writes. >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> >> --- >> arch/arm64/mm/mmu.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 >> --- a/arch/arm64/mm/mmu.c >> +++ b/arch/arm64/mm/mmu.c >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, >> prot, pgd_pgtable_alloc, 0); >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), >> + (unsigned long)tramp_pg_dir, >> + PGD_SIZE, PAGE_KERNEL_RO); > > Hmm, I like the idea but is there a risk that the page table has been mapped > as part of a block entry, which we can't safely split at this point (i.e. > we'll run into one of the BUG_ONs in the mapping code)? > We'd need to create a separate segment for it initially so the mapping is already at the right granularity. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-05-30 9:53 ` Ard Biesheuvel @ 2018-06-19 15:20 ` Will Deacon 2018-06-19 15:23 ` Ard Biesheuvel 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2018-06-19 15:20 UTC (permalink / raw) To: linux-arm-kernel Hi Ard, Sorry, I forgot to reply to this. On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: > On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: > > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: > >> To protect against KSMA(Kernel Space Mirroring Attack), make > >> tramp_pg_dir read-only. The principle of KSMA is to insert a > >> carefully constructed PGD entry into the translation table. > >> The type of this entry is block, which maps the kernel text > >> and its access permissions bits are 01. The user process can > >> then modify kernel text directly through this mapping. In this > >> way, an arbitrary write can be converted to multiple arbitrary > >> writes. > >> > >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> > >> --- > >> arch/arm64/mm/mmu.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 > >> --- a/arch/arm64/mm/mmu.c > >> +++ b/arch/arm64/mm/mmu.c > >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) > >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, > >> prot, pgd_pgtable_alloc, 0); > >> > >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), > >> + (unsigned long)tramp_pg_dir, > >> + PGD_SIZE, PAGE_KERNEL_RO); > > > > Hmm, I like the idea but is there a risk that the page table has been mapped > > as part of a block entry, which we can't safely split at this point (i.e. > > we'll run into one of the BUG_ONs in the mapping code)? > > > > We'd need to create a separate segment for it initially so the mapping > is already at the right granularity. Why do you think that's the case? I can't see anything that guarantees this for the page table itself. Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:20 ` Will Deacon @ 2018-06-19 15:23 ` Ard Biesheuvel 2018-06-19 15:28 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Ard Biesheuvel @ 2018-06-19 15:23 UTC (permalink / raw) To: linux-arm-kernel On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: > Hi Ard, > > Sorry, I forgot to reply to this. > > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: >> >> To protect against KSMA(Kernel Space Mirroring Attack), make >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a >> >> carefully constructed PGD entry into the translation table. >> >> The type of this entry is block, which maps the kernel text >> >> and its access permissions bits are 01. The user process can >> >> then modify kernel text directly through this mapping. In this >> >> way, an arbitrary write can be converted to multiple arbitrary >> >> writes. >> >> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> >> >> --- >> >> arch/arm64/mm/mmu.c | 4 ++++ >> >> 1 file changed, 4 insertions(+) >> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 >> >> --- a/arch/arm64/mm/mmu.c >> >> +++ b/arch/arm64/mm/mmu.c >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, >> >> prot, pgd_pgtable_alloc, 0); >> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), >> >> + (unsigned long)tramp_pg_dir, >> >> + PGD_SIZE, PAGE_KERNEL_RO); >> > >> > Hmm, I like the idea but is there a risk that the page table has been mapped >> > as part of a block entry, which we can't safely split at this point (i.e. >> > we'll run into one of the BUG_ONs in the mapping code)? >> > >> >> We'd need to create a separate segment for it initially so the mapping >> is already at the right granularity. > > Why do you think that's the case? I can't see anything that guarantees this > for the page table itself. > We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), obviously, but that shouldn't hurt since that segment is relatively tiny anyway. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:23 ` Ard Biesheuvel @ 2018-06-19 15:28 ` Will Deacon 2018-06-19 15:29 ` Ard Biesheuvel 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2018-06-19 15:28 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote: > On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: > > Hi Ard, > > > > Sorry, I forgot to reply to this. > > > > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: > >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: > >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: > >> >> To protect against KSMA(Kernel Space Mirroring Attack), make > >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a > >> >> carefully constructed PGD entry into the translation table. > >> >> The type of this entry is block, which maps the kernel text > >> >> and its access permissions bits are 01. The user process can > >> >> then modify kernel text directly through this mapping. In this > >> >> way, an arbitrary write can be converted to multiple arbitrary > >> >> writes. > >> >> > >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> > >> >> --- > >> >> arch/arm64/mm/mmu.c | 4 ++++ > >> >> 1 file changed, 4 insertions(+) > >> >> > >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 > >> >> --- a/arch/arm64/mm/mmu.c > >> >> +++ b/arch/arm64/mm/mmu.c > >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) > >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, > >> >> prot, pgd_pgtable_alloc, 0); > >> >> > >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), > >> >> + (unsigned long)tramp_pg_dir, > >> >> + PGD_SIZE, PAGE_KERNEL_RO); > >> > > >> > Hmm, I like the idea but is there a risk that the page table has been mapped > >> > as part of a block entry, which we can't safely split at this point (i.e. > >> > we'll run into one of the BUG_ONs in the mapping code)? > >> > > >> > >> We'd need to create a separate segment for it initially so the mapping > >> is already at the right granularity. > > > > Why do you think that's the case? I can't see anything that guarantees this > > for the page table itself. > > > > We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), > obviously, but that shouldn't hurt since that segment is relatively > tiny anyway. Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good. Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata... Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:28 ` Will Deacon @ 2018-06-19 15:29 ` Ard Biesheuvel 2018-06-19 15:37 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Ard Biesheuvel @ 2018-06-19 15:29 UTC (permalink / raw) To: linux-arm-kernel On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote: > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote: >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: >> > Hi Ard, >> > >> > Sorry, I forgot to reply to this. >> > >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: >> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make >> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a >> >> >> carefully constructed PGD entry into the translation table. >> >> >> The type of this entry is block, which maps the kernel text >> >> >> and its access permissions bits are 01. The user process can >> >> >> then modify kernel text directly through this mapping. In this >> >> >> way, an arbitrary write can be converted to multiple arbitrary >> >> >> writes. >> >> >> >> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> >> >> >> --- >> >> >> arch/arm64/mm/mmu.c | 4 ++++ >> >> >> 1 file changed, 4 insertions(+) >> >> >> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 >> >> >> --- a/arch/arm64/mm/mmu.c >> >> >> +++ b/arch/arm64/mm/mmu.c >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, >> >> >> prot, pgd_pgtable_alloc, 0); >> >> >> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), >> >> >> + (unsigned long)tramp_pg_dir, >> >> >> + PGD_SIZE, PAGE_KERNEL_RO); >> >> > >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped >> >> > as part of a block entry, which we can't safely split at this point (i.e. >> >> > we'll run into one of the BUG_ONs in the mapping code)? >> >> > >> >> >> >> We'd need to create a separate segment for it initially so the mapping >> >> is already at the right granularity. >> > >> > Why do you think that's the case? I can't see anything that guarantees this >> > for the page table itself. >> > >> >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), >> obviously, but that shouldn't hurt since that segment is relatively >> tiny anyway. > > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good. > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata... > idmap and tramp yes, but swapper needs to be modifiable at runtime, no? ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:29 ` Ard Biesheuvel @ 2018-06-19 15:37 ` Will Deacon 2018-06-19 15:40 ` Ard Biesheuvel 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2018-06-19 15:37 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 19, 2018 at 05:29:03PM +0200, Ard Biesheuvel wrote: > On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote: > > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote: > >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: > >> > Hi Ard, > >> > > >> > Sorry, I forgot to reply to this. > >> > > >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: > >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: > >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: > >> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make > >> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a > >> >> >> carefully constructed PGD entry into the translation table. > >> >> >> The type of this entry is block, which maps the kernel text > >> >> >> and its access permissions bits are 01. The user process can > >> >> >> then modify kernel text directly through this mapping. In this > >> >> >> way, an arbitrary write can be converted to multiple arbitrary > >> >> >> writes. > >> >> >> > >> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> > >> >> >> --- > >> >> >> arch/arm64/mm/mmu.c | 4 ++++ > >> >> >> 1 file changed, 4 insertions(+) > >> >> >> > >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 > >> >> >> --- a/arch/arm64/mm/mmu.c > >> >> >> +++ b/arch/arm64/mm/mmu.c > >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) > >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, > >> >> >> prot, pgd_pgtable_alloc, 0); > >> >> >> > >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), > >> >> >> + (unsigned long)tramp_pg_dir, > >> >> >> + PGD_SIZE, PAGE_KERNEL_RO); > >> >> > > >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped > >> >> > as part of a block entry, which we can't safely split at this point (i.e. > >> >> > we'll run into one of the BUG_ONs in the mapping code)? > >> >> > > >> >> > >> >> We'd need to create a separate segment for it initially so the mapping > >> >> is already at the right granularity. > >> > > >> > Why do you think that's the case? I can't see anything that guarantees this > >> > for the page table itself. > >> > > >> > >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), > >> obviously, but that shouldn't hurt since that segment is relatively > >> tiny anyway. > > > > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good. > > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata... > > > > idmap and tramp yes, but swapper needs to be modifiable at runtime, no? Right, but couldn't we swizzle the permissions in e.g. set_pmd? We could even predicate that on a sanity check of the prot. Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:37 ` Will Deacon @ 2018-06-19 15:40 ` Ard Biesheuvel 2018-06-19 15:50 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Ard Biesheuvel @ 2018-06-19 15:40 UTC (permalink / raw) To: linux-arm-kernel On 19 June 2018 at 17:37, Will Deacon <will.deacon@arm.com> wrote: > On Tue, Jun 19, 2018 at 05:29:03PM +0200, Ard Biesheuvel wrote: >> On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote: >> > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote: >> >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: >> >> > Hi Ard, >> >> > >> >> > Sorry, I forgot to reply to this. >> >> > >> >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: >> >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: >> >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: >> >> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make >> >> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a >> >> >> >> carefully constructed PGD entry into the translation table. >> >> >> >> The type of this entry is block, which maps the kernel text >> >> >> >> and its access permissions bits are 01. The user process can >> >> >> >> then modify kernel text directly through this mapping. In this >> >> >> >> way, an arbitrary write can be converted to multiple arbitrary >> >> >> >> writes. >> >> >> >> >> >> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> >> >> >> >> --- >> >> >> >> arch/arm64/mm/mmu.c | 4 ++++ >> >> >> >> 1 file changed, 4 insertions(+) >> >> >> >> >> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 >> >> >> >> --- a/arch/arm64/mm/mmu.c >> >> >> >> +++ b/arch/arm64/mm/mmu.c >> >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) >> >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, >> >> >> >> prot, pgd_pgtable_alloc, 0); >> >> >> >> >> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), >> >> >> >> + (unsigned long)tramp_pg_dir, >> >> >> >> + PGD_SIZE, PAGE_KERNEL_RO); >> >> >> > >> >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped >> >> >> > as part of a block entry, which we can't safely split at this point (i.e. >> >> >> > we'll run into one of the BUG_ONs in the mapping code)? >> >> >> > >> >> >> >> >> >> We'd need to create a separate segment for it initially so the mapping >> >> >> is already at the right granularity. >> >> > >> >> > Why do you think that's the case? I can't see anything that guarantees this >> >> > for the page table itself. >> >> > >> >> >> >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), >> >> obviously, but that shouldn't hurt since that segment is relatively >> >> tiny anyway. >> > >> > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good. >> > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata... >> > >> >> idmap and tramp yes, but swapper needs to be modifiable at runtime, no? > > Right, but couldn't we swizzle the permissions in e.g. set_pmd? We could > even predicate that on a sanity check of the prot. > Swizzle the permissions of the entire .rodata segment? That sounds doable, but there is a whole class of data that belongs in this category, and I think PaX/grsecurity had an API for that (but I don't think anyone is upstreaming that yet). So let's not reinvent that wheel for swapper_pg_dir only. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:40 ` Ard Biesheuvel @ 2018-06-19 15:50 ` Will Deacon 2018-06-19 15:51 ` Ard Biesheuvel 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2018-06-19 15:50 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 19, 2018 at 05:40:26PM +0200, Ard Biesheuvel wrote: > On 19 June 2018 at 17:37, Will Deacon <will.deacon@arm.com> wrote: > > On Tue, Jun 19, 2018 at 05:29:03PM +0200, Ard Biesheuvel wrote: > >> On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote: > >> > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote: > >> >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: > >> >> > Hi Ard, > >> >> > > >> >> > Sorry, I forgot to reply to this. > >> >> > > >> >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: > >> >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: > >> >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: > >> >> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make > >> >> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a > >> >> >> >> carefully constructed PGD entry into the translation table. > >> >> >> >> The type of this entry is block, which maps the kernel text > >> >> >> >> and its access permissions bits are 01. The user process can > >> >> >> >> then modify kernel text directly through this mapping. In this > >> >> >> >> way, an arbitrary write can be converted to multiple arbitrary > >> >> >> >> writes. > >> >> >> >> > >> >> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> > >> >> >> >> --- > >> >> >> >> arch/arm64/mm/mmu.c | 4 ++++ > >> >> >> >> 1 file changed, 4 insertions(+) > >> >> >> >> > >> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > >> >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 > >> >> >> >> --- a/arch/arm64/mm/mmu.c > >> >> >> >> +++ b/arch/arm64/mm/mmu.c > >> >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) > >> >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, > >> >> >> >> prot, pgd_pgtable_alloc, 0); > >> >> >> >> > >> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), > >> >> >> >> + (unsigned long)tramp_pg_dir, > >> >> >> >> + PGD_SIZE, PAGE_KERNEL_RO); > >> >> >> > > >> >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped > >> >> >> > as part of a block entry, which we can't safely split at this point (i.e. > >> >> >> > we'll run into one of the BUG_ONs in the mapping code)? > >> >> >> > > >> >> >> > >> >> >> We'd need to create a separate segment for it initially so the mapping > >> >> >> is already at the right granularity. > >> >> > > >> >> > Why do you think that's the case? I can't see anything that guarantees this > >> >> > for the page table itself. > >> >> > > >> >> > >> >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), > >> >> obviously, but that shouldn't hurt since that segment is relatively > >> >> tiny anyway. > >> > > >> > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good. > >> > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata... > >> > > >> > >> idmap and tramp yes, but swapper needs to be modifiable at runtime, no? > > > > Right, but couldn't we swizzle the permissions in e.g. set_pmd? We could > > even predicate that on a sanity check of the prot. > > > > Swizzle the permissions of the entire .rodata segment? That sounds > doable, but there is a whole class of data that belongs in this > category, and I think PaX/grsecurity had an API for that (but I don't > think anyone is upstreaming that yet). So let's not reinvent that > wheel for swapper_pg_dir only. I wasn't thinking of the whole .rodata segment -- just the page containing the entry being modified, but ok. I was hoping to avoid getting involved with the PaX/grsec stuff as it seems rather toxic from what I've seen. Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:50 ` Will Deacon @ 2018-06-19 15:51 ` Ard Biesheuvel 2018-06-19 15:55 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Ard Biesheuvel @ 2018-06-19 15:51 UTC (permalink / raw) To: linux-arm-kernel On 19 June 2018 at 17:50, Will Deacon <will.deacon@arm.com> wrote: > On Tue, Jun 19, 2018 at 05:40:26PM +0200, Ard Biesheuvel wrote: >> On 19 June 2018 at 17:37, Will Deacon <will.deacon@arm.com> wrote: >> > On Tue, Jun 19, 2018 at 05:29:03PM +0200, Ard Biesheuvel wrote: >> >> On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote: >> >> > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote: >> >> >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: >> >> >> > Hi Ard, >> >> >> > >> >> >> > Sorry, I forgot to reply to this. >> >> >> > >> >> >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: >> >> >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: >> >> >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: >> >> >> >> >> To protect against KSMA(Kernel Space Mirroring Attack), make >> >> >> >> >> tramp_pg_dir read-only. The principle of KSMA is to insert a >> >> >> >> >> carefully constructed PGD entry into the translation table. >> >> >> >> >> The type of this entry is block, which maps the kernel text >> >> >> >> >> and its access permissions bits are 01. The user process can >> >> >> >> >> then modify kernel text directly through this mapping. In this >> >> >> >> >> way, an arbitrary write can be converted to multiple arbitrary >> >> >> >> >> writes. >> >> >> >> >> >> >> >> >> >> Signed-off-by: YaoJun <yaojun8558363@gmail.com> >> >> >> >> >> --- >> >> >> >> >> arch/arm64/mm/mmu.c | 4 ++++ >> >> >> >> >> 1 file changed, 4 insertions(+) >> >> >> >> >> >> >> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> >> >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 >> >> >> >> >> --- a/arch/arm64/mm/mmu.c >> >> >> >> >> +++ b/arch/arm64/mm/mmu.c >> >> >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) >> >> >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, >> >> >> >> >> prot, pgd_pgtable_alloc, 0); >> >> >> >> >> >> >> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), >> >> >> >> >> + (unsigned long)tramp_pg_dir, >> >> >> >> >> + PGD_SIZE, PAGE_KERNEL_RO); >> >> >> >> > >> >> >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped >> >> >> >> > as part of a block entry, which we can't safely split at this point (i.e. >> >> >> >> > we'll run into one of the BUG_ONs in the mapping code)? >> >> >> >> > >> >> >> >> >> >> >> >> We'd need to create a separate segment for it initially so the mapping >> >> >> >> is already at the right granularity. >> >> >> > >> >> >> > Why do you think that's the case? I can't see anything that guarantees this >> >> >> > for the page table itself. >> >> >> > >> >> >> >> >> >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), >> >> >> obviously, but that shouldn't hurt since that segment is relatively >> >> >> tiny anyway. >> >> > >> >> > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good. >> >> > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata... >> >> > >> >> >> >> idmap and tramp yes, but swapper needs to be modifiable at runtime, no? >> > >> > Right, but couldn't we swizzle the permissions in e.g. set_pmd? We could >> > even predicate that on a sanity check of the prot. >> > >> >> Swizzle the permissions of the entire .rodata segment? That sounds >> doable, but there is a whole class of data that belongs in this >> category, and I think PaX/grsecurity had an API for that (but I don't >> think anyone is upstreaming that yet). So let's not reinvent that >> wheel for swapper_pg_dir only. > > I wasn't thinking of the whole .rodata segment -- just the page containing > the entry being modified, but ok. That means we will need to map .rodata down to pages as well, or at least avoid contiguous mappings. > I was hoping to avoid getting involved > with the PaX/grsec stuff as it seems rather toxic from what I've seen. > Indeed. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:51 ` Ard Biesheuvel @ 2018-06-19 15:55 ` Will Deacon 2018-06-19 16:00 ` Ard Biesheuvel 0 siblings, 1 reply; 13+ messages in thread From: Will Deacon @ 2018-06-19 15:55 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 19, 2018 at 05:51:46PM +0200, Ard Biesheuvel wrote: > On 19 June 2018 at 17:50, Will Deacon <will.deacon@arm.com> wrote: > > On Tue, Jun 19, 2018 at 05:40:26PM +0200, Ard Biesheuvel wrote: > >> On 19 June 2018 at 17:37, Will Deacon <will.deacon@arm.com> wrote: > >> > On Tue, Jun 19, 2018 at 05:29:03PM +0200, Ard Biesheuvel wrote: > >> >> On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote: > >> >> > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote: > >> >> >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: > >> >> >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: > >> >> >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: > >> >> >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: > >> >> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > >> >> >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 > >> >> >> >> >> --- a/arch/arm64/mm/mmu.c > >> >> >> >> >> +++ b/arch/arm64/mm/mmu.c > >> >> >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) > >> >> >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, > >> >> >> >> >> prot, pgd_pgtable_alloc, 0); > >> >> >> >> >> > >> >> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), > >> >> >> >> >> + (unsigned long)tramp_pg_dir, > >> >> >> >> >> + PGD_SIZE, PAGE_KERNEL_RO); > >> >> >> >> > > >> >> >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped > >> >> >> >> > as part of a block entry, which we can't safely split at this point (i.e. > >> >> >> >> > we'll run into one of the BUG_ONs in the mapping code)? > >> >> >> >> > > >> >> >> >> > >> >> >> >> We'd need to create a separate segment for it initially so the mapping > >> >> >> >> is already at the right granularity. > >> >> >> > > >> >> >> > Why do you think that's the case? I can't see anything that guarantees this > >> >> >> > for the page table itself. > >> >> >> > > >> >> >> > >> >> >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), > >> >> >> obviously, but that shouldn't hurt since that segment is relatively > >> >> >> tiny anyway. > >> >> > > >> >> > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good. > >> >> > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata... > >> >> > > >> >> > >> >> idmap and tramp yes, but swapper needs to be modifiable at runtime, no? > >> > > >> > Right, but couldn't we swizzle the permissions in e.g. set_pmd? We could > >> > even predicate that on a sanity check of the prot. > >> > > >> > >> Swizzle the permissions of the entire .rodata segment? That sounds > >> doable, but there is a whole class of data that belongs in this > >> category, and I think PaX/grsecurity had an API for that (but I don't > >> think anyone is upstreaming that yet). So let's not reinvent that > >> wheel for swapper_pg_dir only. > > > > I wasn't thinking of the whole .rodata segment -- just the page containing > > the entry being modified, but ok. > > That means we will need to map .rodata down to pages as well, or at > least avoid contiguous mappings. Doesn't it already avoid those? Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] arm64: mm: mark tramp_pg_dir read-only 2018-06-19 15:55 ` Will Deacon @ 2018-06-19 16:00 ` Ard Biesheuvel 0 siblings, 0 replies; 13+ messages in thread From: Ard Biesheuvel @ 2018-06-19 16:00 UTC (permalink / raw) To: linux-arm-kernel On 19 June 2018 at 17:55, Will Deacon <will.deacon@arm.com> wrote: > On Tue, Jun 19, 2018 at 05:51:46PM +0200, Ard Biesheuvel wrote: >> On 19 June 2018 at 17:50, Will Deacon <will.deacon@arm.com> wrote: >> > On Tue, Jun 19, 2018 at 05:40:26PM +0200, Ard Biesheuvel wrote: >> >> On 19 June 2018 at 17:37, Will Deacon <will.deacon@arm.com> wrote: >> >> > On Tue, Jun 19, 2018 at 05:29:03PM +0200, Ard Biesheuvel wrote: >> >> >> On 19 June 2018 at 17:28, Will Deacon <will.deacon@arm.com> wrote: >> >> >> > On Tue, Jun 19, 2018 at 05:23:41PM +0200, Ard Biesheuvel wrote: >> >> >> >> On 19 June 2018 at 17:20, Will Deacon <will.deacon@arm.com> wrote: >> >> >> >> > On Wed, May 30, 2018 at 11:53:20AM +0200, Ard Biesheuvel wrote: >> >> >> >> >> On 30 May 2018 at 11:14, Will Deacon <will.deacon@arm.com> wrote: >> >> >> >> >> > On Wed, May 30, 2018 at 12:48:06PM +0800, YaoJun wrote: >> >> >> >> >> >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> >> >> >> >> >> index 2dbb2c9f1ec1..ac4b22c7e435 100644 >> >> >> >> >> >> --- a/arch/arm64/mm/mmu.c >> >> >> >> >> >> +++ b/arch/arm64/mm/mmu.c >> >> >> >> >> >> @@ -551,6 +551,10 @@ static int __init map_entry_trampoline(void) >> >> >> >> >> >> __create_pgd_mapping(tramp_pg_dir, pa_start, TRAMP_VALIAS, PAGE_SIZE, >> >> >> >> >> >> prot, pgd_pgtable_alloc, 0); >> >> >> >> >> >> >> >> >> >> >> >> + update_mapping_prot(__pa_symbol(tramp_pg_dir), >> >> >> >> >> >> + (unsigned long)tramp_pg_dir, >> >> >> >> >> >> + PGD_SIZE, PAGE_KERNEL_RO); >> >> >> >> >> > >> >> >> >> >> > Hmm, I like the idea but is there a risk that the page table has been mapped >> >> >> >> >> > as part of a block entry, which we can't safely split at this point (i.e. >> >> >> >> >> > we'll run into one of the BUG_ONs in the mapping code)? >> >> >> >> >> > >> >> >> >> >> >> >> >> >> >> We'd need to create a separate segment for it initially so the mapping >> >> >> >> >> is already at the right granularity. >> >> >> >> > >> >> >> >> > Why do you think that's the case? I can't see anything that guarantees this >> >> >> >> > for the page table itself. >> >> >> >> > >> >> >> >> >> >> >> >> We'd need to pass NO_BLOCK_MAPPINGS to map_kernel_segment(), >> >> >> >> obviously, but that shouldn't hurt since that segment is relatively >> >> >> >> tiny anyway. >> >> >> > >> >> >> > Ah right, with NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS, I agree that we're good. >> >> >> > Ideally, we'd move {idmap,swapper,tramp}_pg_dir into .rodata... >> >> >> > >> >> >> >> >> >> idmap and tramp yes, but swapper needs to be modifiable at runtime, no? >> >> > >> >> > Right, but couldn't we swizzle the permissions in e.g. set_pmd? We could >> >> > even predicate that on a sanity check of the prot. >> >> > >> >> >> >> Swizzle the permissions of the entire .rodata segment? That sounds >> >> doable, but there is a whole class of data that belongs in this >> >> category, and I think PaX/grsecurity had an API for that (but I don't >> >> think anyone is upstreaming that yet). So let's not reinvent that >> >> wheel for swapper_pg_dir only. >> > >> > I wasn't thinking of the whole .rodata segment -- just the page containing >> > the entry being modified, but ok. >> >> That means we will need to map .rodata down to pages as well, or at >> least avoid contiguous mappings. > > Doesn't it already avoid those? > You are right, it does. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-06-19 16:00 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-30 4:48 [PATCH] arm64: mm: mark tramp_pg_dir read-only YaoJun 2018-05-30 9:14 ` Will Deacon 2018-05-30 9:53 ` Ard Biesheuvel 2018-06-19 15:20 ` Will Deacon 2018-06-19 15:23 ` Ard Biesheuvel 2018-06-19 15:28 ` Will Deacon 2018-06-19 15:29 ` Ard Biesheuvel 2018-06-19 15:37 ` Will Deacon 2018-06-19 15:40 ` Ard Biesheuvel 2018-06-19 15:50 ` Will Deacon 2018-06-19 15:51 ` Ard Biesheuvel 2018-06-19 15:55 ` Will Deacon 2018-06-19 16:00 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).