* Re: [PATCH net 1/1] packet: synchronize pressure clearing with ring reconfiguration
2026-07-20 5:16 ` [PATCH net 1/1] packet: synchronize pressure clearing with ring reconfiguration Ren Wei
@ 2026-07-20 11:31 ` Willem de Bruijn
0 siblings, 0 replies; 2+ messages in thread
From: Willem de Bruijn @ 2026-07-20 11:31 UTC (permalink / raw)
To: Ren Wei, netdev
Cc: willemdebruijn.kernel, davem, edumazet, pabeni, horms, vega,
xizh2024, enjou1224z
Ren Wei wrote:
> From: Zihan Xi <xizh2024@lzu.edu.cn>
>
> 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.
> This keeps the receive hook decision consistent with the ring state used
> by the tpacket room checks.
Thanks for the report and proposed fix.
Ideally we can avoid taking sk_receive_queue.lock an extra time in
packet_recvmsg.
packet_rcv_try_clear_pressure only accesses the ring if flag
PACKET_SOCK_PRESSURE is set. One option may be to clear that in
packet_set_ring, after detaching the socket (and thus after any input
could set it again) and before swapping prot_hook.func (with a barrier
to guarantee that). E.g.,:
@@ -4528,6 +4528,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
WRITE_ONCE(po->num, 0);
if (was_running)
__unregister_prot_hook(sk, false);
+ packet_sock_flag_set(po, PACKET_SOCK_PRESSURE, false);
spin_unlock(&po->bind_lock);
> Fixes: 2ccdbaa6d55b ("packet: rollover lock contention avoidance")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zihan Xi <xizh2024@lzu.edu.cn>
> Reviewed-by: Ren Wei <enjou1224z@gmail.com>
> ---
> net/packet/af_packet.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 8e6f3a734ba0..b369f44b4065 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1315,13 +1315,22 @@ 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;
> +
> + 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 +4313,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 +4553,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;
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread