The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=255af72c8059b0117db646f82efa2e4848fa7570
commit 255af72c8059b0117db646f82efa2e4848fa7570 Author: John Baldwin <[email protected]> AuthorDate: 2025-12-10 15:30:31 +0000 Commit: John Baldwin <[email protected]> CommitDate: 2025-12-10 15:30:31 +0000 netlink: Don't overwrite existing data in a linear buffer in snl_writer First, a bit of background on some of the data structures netlink uses to manage data associated with a netlink connection. - struct linear_buffer contains a single virtually-contiguous buffer of bytes. Regions of this buffer are suballocated via lb_allocz() which uses a simple "bump" where the buffer is split into an allocated region at the start and a free region at the end. Each allocation "bumps" the boundary (lb->offset) forward by the allocation size. Individual allocations are not freed. Instead, the entire buffer is freed once all of the allocations are no longer in use. Linear buffers also contain an embedded link to permit chaining buffers together. - snl_state contains various state for a netlink connection including a chain of linear buffers. This chain of linear buffers can contain allocations for netlink messages as well as other ancillary data buffers such as socket address structures. The chain of linear buffers are freed once the connection is torn down. - snl_writer is used to construct a message written on a netlink connection. It contains a single virtually-contiguous buffer (nw->base) allocated from the associated snl_state's linear buffer chain. The buffer distinguishes between the amount of space reserved from the underlying allocator (nw->size) and the current message length actually written (nw->offset). As new chunks of data (e.g. netlink attributes) are added to the write buffer, the buffer is grown by snl_realloc_msg_buffer by reallocating a larger buffer from the associated snl_state and copying over the current message data to the new buffer. Commit 0c511bafdd5b309505c13c8dc7c6816686d1e103 aimed to fix two bugs in snl_realloc_msg_buffer. The first bug is that snl_realloc_msg_buffer originally failed to update nw->size after growing the buffer which could result in spurious re-allocations when growing in the future. It also probably could eventually lead to overflowing the buffer since each reallocation request was just adding the new bytes needed for a chunk to the original 'nw->size' while 'nw->offset' kept growing. Eventually the new 'nw->offset' would be larger than 'nw->size + sz' causing routines like snl_reserve_msg_data_raw() to return an out-of-bounds pointer. The second change in this commit I think was trying to fix the buffer overflows due to 'nw->size' being wrong, but instead introduced a new set of bugs. The second change ignored the returned pointer from snl_allocz() and instead assumed it could use all of the currently-allocated data in the current linear buffer. This is only ok if the only data in the linear buffer chain for the associated snl_state is the snl_writer's message buffer. If there is any other data allocated from the snl_state, it could be earlier in the current linear buffer, so resetting new_base to nw->ss->lb->base can result in overwriting that other data. The second change was also over-allocating storage from the underlying chain of linear buffers (e.g. a writer allocation of 256 followed by 512 would end up using the first 512 bytes, but 768 bytes would be reserved in the underlying linear buffer). To fix, revert the second change keeping only the fix for 'nw->size' being wrong. Reviewed by: igoro, markj Fixes: 0c511bafdd5b ("netlink: fix snl_writer and linear_buffer re-allocation logic") Sponsored by: AFRL, DARPA Differential Revision: https://reviews.freebsd.org/D54148 --- sys/netlink/netlink_snl.h | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/sys/netlink/netlink_snl.h b/sys/netlink/netlink_snl.h index 57f7e1e29d08..52b35e502c98 100644 --- a/sys/netlink/netlink_snl.h +++ b/sys/netlink/netlink_snl.h @@ -1083,6 +1083,7 @@ static inline bool snl_realloc_msg_buffer(struct snl_writer *nw, size_t sz) { uint32_t new_size = nw->size * 2; + char *new_base; while (new_size < nw->size + sz) new_size *= 2; @@ -1090,23 +1091,20 @@ snl_realloc_msg_buffer(struct snl_writer *nw, size_t sz) if (nw->error) return (false); - if (snl_allocz(nw->ss, new_size) == NULL) { + new_base = snl_allocz(nw->ss, new_size); + if (new_base == NULL) { nw->error = true; return (false); } - nw->size = new_size; - void *new_base = nw->ss->lb->base; - if (new_base != nw->base) { - memcpy(new_base, nw->base, nw->offset); - if (nw->hdr != NULL) { - int hdr_off = (char *)(nw->hdr) - nw->base; + memcpy(new_base, nw->base, nw->offset); + if (nw->hdr != NULL) { + int hdr_off = (char *)(nw->hdr) - nw->base; - nw->hdr = (struct nlmsghdr *) - (void *)((char *)new_base + hdr_off); - } - nw->base = (char *)new_base; + nw->hdr = (struct nlmsghdr *)(void *)(new_base + hdr_off); } + nw->base = new_base; + nw->size = new_size; return (true); }
