mirror of
https://github.com/RGBCube/serenity
synced 2025-07-04 18:27:35 +00:00
Kernel/Net: Iron out the locking mechanism across the subsystem
There is a big mix of LockRefPtrs all over the Networking subsystem, as well as lots of room for improvements with our locking patterns, which this commit will not pursue, but will give a good start for such work. To deal with this situation, we change the following things: - Creating instances of NetworkAdapter should always yield a non-locking NonnullRefPtr. Acquiring an instance from the NetworkingManagement should give a simple RefPtr,as giving LockRefPtr does not really protect from concurrency problems in such case. - Since NetworkingManagement works with normal RefPtrs we should protect all instances of RefPtr<NetworkAdapter> with SpinlockProtected to ensure references are gone unexpectedly. - Protect the so_error class member with a proper spinlock. This happens to be important because the clear_so_error() method lacked any proper locking measures. It also helps preventing a possible TOCTOU when we might do a more fine-grained locking in the Socket code, so this could be definitely a start for this. - Change unnecessary LockRefPtr<PacketWithTimestamp> in the structure of OutgoingPacket to a simple RefPtr<PacketWithTimestamp> as the whole list should be MutexProtected.
This commit is contained in:
parent
bd7d4513bf
commit
7c1f645e27
13 changed files with 93 additions and 83 deletions
|
@ -202,7 +202,8 @@ ErrorOr<size_t> TCPSocket::protocol_receive(ReadonlyBytes raw_ipv4_packet, UserO
|
|||
|
||||
ErrorOr<size_t> TCPSocket::protocol_send(UserOrKernelBuffer const& data, size_t data_length)
|
||||
{
|
||||
RoutingDecision routing_decision = route_to(peer_address(), local_address(), bound_interface());
|
||||
auto adapter = bound_interface().with([](auto& bound_device) -> RefPtr<NetworkAdapter> { return bound_device; });
|
||||
RoutingDecision routing_decision = route_to(peer_address(), local_address(), adapter);
|
||||
if (routing_decision.is_zero())
|
||||
return set_so_error(EHOSTUNREACH);
|
||||
size_t mss = routing_decision.adapter->mtu() - sizeof(IPv4Packet) - sizeof(TCPPacket);
|
||||
|
@ -220,7 +221,8 @@ ErrorOr<void> TCPSocket::send_ack(bool allow_duplicate)
|
|||
|
||||
ErrorOr<void> TCPSocket::send_tcp_packet(u16 flags, UserOrKernelBuffer const* payload, size_t payload_size, RoutingDecision* user_routing_decision)
|
||||
{
|
||||
RoutingDecision routing_decision = user_routing_decision ? *user_routing_decision : route_to(peer_address(), local_address(), bound_interface());
|
||||
auto adapter = bound_interface().with([](auto& bound_device) -> RefPtr<NetworkAdapter> { return bound_device; });
|
||||
RoutingDecision routing_decision = user_routing_decision ? *user_routing_decision : route_to(peer_address(), local_address(), adapter);
|
||||
if (routing_decision.is_zero())
|
||||
return set_so_error(EHOSTUNREACH);
|
||||
|
||||
|
@ -409,13 +411,14 @@ NetworkOrdered<u16> TCPSocket::compute_tcp_checksum(IPv4Address const& source, I
|
|||
|
||||
ErrorOr<void> TCPSocket::protocol_bind()
|
||||
{
|
||||
if (has_specific_local_address() && !m_adapter) {
|
||||
m_adapter = NetworkingManagement::the().from_ipv4_address(local_address());
|
||||
if (!m_adapter)
|
||||
return set_so_error(EADDRNOTAVAIL);
|
||||
}
|
||||
|
||||
return {};
|
||||
return m_adapter.with([this](auto& adapter) -> ErrorOr<void> {
|
||||
if (has_specific_local_address() && !adapter) {
|
||||
adapter = NetworkingManagement::the().from_ipv4_address(local_address());
|
||||
if (!adapter)
|
||||
return set_so_error(EADDRNOTAVAIL);
|
||||
}
|
||||
return {};
|
||||
});
|
||||
}
|
||||
|
||||
ErrorOr<void> TCPSocket::protocol_listen(bool did_allocate_port)
|
||||
|
@ -598,7 +601,8 @@ void TCPSocket::retransmit_packets()
|
|||
return;
|
||||
}
|
||||
|
||||
auto routing_decision = route_to(peer_address(), local_address(), bound_interface());
|
||||
auto adapter = bound_interface().with([](auto& bound_device) -> RefPtr<NetworkAdapter> { return bound_device; });
|
||||
auto routing_decision = route_to(peer_address(), local_address(), adapter);
|
||||
if (routing_decision.is_zero())
|
||||
return;
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue