Possible false remove in Face::cancelPendingInterest (address used as handle)
expressInterest returns the memory address of the interest copy as the PendingInterestId.
And removePendingInterest uses this memory address to find the entry to remove it from the PIT.
Note that removePendingInterest is supposed to do nothing if the interest is no longer in the PIT.
Therefore, the following failure mode is possible:
- The application calls expressInterest for interest 1 and receives memory address X of the interest copy as the PendingInterestId.
- The application keeps X.
- A data packet is received, the library removes interest 1 from the PIT, freeing the memory of the interest.
- The application calls expressInterest again for interest 2. (This returned PendingInterestId is ignored.) The library just happens to use the same memory address X for the interest copy.
- Another part of the application wants to cancel the original interest 1, and calls removePendingInterest(X).
- Interest 1 is no longer in the PIT. But the memory address X is re-used as the same PendingInterestId for interest 2, so the library falsely removes interest 2 from the PIT.
Updated by Davide Pesavento over 1 year ago
- Subject changed from Possible false remove in removePendingInterest (address used as ID) to Possible false remove in Face::cancelPendingInterest (address used as handle)
- Target version changed from Unsupported to v0.7
- Start date deleted (
The described sequence of events has become a lot more likely with the introduction of
One of its most common usage patterns is assigning the result of
expressInterest to a class member field of type
ScopedPendingInterestHandle when re-expressing an Interest that has timed out or has been nacked. As Jeff said,
expressInterest may allocate a copy of the new Interest at the same memory address as the timed-out/nacked Interest. If that happens, when
expressInterest returns and its result is assigned to the handle, the move assignment operator will try to cancel the old (timed-out) Interest but it will in fact end up canceling the new (just re-expressed) Interest.
 In some circumstances, if the pattern of memory (de)allocations repeats itself over and over, the memory addresses of newly allocated objects are very predictable (same address >90% of the time in my tests).
Updated by Junxiao Shi over 1 year ago
The same bug also occurs on
RegisteredPrefixId. Since they aren't causing test failures now, I'll fix them in #3919.
we should eventually (after we've removed PendingInterestId from the public API) switch to
using PendingInterestId = std::uintptr_t;and change all "const PendingInterestId*" to just "PendingInterestId"
Yes, and probably more than that.