From 84c5a7b0cdb263f5d1cf3abea47957dc53c7c6d0 Mon Sep 17 00:00:00 2001 From: Christian Helmuth Date: Thu, 26 Jan 2023 14:44:26 +0100 Subject: [PATCH] vfs_lwip: receive payload without breaking refcount lwip uses a sophisticated reference-counting scheme in chains of pbufs, which the former manual implementation of read() for TCP data broke. Using pbuf_free_header() keeps the chain intact and also relieves our implementation from the burden of "offset" maintenance. Fixes #4722 --- repos/libports/src/lib/vfs/lwip/nic_netif.h | 4 +- repos/libports/src/lib/vfs/lwip/vfs.cc | 43 ++++++++------------- 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/repos/libports/src/lib/vfs/lwip/nic_netif.h b/repos/libports/src/lib/vfs/lwip/nic_netif.h index 229e784df9..a93771de53 100644 --- a/repos/libports/src/lib/vfs/lwip/nic_netif.h +++ b/repos/libports/src/lib/vfs/lwip/nic_netif.h @@ -401,8 +401,8 @@ class Lwip::Nic_netif * pbuf into the packet. */ char *dst = tx.packet_content(packet); - for(struct pbuf *q = p; q != 0; q = q->next) { - char const *src = (char*)q->payload; + for (struct pbuf *q = p; q != nullptr; q = q->next) { + char const *src = (char const *)q->payload; Genode::memcpy(dst, src, q->len); dst += q->len; } diff --git a/repos/libports/src/lib/vfs/lwip/vfs.cc b/repos/libports/src/lib/vfs/lwip/vfs.cc index b84bcf7e10..ce808a14e9 100644 --- a/repos/libports/src/lib/vfs/lwip/vfs.cc +++ b/repos/libports/src/lib/vfs/lwip/vfs.cc @@ -1113,7 +1113,6 @@ class Lwip::Tcp_socket_dir final : /* queue of received data */ pbuf *_recv_pbuf = nullptr; - u16_t _recv_off = 0; Open_result _accept_new_socket(Vfs::File_system &fs, Genode::Allocator &alloc, @@ -1195,13 +1194,17 @@ class Lwip::Tcp_socket_dir final : /** * chain a buffer to the queue */ - void recv(struct pbuf *buf) + err_t recv(struct pbuf *buf) { - if (_recv_pbuf && buf) { + if (!buf) + return ERR_ARG; + + if (_recv_pbuf) pbuf_cat(_recv_pbuf, buf); - } else { + else _recv_pbuf = buf; - } + + return ERR_OK; } /** @@ -1325,24 +1328,10 @@ class Lwip::Tcp_socket_dir final : : Read_result::READ_OK; } - u16_t const ucount = count; - u16_t const n = pbuf_copy_partial(_recv_pbuf, dst, ucount, _recv_off); - _recv_off += n; - { - u16_t new_off; - pbuf *new_head = pbuf_skip(_recv_pbuf, _recv_off, &new_off); - if (new_head != NULL && new_head != _recv_pbuf) { - /* increment the references on the new head */ - pbuf_ref(new_head); - /* free the buffers chained to the old head */ - pbuf_free(_recv_pbuf); - } + u16_t const ucount = min(count, (file_size)0xffff); + u16_t const n = pbuf_copy_partial(_recv_pbuf, dst, ucount, 0); - if (!new_head) - pbuf_free(_recv_pbuf); - _recv_pbuf = new_head; - _recv_off = new_off; - } + _recv_pbuf = pbuf_free_header(_recv_pbuf, n); /* ACK the remote */ if (_pcb) @@ -1358,8 +1347,8 @@ class Lwip::Tcp_socket_dir final : case Lwip_file_handle::PEEK: if (_recv_pbuf != nullptr) { - u16_t const ucount = count; - u16_t const n = pbuf_copy_partial(_recv_pbuf, dst, ucount, _recv_off); + u16_t const ucount = min(count, (file_size)0xffff); + u16_t const n = pbuf_copy_partial(_recv_pbuf, dst, ucount, 0); out_count = n; } return Read_result::READ_OK; @@ -1658,16 +1647,18 @@ err_t tcp_recv_callback(void *arg, struct tcp_pcb *pcb, struct pbuf *p, err_t) return ERR_ABRT; } + err_t err = ERR_OK; + Lwip::Tcp_socket_dir *socket_dir = static_cast(arg); if (p == NULL) { socket_dir->shutdown(); } else { - socket_dir->recv(p); + err = socket_dir->recv(p); } socket_dir->wakeup_vfs_user(); socket_dir->process_read_ready(); - return ERR_OK; + return err; }