<p>Nico Huber has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/c/coreboot/+/30083">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">[RFC] commonlib/region: Check `xlate` regions early<br><br>Makes it possible to detect errors early in rdev_chain().<br><br>Change-Id: I2cdb9dd1ab9c00b227839a03fff389ba6fe78b4a<br>Signed-off-by: Nico Huber <nico.huber@secunet.com><br>---<br>M src/commonlib/include/commonlib/region.h<br>M src/commonlib/region.c<br>2 files changed, 30 insertions(+), 39 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/30083/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/commonlib/include/commonlib/region.h b/src/commonlib/include/commonlib/region.h</span><br><span>index 45484dd..f027bc2 100644</span><br><span>--- a/src/commonlib/include/commonlib/region.h</span><br><span>+++ b/src/commonlib/include/commonlib/region.h</span><br><span>@@ -30,6 +30,7 @@</span><br><span>  * chaining together multiple region_devices.</span><br><span>  */</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+struct region;</span><br><span> struct region_device;</span><br><span> </span><br><span> /*</span><br><span>@@ -84,6 +85,7 @@</span><br><span>        ssize_t (*writeat)(const struct region_device *, const void *, size_t,</span><br><span>               size_t);</span><br><span>     ssize_t (*eraseat)(const struct region_device *, size_t, size_t);</span><br><span style="color: hsl(120, 100%, 40%);">+     int (*region_ok)(const struct region_device *, const struct region *);</span><br><span> };</span><br><span> </span><br><span> struct region {</span><br><span>diff --git a/src/commonlib/region.c b/src/commonlib/region.c</span><br><span>index 541a125..6db13a3 100644</span><br><span>--- a/src/commonlib/region.c</span><br><span>+++ b/src/commonlib/region.c</span><br><span>@@ -36,12 +36,6 @@</span><br><span>        return 1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int normalize_and_ok(const struct region *outer, struct region *inner)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">-        inner->offset += region_offset(outer);</span><br><span style="color: hsl(0, 100%, 40%);">-       return region_is_subregion(outer, inner);</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static const struct region_device *rdev_root(const struct region_device *rdev)</span><br><span> {</span><br><span>    if (rdev->root == NULL)</span><br><span>@@ -49,6 +43,18 @@</span><br><span>      return rdev->root;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static int normalize_and_ok(struct region *inner,</span><br><span style="color: hsl(120, 100%, 40%);">+                             const struct region_device *outer)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct region_device *rdev;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   rdev = rdev_root(outer);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    inner->offset += region_offset(&outer->region);</span><br><span style="color: hsl(120, 100%, 40%);">+     return region_is_subregion(&outer->region, inner) &&</span><br><span style="color: hsl(120, 100%, 40%);">+           (!rdev->ops->region_ok || rdev->ops->region_ok(rdev, inner));</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> ssize_t rdev_relative_offset(const struct region_device *p,</span><br><span>                                const struct region_device *c)</span><br><span> {</span><br><span>@@ -69,7 +75,7 @@</span><br><span>              .size = size,</span><br><span>        };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  if (!normalize_and_ok(&rd->region, &req))</span><br><span style="color: hsl(120, 100%, 40%);">+  if (!normalize_and_ok(&req, rd))</span><br><span>                 return NULL;</span><br><span> </span><br><span>     rdev = rdev_root(rd);</span><br><span>@@ -101,7 +107,7 @@</span><br><span>          .size = size,</span><br><span>        };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  if (!normalize_and_ok(&rd->region, &req))</span><br><span style="color: hsl(120, 100%, 40%);">+  if (!normalize_and_ok(&req, rd))</span><br><span>                 return -1;</span><br><span> </span><br><span>       rdev = rdev_root(rd);</span><br><span>@@ -118,7 +124,7 @@</span><br><span>          .size = size,</span><br><span>        };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  if (!normalize_and_ok(&rd->region, &req))</span><br><span style="color: hsl(120, 100%, 40%);">+  if (!normalize_and_ok(&req, rd))</span><br><span>                 return -1;</span><br><span> </span><br><span>       rdev = rdev_root(rd);</span><br><span>@@ -138,7 +144,7 @@</span><br><span>          .size = size,</span><br><span>        };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  if (!normalize_and_ok(&rd->region, &req))</span><br><span style="color: hsl(120, 100%, 40%);">+  if (!normalize_and_ok(&req, rd))</span><br><span>                 return -1;</span><br><span> </span><br><span>       rdev = rdev_root(rd);</span><br><span>@@ -159,7 +165,7 @@</span><br><span>          .size = size,</span><br><span>        };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  if (!normalize_and_ok(&parent->region, &req))</span><br><span style="color: hsl(120, 100%, 40%);">+      if (!normalize_and_ok(&req, parent))</span><br><span>             return -1;</span><br><span> </span><br><span>       /* Keep track of root region device. Note the offsets are relative</span><br><span>@@ -343,16 +349,9 @@</span><br><span>                    size_t size)</span><br><span> {</span><br><span>    const struct xlate_region_device *xldev;</span><br><span style="color: hsl(0, 100%, 40%);">-        struct region req = {</span><br><span style="color: hsl(0, 100%, 40%);">-           .offset = offset,</span><br><span style="color: hsl(0, 100%, 40%);">-               .size = size,</span><br><span style="color: hsl(0, 100%, 40%);">-   };</span><br><span> </span><br><span>       xldev = container_of(rd, __typeof__(*xldev), rdev);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!region_is_subregion(&xldev->sub_region, &req))</span><br><span style="color: hsl(0, 100%, 40%);">-          return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>         offset -= region_offset(&xldev->sub_region);</span><br><span> </span><br><span>      return rdev_mmap(xldev->access_dev, offset, size);</span><br><span>@@ -370,17 +369,10 @@</span><br><span> static ssize_t xlate_readat(const struct region_device *rd, void *b,</span><br><span>                                size_t offset, size_t size)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-        struct region req = {</span><br><span style="color: hsl(0, 100%, 40%);">-           .offset = offset,</span><br><span style="color: hsl(0, 100%, 40%);">-               .size = size,</span><br><span style="color: hsl(0, 100%, 40%);">-   };</span><br><span>   const struct xlate_region_device *xldev;</span><br><span> </span><br><span>         xldev = container_of(rd, __typeof__(*xldev), rdev);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!region_is_subregion(&xldev->sub_region, &req))</span><br><span style="color: hsl(0, 100%, 40%);">-          return -1;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>   offset -= region_offset(&xldev->sub_region);</span><br><span> </span><br><span>      return rdev_readat(xldev->access_dev, b, offset, size);</span><br><span>@@ -389,17 +381,10 @@</span><br><span> static ssize_t xlate_writeat(const struct region_device *rd, const void *b,</span><br><span>                            size_t offset, size_t size)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-        struct region req = {</span><br><span style="color: hsl(0, 100%, 40%);">-           .offset = offset,</span><br><span style="color: hsl(0, 100%, 40%);">-               .size = size,</span><br><span style="color: hsl(0, 100%, 40%);">-   };</span><br><span>   const struct xlate_region_device *xldev;</span><br><span> </span><br><span>         xldev = container_of(rd, __typeof__(*xldev), rdev);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!region_is_subregion(&xldev->sub_region, &req))</span><br><span style="color: hsl(0, 100%, 40%);">-          return -1;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>   offset -= region_offset(&xldev->sub_region);</span><br><span> </span><br><span>      return rdev_writeat(xldev->access_dev, b, offset, size);</span><br><span>@@ -408,26 +393,29 @@</span><br><span> static ssize_t xlate_eraseat(const struct region_device *rd,</span><br><span>                          size_t offset, size_t size)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-        struct region req = {</span><br><span style="color: hsl(0, 100%, 40%);">-           .offset = offset,</span><br><span style="color: hsl(0, 100%, 40%);">-               .size = size,</span><br><span style="color: hsl(0, 100%, 40%);">-   };</span><br><span>   const struct xlate_region_device *xldev;</span><br><span> </span><br><span>         xldev = container_of(rd, __typeof__(*xldev), rdev);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!region_is_subregion(&xldev->sub_region, &req))</span><br><span style="color: hsl(0, 100%, 40%);">-          return -1;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>   offset -= region_offset(&xldev->sub_region);</span><br><span> </span><br><span>      return rdev_eraseat(xldev->access_dev, offset, size);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static int xlate_region_ok(const struct region_device *rd,</span><br><span style="color: hsl(120, 100%, 40%);">+                         const struct region *req)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+  const struct xlate_region_device *xldev;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    xldev = container_of(rd, __typeof__(*xldev), rdev);</span><br><span style="color: hsl(120, 100%, 40%);">+   return region_is_subregion(&xldev->sub_region, req);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> const struct region_device_ops xlate_rdev_ro_ops = {</span><br><span>     .mmap = xlate_mmap,</span><br><span>  .munmap = xlate_munmap,</span><br><span>      .readat = xlate_readat,</span><br><span style="color: hsl(120, 100%, 40%);">+       .region_ok = xlate_region_ok,</span><br><span> };</span><br><span> </span><br><span> const struct region_device_ops xlate_rdev_rw_ops = {</span><br><span>@@ -436,6 +424,7 @@</span><br><span>        .readat = xlate_readat,</span><br><span>      .writeat = xlate_writeat,</span><br><span>    .eraseat = xlate_eraseat,</span><br><span style="color: hsl(120, 100%, 40%);">+     .region_ok = xlate_region_ok,</span><br><span> };</span><br><span> </span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://review.coreboot.org/c/coreboot/+/30083">change 30083</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/c/coreboot/+/30083"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I2cdb9dd1ab9c00b227839a03fff389ba6fe78b4a </div>
<div style="display:none"> Gerrit-Change-Number: 30083 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Nico Huber <nico.h@gmx.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>