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

[Niteesh G. S.]

Hello Christian,

On Fri, Jan 29, 2021 at 1:47 PM Christian MAUDERER <
christian.mauderer at embedded-brains.de> wrote:

> Hello Niteesh,
>
> Am 29.01.21 um 07:33 schrieb Niteesh G. S.:
> > Hello,
> >
> > https://lists.rtems.org/pipermail/devel/2021-January/064115.html
> > <https://lists.rtems.org/pipermail/devel/2021-January/064115.html>
> > I have fixed defects reported in the above thread except
> > CID 1472595, 1472597 (ARRAY_VS_SINGLETON)
> > Along with the buffer we also take the size of the buffer this makes
> > sure that we don't read more than the buffer capacity.
> > But coverity reports this as an defect how can I fix this?
>
> Thanks for taking a look at these. Regarding the CID 1472595:
>
> What Coverity doesn't like is the following:
>
> You create a variable (phandle_t ref). You then pass the pointer of the
> variable to the rtems_ofw_get_enc_prop in the pcell_t *buf parameter.
> And rtems_ofw_get_enc_prop uses a buf[i] which might could be more than
> 0 if you ask Coverity. Coverity is most likely not intelligent enough to
> understand the logic. I think it might be possible to suppress that
> message by creating the variable with
>
>      phandle_t ref[1];
>
> and adapting the following uses of that variable. But please double
> check, that the code is correct when you do something like that. What
> isn't mentioned in that defect: In rtems_ofw_get_enc_prop you pass the
> buf to rtems_ofw_get_prop which uses a strncpy in line 201 to fill the
> buffer. That is _very_ odd for an integer value and might lead to wrong
> results.
>
Before passing the buffer to rtems_ofw_get_prop we also make sure it is
a multiple of 4 using the assert.
assert(len % 4 == 0);
Is an assert the right way to make sure length is a multiple of 4, Can't
they be
removed during build?

Thanks,
Niteesh.

>
> Best regards
>
> Christian
>
> >
> > Thanks,
> > Niteesh.
> >
> > On Fri, Jan 29, 2021 at 11:48 AM G S Niteesh Babu <niteesh.gs at gmail.com
> > <mailto:niteesh.gs at gmail.com>> wrote:
> >
> >     Fixed use after free and null pointer dereference defects
> >
> >     FIXES:
> >     1) CID 1472601 (NULL_RETURNS)
> >     2) CID 1472600 (USE_AFTER_FREE)
> >     3) CID 1472599 (USE_AFTER_FREE)
> >     4) CID 1472598 (USE_AFTER_FREE)
> >     5) CID 1472596 (USE_AFTER_FREE)
> >     ---
> >       bsps/shared/ofw/ofw.c | 10 +++++-----
> >       1 file changed, 5 insertions(+), 5 deletions(-)
> >
> >     diff --git a/bsps/shared/ofw/ofw.c b/bsps/shared/ofw/ofw.c
> >     index 82924b2600..fa94bfbf05 100644
> >     --- a/bsps/shared/ofw/ofw.c
> >     +++ b/bsps/shared/ofw/ofw.c
> >     @@ -313,7 +313,7 @@ ssize_t rtems_ofw_get_prop_alloc(
> >           }
> >
> >           if (rtems_ofw_get_prop(node, propname, *buf, len) == -1) {
> >     -      rtems_ofw_free(buf);
> >     +      rtems_ofw_free(*buf);
> >             *buf = NULL;
> >             return -1;
> >           }
> >     @@ -344,7 +344,7 @@ ssize_t rtems_ofw_get_prop_alloc_multi(
> >           }
> >
> >           if (rtems_ofw_get_prop(node, propname, *buf, len) == -1) {
> >     -      rtems_ofw_free(buf);
> >     +      rtems_ofw_free(*buf);
> >             *buf = NULL;
> >             return -1;
> >           }
> >     @@ -373,7 +373,7 @@ ssize_t rtems_ofw_get_enc_prop_alloc(
> >           }
> >
> >           if (rtems_ofw_get_enc_prop(node, propname, *buf, len) == -1) {
> >     -      rtems_ofw_free(buf);
> >     +      rtems_ofw_free(*buf);
> >             *buf = NULL;
> >             return -1;
> >           }
> >     @@ -404,7 +404,7 @@ ssize_t rtems_ofw_get_enc_prop_alloc_multi(
> >           }
> >
> >           if (rtems_ofw_get_enc_prop(node, propname, *buf, len) == -1) {
> >     -      rtems_ofw_free(buf);
> >     +      rtems_ofw_free(*buf);
> >             *buf = NULL;
> >             return -1;
> >           }
> >     @@ -614,7 +614,7 @@ int rtems_ofw_get_reg(
> >           offset = rtems_fdt_phandle_to_offset(parent);
> >           ptr = fdt_getprop(fdtp, offset, "ranges", &len);
> >
> >     -    if (len < 0) {
> >     +    if (ptr == NULL) {
> >             break;
> >           }
> >
> >     --
> >     2.17.1
> >
>
> --
> --------------------------------------------
> embedded brains GmbH
> Herr Christian MAUDERER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: christian.mauderer at embedded-brains.de
> phone: +49-89-18 94 741 - 18
> fax:   +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210129/c1883dfe/attachment-0001.html>