diff mbox series

opal-prd: handle devtmpfs mounted with noexec

Message ID 20201012212917.1297088-1-gyakovlev@gentoo.org
State Accepted
Headers show
Series opal-prd: handle devtmpfs mounted with noexec | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (f901fcafae14d38e29f1cc11440086ee678785d0)
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present

Commit Message

Georgy Yakovlev Oct. 12, 2020, 9:29 p.m. UTC
On systems using recent versions of systemd
/dev (devtmpfs) is mounted with noexec option.
Such mount prevents mapping HBRT image code region as RWX from /dev.
This commit, as suggested in github PR linked below,
attempts to work around the situation by copying
HBRT image to anon mmaped memory region and
sets mprotect rwx on it, allowing opal-prd to sucessfully
execute the code region.

Having memory region set as RWX is not ideal for security,
but fixing that is a separate and hard to solve problem.
Original code also mmaped region as RWX, so this PR does not
make things worse at least.

Closes: https://github.com/open-power/skiboot/issues/258
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
---
 external/opal-prd/opal-prd.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

Comments

Vasant Hegde Oct. 13, 2020, 1:06 p.m. UTC | #1
On 10/13/20 2:59 AM, Georgy Yakovlev wrote:
> On systems using recent versions of systemd
> /dev (devtmpfs) is mounted with noexec option.
> Such mount prevents mapping HBRT image code region as RWX from /dev.
> This commit, as suggested in github PR linked below,
> attempts to work around the situation by copying
> HBRT image to anon mmaped memory region and
> sets mprotect rwx on it, allowing opal-prd to sucessfully
> execute the code region.
> 
> Having memory region set as RWX is not ideal for security,
> but fixing that is a separate and hard to solve problem.
> Original code also mmaped region as RWX, so this PR does not
> make things worse at least.
> 
> Closes: https://github.com/open-power/skiboot/issues/258
> Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>

Thank you for the fix.Looks good to me.

Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>

-Vasant
diff mbox series

Patch

diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c
index d74d8039..abf43a78 100644
--- a/external/opal-prd/opal-prd.c
+++ b/external/opal-prd/opal-prd.c
@@ -973,7 +973,9 @@  static int map_hbrt_file(struct opal_prd_ctx *ctx, const char *name)
 static int map_hbrt_physmem(struct opal_prd_ctx *ctx, const char *name)
 {
 	struct prd_range *range;
+	int rc;
 	void *buf;
+	void *ro_buf;
 
 	range = find_range(name, 0);
 	if (!range) {
@@ -981,15 +983,41 @@  static int map_hbrt_physmem(struct opal_prd_ctx *ctx, const char *name)
 		return -1;
 	}
 
-	buf = mmap(NULL, range->size, PROT_READ | PROT_WRITE | PROT_EXEC,
+	ro_buf = mmap(NULL, range->size, PROT_READ,
 			MAP_PRIVATE, ctx->fd, range->physaddr);
-	if (buf == MAP_FAILED) {
+	if (ro_buf == MAP_FAILED) {
 		pr_log(LOG_ERR, "IMAGE: mmap(range:%s, "
 				"phys:0x%016lx, size:0x%016lx) failed: %m",
 				name, range->physaddr, range->size);
 		return -1;
 	}
 
+	buf = mmap(NULL, range->size, PROT_WRITE,	
+			MAP_SHARED | MAP_ANONYMOUS, -1 , 0);
+	if (buf == MAP_FAILED) {
+		pr_log(LOG_ERR, "IMAGE: anon mmap(size:0x%016lx) failed: %m",
+				range->size);
+		return -1;
+	}
+
+	memcpy(buf, ro_buf, range->size);
+
+	rc = munmap(ro_buf, range->size);
+	if (rc < 0) {
+		pr_log(LOG_ERR, "IMAGE: munmap("
+				"phys:0x%016lx, size:0x%016lx) failed: %m",
+				range->physaddr, range->size);
+		return -1;
+	}
+
+	rc = mprotect(buf, range->size, PROT_READ | PROT_WRITE | PROT_EXEC);
+	if (rc < 0) {
+		pr_log(LOG_ERR, "IMAGE: mprotect(phys:%p, "
+			"size:0x%016lx, rwx) failed: %m",
+			buf, range->size);
+		return -1;
+	}
+
 	ctx->code_addr = buf;
 	ctx->code_size = range->size;
 	return 0;