Messages in this thread |  | | Date | Mon, 8 Jan 2024 12:15:04 +0100 | | From | Florian Westphal <> | | Subject | Re: [PATCH] neighbour: purge nf_bridged skb from foreign device neigh |
| |
Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote: > An skb can be added to a neigh->arp_queue while waiting for an arp > reply. Where original skb's skb->dev can be different to neigh's > neigh->dev. For instance in case of bridging dnated skb from one veth to > another, the skb would be added to a neigh->arp_queue of the bridge. > > There is no explicit mechanism that prevents the original skb->dev link > of such skb from being freed under us. For instance neigh_flush_dev does > not cleanup skbs from different device's neigh queue. But that original > link can be used and lead to crash on e.g. this stack: > > arp_process > neigh_update > skb = __skb_dequeue(&neigh->arp_queue) > neigh_resolve_output(..., skb) > ... > br_nf_dev_xmit > br_nf_pre_routing_finish_bridge_slow > skb->dev = nf_bridge->physindev > br_handle_frame_finish > > So let's improve neigh_flush_dev to also purge skbs when device > equal to their skb->nf_bridge->physindev gets destroyed.
Can we fix this by replacing physindev pointer with plain ifindex instead? There are not too many places that need to peek into the original net_device struct, so I don't think the additional dev_get_by_index_rcu() would be an issue.
|  |