search

ofiwg / libfabric

Open Fabric Interfaces
http://libfabric.org/
Other
555 stars 376 forks source link

fabtests: Address integer overflow coverity issues in fabtests #10142

Closed Juee14Desai closed 3 months ago

Juee14Desai commented 3 months ago

There are 4 integer overflow issues in fabtests. All of them need a check that ensures integer overflow does not oocur.

  1. changes to harness.c: This modification includes a check to ensure that adding ret to m does not exceed SIZE_MAX.

  2. changes to shared.c: This modification includes a check before the addition to ensure that (rcvd + ret) or (sent + ret) does not exceed SIZE_MAX. If the addition would exceed SIZE_MAX, it sets err to an error value and breaks from the loop, preventing overflow.

ghost commented 3 months ago

This PR is not needed. This is the case where coverity is not aware of the APIs. We are calculating size used for send/recv calls to not exceed the value (size_t lens) passed to our functions. The for-loop/while-loop will only continue if there is more data. You should disposition the coverity issues as "False Positive" and move on. Now, I did not check all the loops for correctness, but I do see the intention of the code.

Juee14Desai commented 3 months ago

@chien-intel I agree with you. I myself was on the fence but coverity has marked it as a high priority issue and since it falls under CWE top 25 defects, I added this check. If there is a consensus on not needing this, I'll mark it as "False Positive" and close the PR.

@j-xiong Please let me know if you think this is not needed.

j-xiong commented 3 months ago

Yes, those are false positives. Coverity doesn't understand the semantics of ofi_send_socket() and ofi_recv_socket() and assumes arbitrary range of return values.

Juee14Desai commented 3 months ago

Coverity issues marked "False Positive". Closing the PR.