smoltcp icon indicating copy to clipboard operation
smoltcp copied to clipboard

recv_slice for RawSocket, IcmpSocket and UdpSocket may lose data

Open trinity-1686a opened this issue 4 years ago • 1 comments

recv_slice for RawSocket, IcmpSocket and UdpSocket may lose data if the provided buffer is smaller than the packet to dequeue.

I don't know if it should not dequeue and return an error, as it might not be possible to get a bigger slice when lacking an allocator, or stay with current behavior. However I think the documentation should be clearer as it seems to say nothing about that.

trinity-1686a avatar Mar 23 '20 01:03 trinity-1686a

Good catch! I seem to have overlooked that while adding these functions. It is possible to use PacketBuffer::dequeue_with to return an error in this case (probably Error::Truncated).

What we should do in this case is an interesting question. The recv_slice methods are, essentially, convenience methods, so any limitations they have do not fundamentally alter the capabilities of the socket. If recv_slice returns an error and does not dequeue, then the caller must handle that case to avoid getting stuck in a receive loop and use recv to dequeue the overlong packet. But that seems to defeat the purpose of using recv_slice.

Therefore I argue that it should return an error (probably Error::Truncated) and dequeue the overlong packet, since that's what the caller will have to do in any case where it can use recv_slice . In any other case recv should be used rather than recv_slice.

whitequark avatar Mar 23 '20 13:03 whitequark

I this still an issue?

stlankes avatar Jul 20 '23 19:07 stlankes

cc @Dirbaio

whitequark avatar Jul 20 '23 19:07 whitequark

Fixed by https://github.com/smoltcp-rs/smoltcp/pull/859.

whitequark avatar Nov 27 '23 14:11 whitequark