Re: [PATCH] netfilter: br_netfilter: reject unsafe neighbour devices for DNAT

[Florian Westphal]

Kyle Zeng <kylebot@xxxxxxxxxx> wrote:
> The IPv4 and IPv6 bridge prerouting callbacks classify DNAT traffic using
> the logical input device saved in skb->dev. The physical input port can
> have a different bridge master when prerouting resumes, for example after
> an NFQUEUE verdict. Netfilter reassembly can also combine the first
> fragment's bridge information with a different fragment's input device.
> 
> br_nf_pre_routing_finish_bridge() replaces skb->dev with the port's
> current bridge master, but looks up the neighbour using the existing
> route. This can put an skb with skb->dev pointing to bridge B on an
> unresolved neighbour of bridge A. Deleting B and resolving the neighbour
> on A then causes a use-after-free in __dev_queue_xmit().
> 
> Before calling the neighbour output method, check that the neighbour
> keeps skb->dev alive. A neighbour holds its device, and a VLAN device
> also holds its real device, so allow a VLAN whose real device is the
> bridge. Release the neighbour and drop the skb on a mismatch.
> 
> Doing this in the shared continuation covers both protocol families,
> including a queue in the remaining bridge prerouting hooks. Leave the
> cached-header path unchanged since it does not enqueue the skb.
> 
> Signed-off-by: Kyle Zeng <kylebot@xxxxxxxxxx>
> ---
> I have a reproducer for this issue and can share via DM

Please do.  I'd like to understand if that needs to be addressed
from nfqueue side instead, or if we can avoid neigh layer to
begin with.