-
Notifications
You must be signed in to change notification settings - Fork 269
Rust sockets can be added to the qdisc more than once #2704
Description
When a socket wants to send, the network interface checks if the socket already exists in the qdisc before adding it.
shadow/src/main/host/network_interface.c
Lines 558 to 575 in b349261
| /* track the new socket for sending if not already tracking */ | |
| switch (interface->qdisc) { | |
| case Q_DISC_MODE_ROUND_ROBIN: { | |
| if (!rrsocketqueue_find(&interface->rrQueue, socket)) { | |
| CompatSocket newSocketRef = compatsocket_refAs(socket); | |
| rrsocketqueue_push(&interface->rrQueue, &newSocketRef); | |
| } | |
| break; | |
| } | |
| case Q_DISC_MODE_FIFO: | |
| default: { | |
| if (!fifosocketqueue_find(&interface->fifoQueue, socket)) { | |
| CompatSocket newSocketRef = compatsocket_refAs(socket); | |
| fifosocketqueue_push(&interface->fifoQueue, &newSocketRef); | |
| } | |
| break; | |
| } | |
| } |
The qdisc functions do this by checking if the CompatSockets tagged pointer (see compatsocket_toTagged) exists in the queue. The problem is that you can have multiple different tagged pointers that point to the same underlying socket. This isn't an issue for the LegacySockets since the tagged pointer is constructed using the LegacySockets address, but this is an issue for rust InetSockets where the tagged pointer is constructed using the InetSockets address, and there may be multiple InetSockets for a single TcpSocket object. This is because LegacySockets control their own reference counting, whereas a TcpSocket uses separate objects (Arcs) for reference counting, and each InetSocket contains a different Arc that might point to the same TcpSocket.
These qdisc queues should be checking if any sockets with the same canonical handle are in the queue, rather than checking if any sockets have the same socket address.
This bug isn't currently being triggered since we don't actually add rust sockets to the network interface yet.