netdev - Re: [GIT PULL] bridge update for 2.6.22

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 11 Apr 2007 07:19:46 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Bart De Schuymer <bdschuym@...dora.be>
CC:	Stephen Hemminger <shemminger@...ux-foundation.org>,
	netdev@...r.kernel.org
Subject: Re: [GIT PULL] bridge update for 2.6.22

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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html