[PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)

[Gedare Bloom]

On Wed, Apr 14, 2021 at 8:49 AM Joel Sherrill <joel at rtems.org> wrote:
>
>
>
> On Mon, Mar 15, 2021 at 5:09 PM Gedare Bloom <gedare at rtems.org> wrote:
>>
>> On Mon, Mar 15, 2021 at 2:27 PM Joel Sherrill <joel at rtems.org> wrote:
>> >
>> >
>> >
>> > On Mon, Mar 15, 2021, 3:21 PM Gedare Bloom <gedare at rtems.org> wrote:
>> >>
>> >> On Fri, Mar 12, 2021 at 7:55 AM Ryan Long <ryan.long at oarcorp.com> wrote:
>> >> >
>> >> > CID 26033: Dereference after null check in _Objects_Extend_information().
>> >> >
>> >> > Closes #4326
>> >> > ---
>> >> >  cpukit/score/src/objectextendinformation.c | 11 +++++++++++
>> >> >  1 file changed, 11 insertions(+)
>> >> >
>> >> > diff --git a/cpukit/score/src/objectextendinformation.c b/cpukit/score/src/objectextendinformation.c
>> >> > index 9796eb9..c669301 100644
>> >> > --- a/cpukit/score/src/objectextendinformation.c
>> >> > +++ b/cpukit/score/src/objectextendinformation.c
>> >> > @@ -171,6 +171,17 @@ Objects_Maximum _Objects_Extend_information(
>> >> >
>> >> >      if ( old_maximum > extend_count ) {
>> >> >        /*
>> >> > +       * Coverity thinks there is a way for this to be NULL (CID #26033).
>> >> > +       * After much time spent analyzing this, no one has identified the
>> >> > +       * conditions where this can actually occur. Adding this _Assert ensures
>> >> > +       * that it is never NULL. If this assert is triggered, condition
>> >> > +       * generating this case will have been identified and it can be revisted.
>> >> > +       * This is being done out of an abundance of caution since we could have
>> >> > +       * easily flagged this as a false positive and ignored it completely.
>> >> > +       */
>> >> > +      _Assert(information->object_blocks != NULL);
>> >> > +
>> >> That's interesting. It would help if you could share your analysis.
>> >
>> >
>> > This is the oldest Coverity issue that is open. It is over five years old. Chris and I have tried multiple times to figure out if it is valid. We never get any confidence that it cannot occur.
>> >
>> >>
>> >> How does
>> >> 70  if ( information->object_blocks == NULL ) {
>> >> be true, and if it is true, how does the exectuion flow proceed in
>> >> such a way that it will not reach this assert?
>> >
>> >
>> > No idea but it apparently doesn't based on our tests.
>> >
>> > Adding the assert is an attempt to finally find the case that trips this. It is either something I can never occur or something we don't know how to make happen. Either way the asserting like a good idea.
>> >
>> > if you have a test case in mind that can reproduce this coverity path, let's try it and push this to failure. But we have no evidence that it's ever occurred in the field.
>>
>> Can information->object_blocks be NULL at line 70?
>
>
> Based on the coverage report case here, the answer is yes.
>
> https://ftp.rtems.org/pub/rtems/people/joel/coverage/coverage-2021-02-28/xilinx_zynq_a9_qemu-coverage/score/annotated.html#range67
>
> Everything was covered in this method except that one error case. And it looks like there may be a covoar bug related to the branch that leads there since it says always taken and the destination is never executed. That can't happen.
>
> This particular case was first reported by Coverity in Jan 2010. Chris and I have looked at it multiple times and never can figure out how it would happen. But we are not confident enough to mark it as false positive. Thus the _Assert() should be tripped if this ever happens, Coverity should be satisfied, and Chris and I won't worry we ignored something.
>

OK

> --joel
>>
>>
>> >>
>> >>
>> >>
>> >> > +      /*
>> >> >         *  Copy each section of the table over. This has to be performed as
>> >> >         *  separate parts as size of each block has changed.
>> >> >         */
>> >> > --
>> >> > 1.8.3.1
>> >> >
>> >> > _______________________________________________
>> >> > devel mailing list
>> >> > devel at rtems.org
>> >> > http://lists.rtems.org/mailman/listinfo/devel
>> >> _______________________________________________
>> >> devel mailing list
>> >> devel at rtems.org
>> >> http://lists.rtems.org/mailman/listinfo/devel