Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)
On Wed, Apr 14, 2021 at 9:57 AM Gedare Bloom wrote:
>
> On Wed, Apr 14, 2021 at 8:49 AM Joel Sherrill wrote:
> >
> >
> >
> > On Mon, Mar 15, 2021 at 5:09 PM Gedare Bloom wrote:
> >>
> >> On Mon, Mar 15, 2021 at 2:27 PM Joel Sherrill wrote:
> >> >
> >> >
> >> >
> >> > On Mon, Mar 15, 2021, 3:21 PM Gedare Bloom wrote:
> >> >>
> >> >> On Fri, Mar 12, 2021 at 7:55 AM Ryan Long 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
>
Add the assert.
> > --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@rtems.org
> >> >> > http://lists.rtems.org/mailman/listinfo/devel
> >> >> ___
> >> >> devel mailing list
> >> >> devel@rtems.org
> >> >> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)
On Wed, Apr 14, 2021 at 8:49 AM Joel Sherrill wrote:
>
>
>
> On Mon, Mar 15, 2021 at 5:09 PM Gedare Bloom wrote:
>>
>> On Mon, Mar 15, 2021 at 2:27 PM Joel Sherrill wrote:
>> >
>> >
>> >
>> > On Mon, Mar 15, 2021, 3:21 PM Gedare Bloom wrote:
>> >>
>> >> On Fri, Mar 12, 2021 at 7:55 AM Ryan Long 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@rtems.org
>> >> > http://lists.rtems.org/mailman/listinfo/devel
>> >> ___
>> >> devel mailing list
>> >> devel@rtems.org
>> >> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)
On Mon, Mar 15, 2021 at 5:09 PM Gedare Bloom wrote:
> On Mon, Mar 15, 2021 at 2:27 PM Joel Sherrill wrote:
> >
> >
> >
> > On Mon, Mar 15, 2021, 3:21 PM Gedare Bloom wrote:
> >>
> >> On Fri, Mar 12, 2021 at 7:55 AM Ryan Long
> 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.
--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@rtems.org
> >> > http://lists.rtems.org/mailman/listinfo/devel
> >> ___
> >> devel mailing list
> >> devel@rtems.org
> >> http://lists.rtems.org/mailman/listinfo/devel
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)
On Mon, Mar 15, 2021 at 2:27 PM Joel Sherrill wrote:
>
>
>
> On Mon, Mar 15, 2021, 3:21 PM Gedare Bloom wrote:
>>
>> On Fri, Mar 12, 2021 at 7:55 AM Ryan Long 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?
>>
>>
>>
>> > + /*
>> > * 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@rtems.org
>> > http://lists.rtems.org/mailman/listinfo/devel
>> ___
>> devel mailing list
>> devel@rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)
On Mon, Mar 15, 2021, 3:21 PM Gedare Bloom wrote:
> On Fri, Mar 12, 2021 at 7:55 AM Ryan Long 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.
>
>
> > + /*
> > * 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@rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)
On Fri, Mar 12, 2021 at 7:55 AM Ryan Long 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.
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?
> + /*
> * 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@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)
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);
+
+ /*
* 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@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
