diff mbox series

[v4,19/21] sandbox: Allow to execute from RAM

Message ID 20180618152315.34233-20-agraf@suse.de
State New
Headers show
Series sandbox: efi_loader support | expand

Commit Message

Alexander Graf June 18, 2018, 3:23 p.m. UTC
With efi_loader, we may want to execute payloads from RAM. By default,
permissions on the RAM region don't allow us to execute from there though.

So whenever we get into the efi_loader case, let's mark RAM as executable.
That way we still protect normal cases, but allow for efi binaries to
directly get executed from within RAM.

For this, we hook into the already existing allow_unaligned() call which
also transitions the system over into semantics required by the UEFI
specification.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/sandbox/cpu/cpu.c | 14 ++++++++++++++
 arch/sandbox/cpu/os.c  | 14 ++++++++++++++
 include/os.h           | 19 +++++++++++++++++++
 3 files changed, 47 insertions(+)

Comments

Simon Glass June 21, 2018, 2:02 a.m. UTC | #1
Hi Alex,

On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote:
> With efi_loader, we may want to execute payloads from RAM. By default,
> permissions on the RAM region don't allow us to execute from there though.
>
> So whenever we get into the efi_loader case, let's mark RAM as executable.
> That way we still protect normal cases, but allow for efi binaries to
> directly get executed from within RAM.
>
> For this, we hook into the already existing allow_unaligned() call which
> also transitions the system over into semantics required by the UEFI
> specification.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/sandbox/cpu/cpu.c | 14 ++++++++++++++
>  arch/sandbox/cpu/os.c  | 14 ++++++++++++++
>  include/os.h           | 19 +++++++++++++++++++
>  3 files changed, 47 insertions(+)
>

What is this patch actually for? Does it make something work that did
not before? Where is it called?

Regards.
Simon
Alexander Graf June 21, 2018, 9:44 a.m. UTC | #2
On 06/21/2018 04:02 AM, Simon Glass wrote:
> Hi Alex,
>
> On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote:
>> With efi_loader, we may want to execute payloads from RAM. By default,
>> permissions on the RAM region don't allow us to execute from there though.
>>
>> So whenever we get into the efi_loader case, let's mark RAM as executable.
>> That way we still protect normal cases, but allow for efi binaries to
>> directly get executed from within RAM.
>>
>> For this, we hook into the already existing allow_unaligned() call which
>> also transitions the system over into semantics required by the UEFI
>> specification.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>   arch/sandbox/cpu/cpu.c | 14 ++++++++++++++
>>   arch/sandbox/cpu/os.c  | 14 ++++++++++++++
>>   include/os.h           | 19 +++++++++++++++++++
>>   3 files changed, 47 insertions(+)
>>
> What is this patch actually for? Does it make something work that did
> not before? Where is it called?

At least on aarch64 executing from the RAM region fails on the first 
instruction you call inside it, because it's not mapped with PROT_EXEC. 
I think not mapping it with PROT_EXEC is a good thing in the normal 
sandbox use case, but for EFI we need to run from RAM ;).

So yes, this patch makes that work. It's called from allow_unaligned() 
which gets called from the bootefi command function.

Alex
Simon Glass June 21, 2018, 7:45 p.m. UTC | #3
Hi Alex,

On 21 June 2018 at 03:44, Alexander Graf <agraf@suse.de> wrote:
> On 06/21/2018 04:02 AM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote:
>>>
>>> With efi_loader, we may want to execute payloads from RAM. By default,
>>> permissions on the RAM region don't allow us to execute from there
>>> though.
>>>
>>> So whenever we get into the efi_loader case, let's mark RAM as
>>> executable.
>>> That way we still protect normal cases, but allow for efi binaries to
>>> directly get executed from within RAM.
>>>
>>> For this, we hook into the already existing allow_unaligned() call which
>>> also transitions the system over into semantics required by the UEFI
>>> specification.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>   arch/sandbox/cpu/cpu.c | 14 ++++++++++++++
>>>   arch/sandbox/cpu/os.c  | 14 ++++++++++++++
>>>   include/os.h           | 19 +++++++++++++++++++
>>>   3 files changed, 47 insertions(+)
>>>
>> What is this patch actually for? Does it make something work that did
>> not before? Where is it called?
>
>
> At least on aarch64 executing from the RAM region fails on the first
> instruction you call inside it, because it's not mapped with PROT_EXEC. I
> think not mapping it with PROT_EXEC is a good thing in the normal sandbox
> use case, but for EFI we need to run from RAM ;).
>
> So yes, this patch makes that work. It's called from allow_unaligned() which
> gets called from the bootefi command function.

