linux-kernel - Re: bugs in page colouring code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120614084219.GD22007@linux-sh.org>
Date:	Thu, 14 Jun 2012 17:42:20 +0900
From:	Paul Mundt <lethal@...ux-sh.org>
To:	Rik van Riel <riel@...hat.com>
Cc:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, sjhill@...s.com, ralf@...ux-mips.org,
	Borislav Petkov <borislav.petkov@....com>,
	"H. Peter Anvin" <hpa@...ux.intel.com>,
	Rob Herring <rob.herring@...xeda.com>,
	Russell King <rmk+kernel@....linux.org.uk>,
	Nicolas Pitre <nico@...aro.org>
Subject: Re: bugs in page colouring code

On Wed, Jun 13, 2012 at 03:29:36PM -0400, Rik van Riel wrote:
> ARM & MIPS seem to share essentially the same page colouring code, with
> these two bugs:
> 
> COLOUR_ALIGN_DOWN can use the pgoff % shm_align_mask either positively
>    or negatively, depending on the address initially found by 
>    get_unmapped_area
> 
> static inline unsigned long COLOUR_ALIGN_DOWN(unsigned long addr,
>                                               unsigned long pgoff)
> {
>         unsigned long base = addr & ~shm_align_mask;
>         unsigned long off = (pgoff << PAGE_SHIFT) & shm_align_mask;
>
>         if (base + off <= addr)
>                 return base + off;
>
>         return base - off;
> }
>
> The fix would be to return an address that is a whole shm_align_mask
> lower: (((base - shm_align_mask) & ~shm_align_mask) + off

'addr' in this case is already adjusted by callers of COLOUR_ALIGN_DOWN(), so
this shouldn't be an issue, unless I'm missing something?

> The second bug relates to MAP_FIXED mappings of files.  In the
> MAP_FIXED conditional, arch_get_unmapped_area(_topdown) checks
> whether the mapping is colour aligned, but only for MAP_SHARED
> mappings.
> 
>                 /*
>                  * We do not accept a shared mapping if it would violate
>                  * cache aliasing constraints.
>                  */
>                 if ((flags & MAP_SHARED) &&
>                     ((addr - (pgoff << PAGE_SHIFT)) & shm_align_mask))
>                         return -EINVAL;
> 
These observations hold true for other architectures, too. I modelled the
SH implementation off of both MIPS and sparc, where these same patterns
exist. I would be surprised if there are any architectures that do
colouring in a different way.

The logic is such that in the MAP_FIXED case we can't align addr on to some
other boundary, and so anything that violates the aliasing constraints fails.
This is a departure from POSIX, and does occasionally lead to people sending in
patches to "correct" the behaviour for the LTP mmap01 testcase which does
iterative MAP_FIXED|MAP_SHARED PAGE_SIZE apart.

> This fails to take into account that the same file might be mapped
> MAP_SHARED from some programs, and MAP_PRIVATE from another.  The
> fix could be a simple as always enforcing colour alignment when we
> are mmapping a file (filp is non-zero).
> 
If that combination is possible then defaulting to colour alignment seems
reasonable. Whether that combination is reasonable or not is another matter.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/