mirror of
https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake.git
synced 2025-10-13 20:11:19 -04:00
Revert "Take ownership of buffer in QueuePacketConn QueueIncoming/WriteTo."
This reverts commit 839d221883
. (Except for
the added benchmarks in queuepacketconn_test.go.) This change
corresponds to the issues #40187 and #40199.
The analysis in https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40199
was wrong; kcp-go does reuse the buffers it passes to
QueuePacketConn.WriteTo. This led to unsynchronized reuse of packet
buffers and mangled packets observable at the client:
https://gitlab.torproject.org/tpo/anti-censorship/pluggable-transports/snowflake/-/issues/40260.
Undoing the change in QueuePacketConn.QueueIncoming as well, for
symmetry, even though it is not implicated in any correctness problems.
This commit is contained in:
parent
b63d2272bf
commit
d2858aeb7e
1 changed files with 13 additions and 9 deletions
|
@ -42,9 +42,8 @@ func NewQueuePacketConn(localAddr net.Addr, timeout time.Duration) *QueuePacketC
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// QueueIncoming queues an incoming packet and its source address, to be
|
// QueueIncoming queues and incoming packet and its source address, to be
|
||||||
// returned in a future call to ReadFrom. This function takes ownership of p and
|
// returned in a future call to ReadFrom.
|
||||||
// the caller must not reuse it.
|
|
||||||
func (c *QueuePacketConn) QueueIncoming(p []byte, addr net.Addr) {
|
func (c *QueuePacketConn) QueueIncoming(p []byte, addr net.Addr) {
|
||||||
select {
|
select {
|
||||||
case <-c.closed:
|
case <-c.closed:
|
||||||
|
@ -52,8 +51,11 @@ func (c *QueuePacketConn) QueueIncoming(p []byte, addr net.Addr) {
|
||||||
return
|
return
|
||||||
default:
|
default:
|
||||||
}
|
}
|
||||||
|
// Copy the slice so that the caller may reuse it.
|
||||||
|
buf := make([]byte, len(p))
|
||||||
|
copy(buf, p)
|
||||||
select {
|
select {
|
||||||
case c.recvQueue <- taggedPacket{p, addr}:
|
case c.recvQueue <- taggedPacket{buf, addr}:
|
||||||
default:
|
default:
|
||||||
// Drop the incoming packet if the receive queue is full.
|
// Drop the incoming packet if the receive queue is full.
|
||||||
}
|
}
|
||||||
|
@ -82,20 +84,22 @@ func (c *QueuePacketConn) ReadFrom(p []byte) (int, net.Addr, error) {
|
||||||
}
|
}
|
||||||
|
|
||||||
// WriteTo queues an outgoing packet for the given address. The queue can later
|
// WriteTo queues an outgoing packet for the given address. The queue can later
|
||||||
// be retrieved using the OutgoingQueue method. This function takes ownership of
|
// be retrieved using the OutgoingQueue method.
|
||||||
// p and the caller must not reuse it.
|
|
||||||
func (c *QueuePacketConn) WriteTo(p []byte, addr net.Addr) (int, error) {
|
func (c *QueuePacketConn) WriteTo(p []byte, addr net.Addr) (int, error) {
|
||||||
select {
|
select {
|
||||||
case <-c.closed:
|
case <-c.closed:
|
||||||
return 0, &net.OpError{Op: "write", Net: c.LocalAddr().Network(), Addr: c.LocalAddr(), Err: c.err.Load().(error)}
|
return 0, &net.OpError{Op: "write", Net: c.LocalAddr().Network(), Addr: c.LocalAddr(), Err: c.err.Load().(error)}
|
||||||
default:
|
default:
|
||||||
}
|
}
|
||||||
|
// Copy the slice so that the caller may reuse it.
|
||||||
|
buf := make([]byte, len(p))
|
||||||
|
copy(buf, p)
|
||||||
select {
|
select {
|
||||||
case c.clients.SendQueue(addr) <- p:
|
case c.clients.SendQueue(addr) <- buf:
|
||||||
return len(p), nil
|
return len(buf), nil
|
||||||
default:
|
default:
|
||||||
// Drop the outgoing packet if the send queue is full.
|
// Drop the outgoing packet if the send queue is full.
|
||||||
return len(p), nil
|
return len(buf), nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue