Re: [PATCH v2] bsps/shared/ofw: Fix coverity defects
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 wrote:
> Hi all,
>
> On Wed, May 5, 2021 at 8:42 AM Gedare Bloom 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 = 0x0030 R10 = 0x80104640
> R3 = 0x80104648 R11 = 0x0002
> R4 = 0x0008 R12 = 0x8001e68b
> R5 = 0x2ebc SP = 0x801045b8
> R6 = 0x80104640 LR = 0x8000d720
> R7 = 0x80101e28 PC = 0x80011ba8
> CPSR = 0x8193 VEC = 0x0004
> 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-2-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.
> wrote:
> > >
> > >
> > >
> > > On Mon, May 3, 2021 at 11:23 PM Gedare Bloom 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
> 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
> > >> > #include
> > >> > #include
> > >> > +#include
> > >> >
> > >> > 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([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
> > >> >
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2] bsps/shared/ofw: Fix coverity defects
Hi all,
On Wed, May 5, 2021 at 8:42 AM Gedare Bloom 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 = 0x0030 R10 = 0x80104640
R3 = 0x80104648 R11 = 0x0002
R4 = 0x0008 R12 = 0x8001e68b
R5 = 0x2ebc SP = 0x801045b8
R6 = 0x80104640 LR = 0x8000d720
R7 = 0x80101e28 PC = 0x80011ba8
CPSR = 0x8193 VEC = 0x0004
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-2-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. wrote:
> >
> >
> >
> > On Mon, May 3, 2021 at 11:23 PM Gedare Bloom 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
> >> 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
> >> > #include
> >> > #include
> >> > +#include
> >> >
> >> > 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([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
> >> >
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2] bsps/shared/ofw: Fix coverity defects
alright looks good. Vijay or Christian please confirm and push if
you're good with it too.
On Wed, May 5, 2021 at 12:52 AM Niteesh G. S. wrote:
>
>
>
> On Mon, May 3, 2021 at 11:23 PM Gedare Bloom 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 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
>> > #include
>> > #include
>> > +#include
>> >
>> > 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([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
>> >
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2] bsps/shared/ofw: Fix coverity defects
On Mon, May 3, 2021 at 11:23 PM Gedare Bloom 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
> 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
> > #include
> > #include
> > +#include
> >
> > 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([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
> >
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2] bsps/shared/ofw: Fix coverity defects
Hi Niteesh,
This looks good to me. What/how did you test it?
Gedare
On Sat, May 1, 2021 at 6:31 AM G S Niteesh Babu 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
> #include
> #include
> +#include
>
> 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([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
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v2] bsps/shared/ofw: Fix coverity defects
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
#include
#include
+#include
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([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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
