linux-kernel - Re: [PATCH] ARM: allow, but warn, when issuing ioremap() on RAM

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]

Date:	Mon, 11 Oct 2010 11:05:44 +0100
From:	Catalin Marinas <catalin.marinas@....com>
To:	Nicolas Pitre <nico@...xnic.net>
Cc:	Russell King - ARM Linux <linux@....linux.org.uk>,
	Arnd Hannemann <arnd@...dnet.de>,
	Hemant Pedanekar <hemantp@...com>, Greg KH <greg@...ah.com>,
	Felipe Contreras <felipe.contreras@...il.com>,
	linux-main <linux-kernel@...r.kernel.org>,
	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	Han Jonghun <jonghun79.han@...il.com>,
	linux-arm <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] ARM: allow, but warn, when issuing ioremap() on RAM

On Sat, 2010-10-09 at 03:41 +0100, Nicolas Pitre wrote:
> On Sat, 9 Oct 2010, Russell King - ARM Linux wrote:
> 
> > On Fri, Oct 08, 2010 at 05:00:46PM -0700, Greg KH wrote:
> > > But you can't expect that you make this change, and not fix up the
> > > drivers, and people would be happy, right?  The rule for API changes
> > > like this, or anything, is that the person making the change fixes the
> > > other drivers, and that seems to be the issue here.
> >
> > Let's entirely revert the change and wait for people's data to be
> > corrupted then.  I don't have the time nor the motivation to work
> > through crap driver code to fix up these unreliable games which are
> > already illegal on platforms such as x86.
> >
> > If people want their system to be unpredictable, then let's carry on
> > giving them the rope to hang themselves in that manner.
> >
> > > Any pointers to patches where people have fixed up the drivers?
> >
> > Despite the discussion, I'm unaware of anyone really taking the issue
> > seriously and producing any patches during the last six months.
> >
> > So, I say sod it, let's revert the change.
> 
> I think that the real issue here is to avoid breaking those drivers
> which were using this legitimately.  So what about this compromize
> instead:
> 
> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
> index 99627d3..4f071e4 100644
> --- a/arch/arm/mm/ioremap.c
> +++ b/arch/arm/mm/ioremap.c
> @@ -201,6 +201,15 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>         if (pfn >= 0x100000 && (__pfn_to_phys(pfn) & ~SUPERSECTION_MASK))
>                 return NULL;
> 
> +       /*
> +        * Warn if RAM is mapped to discourage this usage. Let's forbid it
> +        * outright on ARMv6+ where this became architecturally undefined
> +        * in theory and causes memory corruption in practice.
> +        */
> +       if (WARN_ON(pfn_valid(pfn)))
> +               if (__LINUX_ARM_ARCH__ >= 6)
> +                       return NULL;
> +
>         type = get_mem_type(mtype);
>         if (!type)
>                 return NULL;

We could be a bit more flexible as a temporary solution. But that's a
hack and doesn't guarantee the attributes that the driver requested. If
the driver would use writel/readl, that's not too bad since we pushed
explicit barriers in these macros.

It would need to be improved to invalidate the corresponding cache lines
(using the DMA API?) but it would look even worse.


diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 6bdf42c..082bbd0 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -204,10 +204,13 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
 #endif
 
 	/*
-	 * Don't allow RAM to be mapped - this causes problems with ARMv6+
+	 * Don't allow RAM to be mapped as Device memory - this causes
+	 * problems with ARMv6+
 	 */
 	if (WARN_ON(pfn_valid(pfn)))
-		return NULL;
+		if (__LINUX_ARM_ARCH__ >= 6 &&
+		    (mtype != MT_DEVICE_CACHED || mtype != MT_DEVICE_WC))
+			mtype = MT_DEVICE_WC;
 
 	type = get_mem_type(mtype);
 	if (!type)



-- 
Catalin

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