• Matt Fleming's avatar
    x86/mm/pat: Avoid truncation when converting cpa->numpages to address · 67022807
    Matt Fleming authored
    commit 74256377 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
    
      a5caa209 ("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: default avatarViorel-Cătălin Răpițeanu <rapiteanu.catalin@gmail.com>
    Acked-by: default avatarBorislav Petkov <bp@alien8.de>
    Cc: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
    Signed-off-by: default avatarMatt 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.ukSigned-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
    Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
    Signed-off-by: default avatarWilly Tarreau <w@1wt.eu>
    67022807
pageattr.c 30.7 KB