Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple TcpSockets share the same static mut buffers #76

Closed
IsaacDynamo opened this issue Oct 22, 2022 · 1 comment · Fixed by #82
Closed

Multiple TcpSockets share the same static mut buffers #76

IsaacDynamo opened this issue Oct 22, 2022 · 1 comment · Fixed by #82
Labels
bug Something isn't working

Comments

@IsaacDynamo
Copy link

In function create_network_interface() from src/wifi/utils.rs

    for _ in 0..sockets_to_add {
        let rx_tx_socket1 = {
            static mut TCP_SERVER_RX_DATA: [u8; 1536] = [0; 1536];
            static mut TCP_SERVER_TX_DATA: [u8; 1536] = [0; 1536];

            let tcp_rx_buffer = unsafe { TcpSocketBuffer::new(&mut TCP_SERVER_RX_DATA[..]) };
            let tcp_tx_buffer = unsafe { TcpSocketBuffer::new(&mut TCP_SERVER_TX_DATA[..]) };

            TcpSocket::new(tcp_rx_buffer, tcp_tx_buffer)
        };
        ethernet.add_socket(rx_tx_socket1);
    }

When sockets_to_add is more than one, the static mutable variables TCP_SERVER_RX_DATA and TCP_SERVER_TX_DATA are used multiple times to create multiple TcpSocket's.

This will result in multiple mutable references to the same buffer which is unsound.

And as a consequence data is corrupted when multiple sockets are used at the same time.

@bjoernQ bjoernQ added the bug Something isn't working label Oct 24, 2022
@bjoernQ
Copy link
Contributor

bjoernQ commented Oct 24, 2022

Thanks for reporting this! This should get changed, certainly.

In the meantime, it's possible to not use the utils and do the initialization manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants