linux-kernel - Re: MIPS: c17a6554 broke 64BIT_PHYS

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] [day] [month] [year] [list]

Message-ID: <20130423153815.GB30148@linux-mips.org>
Date:	Tue, 23 Apr 2013 17:38:15 +0200
From:	Ralf Baechle <ralf@...ux-mips.org>
To:	Thomas Lange <thomas@...elatus.se>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: MIPS: c17a6554 broke 64BIT_PHYS_ADDR for 32 bit systems

On Sat, Apr 20, 2013 at 11:42:54AM +0200, Thomas Lange wrote:

> commit c17a6554 unintentionally(?) modified the PAGE_MASK type
> from (int) to (long unsigned int).
> 
> This breaks ioremap (and possibly more) when using 64BIT_PHYS_ADDR on
> 32 bit systems.
> Example of failing code from ioremap.c:
> 
> 	phys_addr &= PAGE_MASK;
> 
> Since phys_addr is 64 bit (unsigned long long) when 64BIT_PHYS_ADDR and
> PAGE_MASK is 32bit (long unsigned int), the upper 32 bits will always
> be zeroed which is not what we want/expect.
> 
> The code above works if PAGE_MASK is a _signed_ 32bit int though.
> 
> Some possible fixes:
> 
> A) Simply revert the commit. Makes ioremap work again, but then PAGE_MASK
>    is a signed int. Do we really want a mask that is 'signed'?
> 
> B) Don't use PAGE_MASK for physical addresses. x86 defines this:
> 
>   /* Cast PAGE_MASK to a signed type so that it is sign-extended if
>      virtual addresses are 32-bits but physical addresses are larger
>      (ie, 32-bit PAE). */
>   #define PHYSICAL_PAGE_MASK	(((signed long)PAGE_MASK) & __PHYSICAL_MASK)
> 
>   Perhaps mips need something similar?
> 
> 
> This is an issue with 3.8 and doesn't seem to be solved in master either.

Thanks for noting this issue.  Manuel Lauss reported the same issue and
I already fixed it yesterday by reverting the offending patch.  Post rc7
reverting was the right things to do - and now that it's reverted I don't
really see the need for another patch?

Linus noticed that the revert is for an issue of which more instances might
exist and so hacked sparse to look for it, see message id

  CA+55aFy0jPr5udwJx8cZ-QFs-OgZzQ_NJm_VSj_cS7tsKUn3Kw@...l.gmail.com

on the linux-kernel mailing list or simply https://lkml.org/lkml/2013/4/22/518.

  Ralf
--
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/