Re: arm32 insecure W+X mapping

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Tim Harvey <tharvey@gateworks.com>
Cc: Shawn Guo <shawnguo@kernel.org>,
	Fabio Estevam <festevam@gmail.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>,
	Lee Jones <lee.jones@linaro.org>,
	Robin Murphy <robin.murphy@arm.com>
Subject: Re: arm32 insecure W+X mapping
Date: Tue, 7 Sep 2021 20:22:39 +0100	[thread overview]
Message-ID: <YTe7/1OXxyWv8RMc@shell.armlinux.org.uk> (raw)
In-Reply-To: <CAJ+vNU09ux-aoohB1TpdnPesg8MHzH=ZZDEvAHsajX7+UDRQ0g@mail.gmail.com>

On Tue, Sep 07, 2021 at 10:48:49AM -0700, Tim Harvey wrote:
> On Fri, Aug 20, 2021 at 11:41 AM Tim Harvey <tharvey@gateworks.com> wrote:
> > # uname -r
> > 5.13.12
> > # cat /proc/cmdline
> > console=ttymxc1,115200 no_hash_pointers
> > # echo 1 > /proc/sys/kernel/kptr_restrict
> > # dmesg | grep insecure
> > [   13.247957] arm/mm: Found insecure W+X mapping at address 0xf087d000
> > # cat /proc/vmallocinfo | grep 0xf087d000
> > 0xf0878000-0xf087d000   20480 of_iomap+0x44/0x68 phys=0x021b0000 ioremap
> > 0xf087d000-0xf087f000    8192 imx6_pm_common_init+0x118/0x36c
> > phys=0x00900000 ioremap
> >
> > Some debugging showed me that 0xf087d000 is 'suspend_ocram_base'
> > remapped from imx6q_suspend_init() (called form imx6_pm_common_init()
> > [1]
> > suspend_ocram_base = __arm_ioremap_exec(ocram_pbase,
> > MX6Q_SUSPEND_OCRAM_SIZE, false);
> >
> > This should be throwing 'Checked W+X mappings: FAILED, 1 W+X pages
> > found' messages for all IMX6 users that have CONFIG_SUSPEND and
> > CONFIG_DEBUG_WX enabled so I'm adding the IMX6 players to the thread
> > to see if they know why this happens.
> >
> 
> Shawn, Fabio and Pengutronix Kernel team,
> 
> Do you know why we get 'Checked W+X mappings: FAILED, 1 W+X pages
> found' messages for IMX6 with CONFIG_SUSPEND and CONFIG_DEBUG_WX
> enabled due to to __arm_ioremap_exec call remapping ocram? [1]

The current situation looks like the OCRAM is used to store some
suspend/resume code (see arch/arm/mach-imx/suspend-imx6.S), along
with some data.

It looks like once the code has been copied and the data has been
written, the mapping is left as-is - it isn't changed to be
read-only-execute. However, I don't think we have any APIs to do
that on iomem.

set_memory_ro() could be leveraged to do it _if_ we are certain
the memory is not mapped using a section mapping, but that would
depend on the size and alignment of the mapping.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel