[PATCH] bsps/shared/ofw: Fix coverity defects

Gedare Bloom gedare at rtems.org
Wed Apr 28 19:20:35 UTC 2021


On Wed, Apr 28, 2021 at 11:30 AM G S Niteesh Babu <niteesh.gs at gmail.com> wrote:
>
> This patch adds asserts to fix coverity defects
> 1) CID 1474437 (Out-of-bounds access)
> 2) CID 1474436 (Out-of-bounds access)
>
> From manual inspection, out of bounds access cannot occur due to
> bounds checking but coverity fails to detect the checks.
> We are adding asserts as a secondary check.
> ---
>  bsps/shared/ofw/ofw.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/bsps/shared/ofw/ofw.c b/bsps/shared/ofw/ofw.c
> index f4b8b63931..808fa85d81 100644
> --- a/bsps/shared/ofw/ofw.c
> +++ b/bsps/shared/ofw/ofw.c
> @@ -42,6 +42,7 @@
>  #include <assert.h>
>  #include <rtems/sysinit.h>
>  #include <ofw/ofw_test.h>
> +#include <rtems/score/assert.h>
>
>  static void *fdtp = NULL;
>
> @@ -186,6 +187,7 @@ ssize_t rtems_ofw_get_prop(
>    const void *prop;
>    int offset;
>    int len;
> +  int copy_len;
>    uint32_t cpuid;
>
>    offset = rtems_fdt_phandle_to_offset(node);
> @@ -226,7 +228,9 @@ ssize_t rtems_ofw_get_prop(
>      return -1;
>    }
>
> -  bcopy(prop, buf, MIN(len, bufsize));
> +  copy_len = MIN(len, bufsize);
> +  _Assert(copy_len <= bufsize);
> +  memmove(prop, buf, copy_len);
>
>    return len;
>  }
> @@ -637,6 +641,12 @@ int rtems_ofw_get_reg(
>          range.child_bus = fdt32_to_cpu(ptr[j].child_bus);
>          range.size = fdt32_to_cpu(ptr[j].size);
>
> +        /*
> +         * buf[i + 1] should upperbound the access for buf[i].
> +         * Thus by making sure buf[i + 1] <= (buf + size) we
> +         * can be sure buf[i] will always be inbounds.
> +         */
> +        _Assert(buf[i + 1] <= (buf + size));
This looks suspect. It can make an out-of-bounds read access I think. How about
_Assert(i < size);
Is that equivalent?

>          if (buf[i].start >= range.child_bus &&
>              buf[i].start < range.child_bus + range.size) {
>            offset = range.parent_bus - range.child_bus;
> --
> 2.17.1
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list