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

Niteesh G. S. niteesh.gs at gmail.com
Thu May 6 08:20:20 UTC 2021


Hi,

I am really sorry for sending the wrong patch, the crash happened because of
wrong arguments to memove, bcopy has src first and dest second whereas it
is vice versa in memmove. I had fixed this but had sent the old one.
I am really sorry for the trouble. I'll make sure this doesn't happen again.

I have sent a v3 please take a look at that. I tested the patch by creating
a new
branch, applying the patch, and then building and testing. So I am sure it
will work.

Thanks,
Niteesh

On Thu, May 6, 2021 at 7:17 AM Vijay Kumar Banerjee <vijay at rtems.org> wrote:

> Hi all,
>
> On Wed, May 5, 2021 at 8:42 AM Gedare Bloom <gedare at rtems.org> wrote:
> >
> > alright looks good. Vijay or Christian please confirm and push if
> > you're good with it too.
> >
> ofw01.exe breaks after this patch. This probably needs some more debugging.
>
> If it helps, I'm pasting the error:
> ```
> *** FATAL ***
> fatal source: 9 (RTEMS_FATAL_SOURCE_EXCEPTION)
>
> R0   = 0x8001e68c R8  = 0x8001e68c
> R1   = 0x80104641 R9  = 0x8005b90c
> R2   = 0x00000030 R10 = 0x80104640
> R3   = 0x80104648 R11 = 0x00000002
> R4   = 0x00000008 R12 = 0x8001e68b
> R5   = 0x00002ebc SP  = 0x801045b8
> R6   = 0x80104640 LR  = 0x8000d720
> R7   = 0x80101e28 PC  = 0x80011ba8
> CPSR = 0x80000193 VEC = 0x00000004
> RTEMS version: 6.0.0.cfabe5d6e9d8b428110732fffb7ff6ee8211de04
> RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
> 4e256fc4cb0d91a098d2f3d88c3d302fc0426d56, Newlib 436e475)
> executing thread ID: 0x089010001
> executing thread name: IDLE
> U-Boot SPL 2018.09-00002-g0b54a51eee (Sep 10 2018 - 19:41:39 -0500)
> Trying to boot from MMC2
> Loading Environment from EXT4...
> ** Unable to use mmc 0:1 for loading the env **
>
> ```
>
> Best regards,
> Vijay
>
> > On Wed, May 5, 2021 at 12:52 AM Niteesh G. S. <niteesh.gs at gmail.com>
> wrote:
> > >
> > >
> > >
> > > On Mon, May 3, 2021 at 11:23 PM Gedare Bloom <gedare at rtems.org> wrote:
> > >>
> > >> Hi Niteesh,
> > >>
> > >> This looks good to me. What/how did you test it?
> > >
> > > I tested it using the ofw01 test
> > > https://git.rtems.org/rtems/tree/testsuites/libtests/ofw01/init.c
> > > and read EEPROM using i2c.
> > >
> > >>
> > >> Gedare
> > >>
> > >> On Sat, May 1, 2021 at 6:31 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..0e0a7033ab 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 + size - (sizeof(buf[0]) - 1) is the last valid
> > >> > +         * address for buf[i]. If buf[i] points to any address
> larger
> > >> > +         * than this, it will be an out of bound access
> > >> > +         */
> > >> > +        _Assert(&buf[i] < (buf + size - (sizeof(buf[0]) - 1)));
> > >> >          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
> > >> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210506/bc2599fc/attachment.html>


More information about the devel mailing list