[PATCH] bsps/shared/ofw: Fix coverity reported defects
Christian MAUDERER
christian.mauderer at embedded-brains.de
Fri Jan 29 11:09:35 UTC 2021
Hello Niteesh,
Am 29.01.21 um 09:58 schrieb Niteesh G. S.:
> Hello Christian,
>
> On Fri, Jan 29, 2021 at 1:47 PM Christian MAUDERER
> <christian.mauderer at embedded-brains.de
> <mailto: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>
> > <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?
Depends.
Asserts can be removed in a production build. If the interface is a
non-public one that is only used RTEMS internal with fixed sizes (like
sizeof(uint32_t)), the assert isn't wrong. It's a programming error that
would be visible with the assert.
If a user can use the interface, a "return SOME_ERROR_CODE" might would
be better.
Best regards
Christian
>
> 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>
> > <mailto: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
> <mailto: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/
> <https://embedded-brains.de/datenschutzerklaerung/>
>
--
--------------------------------------------
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/
More information about the devel
mailing list