lkml.org 
[lkml]   [2016]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.12 076/142] x86/mm/pat: Avoid truncation when converting cpa->numpages to address
    Date
    From: Matt Fleming <matt@codeblueprint.co.uk>

    3.12-stable review patch. If anyone has any objections, please let me know.

    ===============

    commit 742563777e8da62197d6cb4b99f4027f59454735 upstream.

    There are a couple of nasty truncation bugs lurking in the pageattr
    code that can be triggered when mapping EFI regions, e.g. when we pass
    a cpa->pgd pointer. Because cpa->numpages is a 32-bit value, shifting
    left by PAGE_SHIFT will truncate the resultant address to 32-bits.

    Viorel-Cătălin managed to trigger this bug on his Dell machine that
    provides a ~5GB EFI region which requires 1236992 pages to be mapped.
    When calling populate_pud() the end of the region gets calculated
    incorrectly in the following buggy expression,

    end = start + (cpa->numpages << PAGE_SHIFT);

    And only 188416 pages are mapped. Next, populate_pud() gets invoked
    for a second time because of the loop in __change_page_attr_set_clr(),
    only this time no pages get mapped because shifting the remaining
    number of pages (1048576) by PAGE_SHIFT is zero. At which point the
    loop in __change_page_attr_set_clr() spins forever because we fail to
    map progress.

    Hitting this bug depends very much on the virtual address we pick to
    map the large region at and how many pages we map on the initial run
    through the loop. This explains why this issue was only recently hit
    with the introduction of commit

    a5caa209ba9c ("x86/efi: Fix boot crash by mapping EFI memmap
    entries bottom-up at runtime, instead of top-down")

    It's interesting to note that safe uses of cpa->numpages do exist in
    the pageattr code. If instead of shifting ->numpages we multiply by
    PAGE_SIZE, no truncation occurs because PAGE_SIZE is a UL value, and
    so the result is unsigned long.

    To avoid surprises when users try to convert very large cpa->numpages
    values to addresses, change the data type from 'int' to 'unsigned
    long', thereby making it suitable for shifting by PAGE_SHIFT without
    any type casting.

    The alternative would be to make liberal use of casting, but that is
    far more likely to cause problems in the future when someone adds more
    code and fails to cast properly; this bug was difficult enough to
    track down in the first place.

    Reported-and-tested-by: Viorel-Cătălin Răpițeanu <rapiteanu.catalin@gmail.com>
    Acked-by: Borislav Petkov <bp@alien8.de>
    Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
    Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
    Link: https://bugzilla.kernel.org/show_bug.cgi?id=110131
    Link: http://lkml.kernel.org/r/1454067370-10374-1-git-send-email-matt@codeblueprint.co.uk
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: Jiri Slaby <jslaby@suse.cz>
    ---
    arch/x86/mm/pageattr.c | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

    diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
    index aabdf762f592..0fcd960b382a 100644
    --- a/arch/x86/mm/pageattr.c
    +++ b/arch/x86/mm/pageattr.c
    @@ -32,7 +32,7 @@ struct cpa_data {
    unsigned long *vaddr;
    pgprot_t mask_set;
    pgprot_t mask_clr;
    - int numpages;
    + unsigned long numpages;
    int flags;
    unsigned long pfn;
    unsigned force_split : 1;
    @@ -884,7 +884,7 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int checkalias)
    * CPA operation. Either a large page has been
    * preserved or a single page update happened.
    */
    - BUG_ON(cpa->numpages > numpages);
    + BUG_ON(cpa->numpages > numpages || !cpa->numpages);
    numpages -= cpa->numpages;
    if (cpa->flags & (CPA_PAGES_ARRAY | CPA_ARRAY))
    cpa->curpage++;
    --
    2.7.1
    \
     
     \ /
      Last update: 2016-02-24 12:21    [W:5.121 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site