search

mirage / mirage-tcpip

TCP/IP networking stack in pure OCaml, using the Mirage platform libraries. Includes IPv4/6, ICMP, and UDP/TCP support.
https://mirage.io
ISC License
341 stars 87 forks source link

Possible data corruption in User_buffer #486

Closed ansiwen closed 2 years ago

ansiwen commented 2 years ago

The write function in User_buffer possibly copies Cstruct values into a list and returns:

https://github.com/mirage/mirage-tcpip/blob/a7b799cb36435ecb810a92df05ff49fbd264bfbb/src/tcp/user_buffer.ml#L256-L277

which basically means it takes ownership, because copies of Cstruct share the data and it's not transparent when they will be flushed. This leads to problems, when code assumes otherwise and reuses the buffers after write returns, like the Faraday library. This leads to issues like this: https://github.com/anmonteiro/gluten/issues/29

I open this issue to clarify the contract conventions (if there are any) for the write function. Is the buffer ownership transferred, or only "borrowed" until write returns?

haesbaert commented 2 years ago

I'm not sure the authors considered this much, but as you can see only the very last case (transmit_segments) is not a clear offender. We should at least document it.

haesbaert commented 2 years ago

Seems we missed this ? https://github.com/mirage/mirage-flow/blob/e661203b4305b6ea34d625ec725337d854c4ae83/src/mirage_flow.mli#L78-L84

ansiwen commented 2 years ago

Ok, so ownership is transferred. Thanks. So I will close this.