[haiku-commits] Re: haiku: hrev53089 - src/add-ons/kernel/network/stack

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Tue, 23 Apr 2019 14:05:54 -0400

On Tue, Apr 23, 2019 at 9:44 AM Adrien Destugues
<pulkomandy@xxxxxxxxxxxxx> wrote:

This should be in Gerrit discussions, and if the issue is non-obvious, 
possibly lead to a comment
in the code. Now it's lost in IRC logs which are not easily related to the 
code in question.

Well, there wasn't a comment about this when I came to this code
originally, and I'm not sure where one should be added...

Admittedly there is still a problem that this depends on internal FIFO 
behavior on destruction, and
there is still a small race related to buffer management. How to resolve 
that, I don't really know
as it would probably require a much more substantial rework of this code. 
This change is at least
much more correct and should stop the KDLs people have been getting.

So jessicah is right, this is still incorrect, even if it is less likely to 
fail.

You cannot safely test if an object has been deleted, because another object 
of the same type may be
allocated at the same address (bad luck, but it can happen).

No, there is no use-after-free here. The FIFO does not own the buffers
it is returning; once it returns the buffer, it no longer owns it; and
while it is still inside dequeue(), it holds a lock which uninit_fifo
has to grab before it can do the teardown. So if we tear down the FIFO
while the consumer_thread is still processing the buffer it got from
the FIFO, that's just fine, as us destroying the FIFO does not affect
that buffer.

-waddlesplash

Other related posts: