From: "Aneesh Kumar K.V" <aneesh.kumar@linux•vnet.ibm.com>
To: Paul Mackerras <paulus@ozlabs•org>, linuxppc-dev@lists•ozlabs.org
Subject: Re: [PATCH 1/3] powerpc/mm: Don't alias user region to other regions below PAGE_OFFSET
Date: Fri, 02 Sep 2016 17:52:16 +0530 [thread overview]
Message-ID: <87vayeqyo7.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <20160902114759.GA12433@fergus.ozlabs.ibm.com>
Hi Paul,
Really nice catch. Was this found by code analysis or do we have any
reported issue around this ?
Paul Mackerras <paulus@ozlabs•org> writes:
> In commit c60ac5693c47 ("powerpc: Update kernel VSID range", 2013-03-13)
> we lost a check on the region number (the top four bits of the effective
> address) for addresses below PAGE_OFFSET. That commit replaced a check
> that the top 18 bits were all zero with a check that bits 46 - 59 were
> zero (performed for all addresses, not just user addresses).
To make review easy for others, here is the relevant diff from that commit.
_GLOBAL(slb_allocate_realmode)
- /* r3 = faulting address */
+ /*
+ * check for bad kernel/user address
+ * (ea & ~REGION_MASK) >= PGTABLE_RANGE
+ */
+ rldicr. r9,r3,4,(63 - 46 - 4)
+ bne- 8f
srdi r9,r3,60 /* get region */
......
And because we were doing the above check, I removed
.........
BEGIN_FTR_SECTION
b slb_finish_load
END_MMU_FTR_SECTION_IFCLR(MMU_FTR_1T_SEGMENT)
b slb_finish_load_1T
-0: /* user address: proto-VSID = context << 15 | ESID. First check
- * if the address is within the boundaries of the user region
- */
- srdi. r9,r10,USER_ESID_BITS
- bne- 8f /* invalid ea bits set */
-
-
+0:
>
> This means that userspace can access an address like 0x1000_0xxx_xxxx_xxxx
> and we will insert a valid SLB entry for it. The VSID used will be the
> same as if the top 4 bits were 0, but the page size will be some random
> value obtained by indexing beyond the end of the mm_ctx_high_slices_psize
> array in the paca. If that page size is the same as would be used for
> region 0, then userspace just has an alias of the region 0 space. If the
> page size is different, then no HPTE will be found for the access, and
> the process will get a SIGSEGV (since hash_page_mm() will refuse to create
> a HPTE for the bogus address).
>
> The access beyond the end of the mm_ctx_high_slices_psize can be at most
> 5.5MB past the array, and so will be in RAM somewhere. Since the access
> is a load performed in real mode, it won't fault or crash the kernel.
> At most this bug could perhaps leak a little bit of information about
> blocks of 32 bytes of memory located at offsets of i * 512kB past the
> paca->mm_ctx_high_slices_psize array, for 1 <= i <= 11.
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux•vnet.ibm.com>
>
> Cc: stable@vger•kernel.org # v3.10+
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux•vnet.ibm.com>
> Signed-off-by: Paul Mackerras <paulus@ozlabs•org>
> ---
> arch/powerpc/mm/slb_low.S | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/slb_low.S b/arch/powerpc/mm/slb_low.S
> index dfdb90c..9f19834 100644
> --- a/arch/powerpc/mm/slb_low.S
> +++ b/arch/powerpc/mm/slb_low.S
> @@ -113,7 +113,12 @@ BEGIN_FTR_SECTION
> END_MMU_FTR_SECTION_IFCLR(MMU_FTR_1T_SEGMENT)
> b slb_finish_load_1T
>
> -0:
> +0: /*
> + * For userspace addresses, make sure this is region 0.
> + */
> + cmpdi r9, 0
> + bne 8f
> +
> /* when using slices, we extract the psize off the slice bitmaps
> * and then we need to get the sllp encoding off the mmu_psize_defs
> * array.
> --
> 2.7.4
next prev parent reply other threads:[~2016-09-02 12:22 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-02 11:47 [PATCH 1/3] powerpc/mm: Don't alias user region to other regions below PAGE_OFFSET Paul Mackerras
2016-09-02 11:49 ` [PATCH 2/3] powerpc/mm: Preserve CFAR value on SLB miss caused by access to bogus address Paul Mackerras
2016-09-04 11:30 ` Aneesh Kumar K.V
2016-09-07 5:52 ` Paul Mackerras
2016-09-13 12:16 ` [2/3] " Michael Ellerman
2016-09-02 11:50 ` [PATCH 3/3] powerpc/mm: Speed up computation of base and actual page size for a HPTE Paul Mackerras
2016-09-04 11:16 ` Aneesh Kumar K.V
2016-09-05 5:04 ` Aneesh Kumar K.V
2016-09-07 5:07 ` Paul Mackerras
2016-09-07 6:17 ` [PATCH v2 " Paul Mackerras
2016-09-08 10:08 ` Paul Mackerras
2016-09-08 10:16 ` Paolo Bonzini
2016-09-12 0:58 ` Paul Mackerras
2016-09-12 3:03 ` Michael Ellerman
2016-09-12 9:45 ` Paolo Bonzini
2016-09-02 12:22 ` Aneesh Kumar K.V [this message]
2016-09-03 9:54 ` [PATCH 1/3] powerpc/mm: Don't alias user region to other regions below PAGE_OFFSET Paul Mackerras
2016-09-04 11:31 ` Aneesh Kumar K.V
2016-09-08 9:47 ` [1/3] " Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87vayeqyo7.fsf@linux.vnet.ibm.com \
--to=aneesh.kumar@linux$(echo .)vnet.ibm.com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=paulus@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox