Change in ...coreboot[master]: [RFC] commonlib/region: Check `xlate` regions early

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30083

Change subject: [RFC] commonlib/region: Check `xlate` regions early
......................................................................

[RFC] commonlib/region: Check `xlate` regions early

Makes it possible to detect errors early in rdev_chain().

Change-Id: I2cdb9dd1ab9c00b227839a03fff389ba6fe78b4a
Signed-off-by: Nico Huber 
---
M src/commonlib/include/commonlib/region.h
M src/commonlib/region.c
2 files changed, 30 insertions(+), 39 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/30083/1

diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h
index 45484dd..f027bc2 100644
--- a/src/commonlib/include/commonlib/region.h
+++ b/src/commonlib/include/commonlib/region.h
@@ -30,6 +30,7 @@
  * chaining together multiple region_devices.
  */

+struct region;
 struct region_device;

/*
@@ -84,6 +85,7 @@
    ssize_t (*writeat)(const struct region_device *, const void *, size_t,
    	size_t);
    ssize_t (*eraseat)(const struct region_device *, size_t, size_t);
+	int (*region_ok)(const struct region_device *, const struct region *);
 };

struct region {
diff --git a/src/commonlib/region.c b/src/commonlib/region.c
index 541a125..6db13a3 100644
--- a/src/commonlib/region.c
+++ b/src/commonlib/region.c
@@ -36,12 +36,6 @@
    return 1;
 }

-static int normalize_and_ok(const struct region *outer, struct region *inner)
-{
-	inner->offset += region_offset(outer);
-	return region_is_subregion(outer, inner);
-}
-
 static const struct region_device *rdev_root(const struct region_device *rdev)
 {
    if (rdev->root == NULL)
@@ -49,6 +43,18 @@
    return rdev->root;
 }

+static int normalize_and_ok(struct region *inner,
+				const struct region_device *outer)
+{
+	const struct region_device *rdev;
+
+	rdev = rdev_root(outer);
+
+	inner->offset += region_offset(&outer->region);
+	return region_is_subregion(&outer->region, inner) &&
+		(!rdev->ops->region_ok || rdev->ops->region_ok(rdev, inner));
+}
+
 ssize_t rdev_relative_offset(const struct region_device *p,
    			const struct region_device *c)
 {
@@ -69,7 +75,7 @@
    	.size = size,
    };

-	if (!normalize_and_ok(&rd->region, &req))
+	if (!normalize_and_ok(&req, rd))
    	return NULL;

rdev = rdev_root(rd);
@@ -101,7 +107,7 @@
    	.size = size,
    };

-	if (!normalize_and_ok(&rd->region, &req))
+	if (!normalize_and_ok(&req, rd))
    	return -1;

rdev = rdev_root(rd);
@@ -118,7 +124,7 @@
    	.size = size,
    };

-	if (!normalize_and_ok(&rd->region, &req))
+	if (!normalize_and_ok(&req, rd))
    	return -1;

rdev = rdev_root(rd);
@@ -138,7 +144,7 @@
    	.size = size,
    };

-	if (!normalize_and_ok(&rd->region, &req))
+	if (!normalize_and_ok(&req, rd))
    	return -1;

rdev = rdev_root(rd);
@@ -159,7 +165,7 @@
    	.size = size,
    };

-	if (!normalize_and_ok(&parent->region, &req))
+	if (!normalize_and_ok(&req, parent))
    	return -1;

/* Keep track of root region device. Note the offsets are relative
@@ -343,16 +349,9 @@
    		size_t size)
 {
    const struct xlate_region_device *xldev;
-	struct region req = {
-		.offset = offset,
-		.size = size,
-	};

xldev = container_of(rd, __typeof__(*xldev), rdev);

-	if (!region_is_subregion(&xldev->sub_region, &req))
-		return NULL;
-
    offset -= region_offset(&xldev->sub_region);

return rdev_mmap(xldev->access_dev, offset, size);
@@ -370,17 +369,10 @@
 static ssize_t xlate_readat(const struct region_device *rd, void *b,
    			size_t offset, size_t size)
 {
-	struct region req = {
-		.offset = offset,
-		.size = size,
-	};
    const struct xlate_region_device *xldev;

xldev = container_of(rd, __typeof__(*xldev), rdev);

-	if (!region_is_subregion(&xldev->sub_region, &req))
-		return -1;
-
    offset -= region_offset(&xldev->sub_region);

return rdev_readat(xldev->access_dev, b, offset, size);
@@ -389,17 +381,10 @@
 static ssize_t xlate_writeat(const struct region_device *rd, const void *b,
    			size_t offset, size_t size)
 {
-	struct region req = {
-		.offset = offset,
-		.size = size,
-	};
    const struct xlate_region_device *xldev;

xldev = container_of(rd, __typeof__(*xldev), rdev);

-	if (!region_is_subregion(&xldev->sub_region, &req))
-		return -1;
-
    offset -= region_offset(&xldev->sub_region);

return rdev_writeat(xldev->access_dev, b, offset, size);
@@ -408,26 +393,29 @@
 static ssize_t xlate_eraseat(const struct region_device *rd,
    			size_t offset, size_t size)
 {
-	struct region req = {
-		.offset = offset,
-		.size = size,
-	};
    const struct xlate_region_device *xldev;

xldev = container_of(rd, __typeof__(*xldev), rdev);

-	if (!region_is_subregion(&xldev->sub_region, &req))
-		return -1;
-
    offset -= region_offset(&xldev->sub_region);

return rdev_eraseat(xldev->access_dev, offset, size);
 }

+static int xlate_region_ok(const struct region_device *rd,
+				const struct region *req)
+{
+	const struct xlate_region_device *xldev;
+
+	xldev = container_of(rd, __typeof__(*xldev), rdev);
+	return region_is_subregion(&xldev->sub_region, req);
+}
+
 const struct region_device_ops xlate_rdev_ro_ops = {
    .mmap = xlate_mmap,
    .munmap = xlate_munmap,
    .readat = xlate_readat,
+	.region_ok = xlate_region_ok,
 };

const struct region_device_ops xlate_rdev_rw_ops = {
@@ -436,6 +424,7 @@
    .readat = xlate_readat,
    .writeat = xlate_writeat,
    .eraseat = xlate_eraseat,
+	.region_ok = xlate_region_ok,
 };