public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: linux@armlinux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] misc: sram-exec: Use aligned fncpy instead of memcpy
Date: Thu, 6 Apr 2017 20:07:47 +0100	[thread overview]
Message-ID: <20170406190747.GP23750@n2100.armlinux.org.uk> (raw)
In-Reply-To: <ab1583fc-0560-b4d2-851b-4be6201edf8b@ti.com>

On Wed, Apr 05, 2017 at 02:22:33PM -0500, Dave Gerlach wrote:
> Russell,
> On 04/05/2017 02:21 PM, Dave Gerlach wrote:
> >Currently the sram-exec functionality, which allows allocation of
> >executable memory and provides an API to move code to it, is only
> >selected in configs for the ARM architecture. Based on commit
> >5756e9dd0de6 ("ARM: 6640/1: Thumb-2: Symbol manipulation macros for
> >function body copying") simply copying a C function pointer address
> >using memcpy without consideration of alignment and Thumb is unsafe on
> >ARM platforms.
> >
> >The aforementioned patch introduces the fncpy macro which is a safe way
> >to copy executable code on ARM platforms, so let's make use of that here
> >rather than the unsafe plain memcpy that was previously used by
> >sram_exec_copy.
> >
> >In the future, architectures hoping to make use of the sram-exec
> >functionality must define an fncpy macro just as ARM has done to
> >guarantee or check for safe copying to executable memory before allowing
> >the arch to select CONFIG_SRAM_EXEC.
> >
> >Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> >---
> > drivers/misc/sram-exec.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/misc/sram-exec.c b/drivers/misc/sram-exec.c
> >index ac522417c462..0057eabe5c03 100644
> >--- a/drivers/misc/sram-exec.c
> >+++ b/drivers/misc/sram-exec.c
> >@@ -19,6 +19,7 @@
> > #include <linux/sram.h>
> >
> > #include <asm/cacheflush.h>
> >+#include <asm/fncpy.h>
> >
> > #include "sram.h"
> >
> >@@ -93,7 +94,7 @@ int sram_exec_copy(struct gen_pool *pool, void *dst, void *src,
> > 	set_memory_nx((unsigned long)base, pages);
> > 	set_memory_rw((unsigned long)base, pages);
> >
> >-	memcpy(dst, src, size);
> >+	fncpy(dst, src, size);
> >
> > 	set_memory_ro((unsigned long)base, pages);
> > 	set_memory_x((unsigned long)base, pages);
> >
> 
> Does this address your concerns from here [1]? Because the only user of this
> code is ARM right now I already only build the sram-exec code in if
> CONFIG_ARM is selected.

Sorry, it does not.  Please read the comments in asm/fncpy.h.

Deviating from the proscribed usage means your code is, quite simply,
buggy.  There's no two ways about that.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

  reply	other threads:[~2017-04-06 19:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-05 19:21 [PATCH] misc: sram-exec: Use aligned fncpy instead of memcpy Dave Gerlach
2017-04-05 19:22 ` Dave Gerlach
2017-04-06 19:07   ` Russell King - ARM Linux [this message]
2017-04-06 19:14     ` Dave Gerlach
2017-04-06 19:29       ` Russell King - ARM Linux
2017-04-06 19:35         ` Dave Gerlach

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170406190747.GP23750@n2100.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox