Stephen Hemminger wrote:
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index a260679..8a55276 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> if (unlikely(is_link_local(dest))) {
> skb->pkt_type = PACKET_HOST;
> - return NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
> - NULL, br_handle_local_finish) != 0;
> +
> + return (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
> + NULL, br_handle_local_finish) == 0) ? skb :
> NULL;
> }
I Just want to note, this is broken in multiple ways (not by this
patch, it was already broken before). When a packet is stolen or
queued, NF_HOOK will return 0, but the packet is not owned by the
caller anymore, so we have a potential use-after-free. Additionally
the okfn owns the skb and needs to make sure it continues its path,
which br_handle_local_finish doesn't do, resulting in leaks and
broken queueing. The fix looks quite ugly, bf_handle_local_finish
would need to pass the skb back to netif_receive_skb just after the
handle_bridge call.
All this is not a problem for mainline currently since ebtables
doesn't support QUEUE yet, but since its mentioned on the TODO
list and in any case is incorrect use of NF_HOOK it feels worth
mentioning.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
