Re: [PATCH net v2 1/1] packet: synchronize pressure clearing with ring reconfiguration
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2026-07-28 11:44:24
Ren Wei wrote:
From: Zihan Xi <redacted> packet_set_ring() updates the RX ring state under sk_receive_queue.lock, but publishes the tpacket receive mode through po->prot_hook.func after releasing that lock. packet_poll() and packet_recvmsg() can therefore run the pressure clearing path after the ring has been cleared while still seeing tpacket_rcv, causing __packet_rcv_has_room() to dereference stale or NULL ring storage. Serialize pressure clearing with RX ring reconfiguration and update the receive hook while holding the same queue lock when changing the RX ring. Keep packet_poll() on the unlocked helper,
Because it already holds the lock.
and let packet_recvmsg() take the queue lock only when PACKET_SOCK_PRESSURE is already set
which is sufficient, because if the socket moves from tpacket_rcv to packet_rcv, the socket is detached and a synchronize_net has passed, so no new packets could arrive to set PACKET_SOCK_PRESSURE if it was unset.
quoted hunk ↗ jump to hunk
so the fix avoids an unconditional extra lock acquisition on the normal recv path. Fixes: 2ccdbaa6d55b ("packet: rollover lock contention avoidance") Cc: stable@vger.kernel.org Reported-by: Vega <redacted> Assisted-by: Codex:gpt-5.4 Signed-off-by: Zihan Xi <redacted> Signed-off-by: Ren Wei <redacted> --- changes in v2: - only take sk_receive_queue.lock in packet_recvmsg() when PACKET_SOCK_PRESSURE is already set - keep packet_poll() on the unlocked helper under its existing queue lock - v1 Link: https://lore.kernel.org/all/24f7311aed0c9ff06b8ea982647b82bf543ec369.1784454542.git.xizh2024@lzu.edu.cn/ (local) net/packet/af_packet.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 8e6f3a734ba0..0107e55de6ff 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c@@ -1315,13 +1315,25 @@ static int packet_rcv_has_room(struct packet_sock *po, struct sk_buff *skb) return ret; } -static void packet_rcv_try_clear_pressure(struct packet_sock *po) +static void __packet_rcv_try_clear_pressure(struct packet_sock *po) { if (packet_sock_flag(po, PACKET_SOCK_PRESSURE) && __packet_rcv_has_room(po, NULL) == ROOM_NORMAL) packet_sock_flag_set(po, PACKET_SOCK_PRESSURE, false); } +static void packet_rcv_try_clear_pressure(struct packet_sock *po) +{ + struct sock *sk = &po->sk; + + if (!packet_sock_flag(po, PACKET_SOCK_PRESSURE)) + return; + + spin_lock_bh(&sk->sk_receive_queue.lock); + __packet_rcv_try_clear_pressure(po); + spin_unlock_bh(&sk->sk_receive_queue.lock); +} + static void packet_sock_destruct(struct sock *sk) { skb_queue_purge(&sk->sk_error_queue);@@ -4304,7 +4316,7 @@ static __poll_t packet_poll(struct file *file, struct socket *sock, TP_STATUS_KERNEL)) mask |= EPOLLIN | EPOLLRDNORM; } - packet_rcv_try_clear_pressure(po); + __packet_rcv_try_clear_pressure(po); spin_unlock_bh(&sk->sk_receive_queue.lock); spin_lock_bh(&sk->sk_write_queue.lock); if (po->tx_ring.pg_vec) {@@ -4544,14 +4556,15 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u, rb->frame_max = (req->tp_frame_nr - 1); rb->head = 0; rb->frame_size = req->tp_frame_size; + if (!tx_ring) + po->prot_hook.func = po->rx_ring.pg_vec ? + tpacket_rcv : packet_rcv;
Same question: why this new condition on !tx_ring here, that is missing below. It looks benign to me, but especially for fixes only make necessary changes.
spin_unlock_bh(&rb_queue->lock);
swap(rb->pg_vec_order, order);
swap(rb->pg_vec_len, req->tp_block_nr);
rb->pg_vec_pages = req->tp_block_size/PAGE_SIZE;
- po->prot_hook.func = (po->rx_ring.pg_vec) ?
- tpacket_rcv : packet_rcv;
skb_queue_purge(rb_queue);
if (atomic_long_read(&po->mapped))
pr_err("packet_mmap: vma is busy: %ld\n",
--
2.43.0