OK I see. This is so that sandbox running on an ARM platform can
actually load and run an EFI application, right?

Can you please make this code and commit message ARM-specific? Also,
just call your code on start-up in cpu.c. We don't need to connect to
an allow_unaligned() thing, which is a misnomer in this case.

Regards,
Simon
Alexander Graf June 22, 2018, 9:43 a.m. UTC | #4
On 06/21/2018 09:45 PM, Simon Glass wrote:
> Hi Alex,
>
> On 21 June 2018 at 03:44, Alexander Graf <agraf@suse.de> wrote:
>> On 06/21/2018 04:02 AM, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote:
>>>> With efi_loader, we may want to execute payloads from RAM. By default,
>>>> permissions on the RAM region don't allow us to execute from there
>>>> though.
>>>>
>>>> So whenever we get into the efi_loader case, let's mark RAM as
>>>> executable.
>>>> That way we still protect normal cases, but allow for efi binaries to
>>>> directly get executed from within RAM.
>>>>
>>>> For this, we hook into the already existing allow_unaligned() call which
>>>> also transitions the system over into semantics required by the UEFI
>>>> specification.
>>>>
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>>    arch/sandbox/cpu/cpu.c | 14 ++++++++++++++
>>>>    arch/sandbox/cpu/os.c  | 14 ++++++++++++++
>>>>    include/os.h           | 19 +++++++++++++++++++
>>>>    3 files changed, 47 insertions(+)
>>>>
>>> What is this patch actually for? Does it make something work that did
>>> not before? Where is it called?
>>
>> At least on aarch64 executing from the RAM region fails on the first
>> instruction you call inside it, because it's not mapped with PROT_EXEC. I
>> think not mapping it with PROT_EXEC is a good thing in the normal sandbox
>> use case, but for EFI we need to run from RAM ;).
>>
>> So yes, this patch makes that work. It's called from allow_unaligned() which
>> gets called from the bootefi command function.
> OK I see. This is so that sandbox running on an ARM platform can
> actually load and run an EFI application, right?
>
> Can you please make this code and commit message ARM-specific? Also,

Not, it's not ARM specific. You may hit this on any system that actually 
checks for PROT_EXEC.

> just call your code on start-up in cpu.c. We don't need to connect to
> an allow_unaligned() thing, which is a misnomer in this case.

In that case we can just tell mmap() to map RAM with PROC_EXEC.


Alex
diff mbox series

Patch

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 9087026d71..641b66a0a7 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -192,3 +192,17 @@  void efi_add_known_memory(void)
 }
 
 #endif
+
+#if CONFIG_IS_ENABLED(EFI_LOADER)
+
+void allow_unaligned(void)
+{
+	int r;
+
+	r = os_mprotect(gd->arch.ram_buf, gd->ram_size,
+			OS_PROT_READ | OS_PROT_WRITE | OS_PROT_EXEC);
+
+	assert(!r);
+}
+
+#endif
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 9cfdc9f31f..9fa79a8843 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -183,6 +183,20 @@  void *os_realloc(void *ptr, size_t length)
 	return buf;
 }
 
+int os_mprotect(void *ptr, size_t length, int prot)
+{
+	int p = 0;
+
+	if (prot & OS_PROT_READ)
+		p |= PROT_READ;
+	if (prot & OS_PROT_WRITE)
+		p |= PROT_WRITE;
+	if (prot & OS_PROT_EXEC)
+		p |= PROT_EXEC;
+
+	return mprotect(ptr, length, p);
+}
+
 void os_usleep(unsigned long usec)
 {
 	usleep(usec);
diff --git a/include/os.h b/include/os.h
index c8e0f52d30..d451e12064 100644
--- a/include/os.h
+++ b/include/os.h
@@ -157,6 +157,25 @@  void os_free(void *ptr);
 void *os_realloc(void *ptr, size_t length);
 
 /**
+ * Modify protection of a memory region
+ *
+ * This function changes the memory protection scheme of a given memory
+ * region. Using it you can for example allow execution of memory that
+ * would otherwise prohibit it.
+ *
+ * \param ptr		Pointer to memory region to modify
+ * \param length	New length for memory block
+ * \param prot		New protection scheme (ORed OS_PROT_ values)
+ * \return 0 on success, -1 otherwise.
+ */
+int os_mprotect(void *ptr, size_t length, int prot);
+
+/* Defines for "prot" in os_mprotect() */
+#define OS_PROT_READ	0x1
+#define OS_PROT_WRITE	0x2
+#define OS_PROT_EXEC	0x4
+
+/**
  * Access to the usleep function of the os
  *
  * \param usec Time to sleep in micro seconds