[net] bridge: br_handle_local_finish should not return zero

br_handle_local_finish is called by NF_HOOK(...) after accepting the packet. If
the return value of NF_HOOK(...) is zero (i.e. the return value of
br_handle_local_finish), the packet is passed to the network stack. This
behavior conflicts with netfilter hooks which return NF_STOLEN/NF_QUEUE. In this
case, NF_HOOK(...) returns also zero (see nf_hook_slow) but
br_handle_local_finish was never called. The packet will still passed to the
network stack.

Signed-off-by: Martin Faecknitz

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Martin Faecknitz <faecknitz@hotsplots.de>
Date: Sat, 14 Dec 2013 21:52:08 +0100

> br_handle_local_finish is called by NF_HOOK(...) after accepting the packet. If
> the return value of NF_HOOK(...) is zero (i.e. the return value of
> br_handle_local_finish), the packet is passed to the network stack. This
> behavior conflicts with netfilter hooks which return NF_STOLEN/NF_QUEUE. In this
> case, NF_HOOK(...) returns also zero (see nf_hook_slow) but
> br_handle_local_finish was never called. The packet will still passed to the
> network stack.
> 
> Signed-off-by: Martin Faecknitz

This is not a proper Signed-off-by: tag, you have to put your name
as well as your email address, similar to how it appears in the
"From: " field of your outgoing emails.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, 14 Dec 2013 21:52:08 +0100
Martin Faecknitz <faecknitz@hotsplots.de> wrote:

> br_handle_local_finish is called by NF_HOOK(...) after accepting the packet. If
> the return value of NF_HOOK(...) is zero (i.e. the return value of
> br_handle_local_finish), the packet is passed to the network stack. This
> behavior conflicts with netfilter hooks which return NF_STOLEN/NF_QUEUE. In this
> case, NF_HOOK(...) returns also zero (see nf_hook_slow) but
> br_handle_local_finish was never called. The packet will still passed to the
> network stack.
> 
> Signed-off-by: Martin Faecknitz
> 
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -149,7 +149,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
>  	br_vlan_get_tag(skb, &vid);
>  	if (p->flags & BR_LEARNING)
>  		br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid);
> -	return 0;	 /* process further */
> +	return 1;	 /* process further */
>  }
> 
>  /*
> @@ -208,7 +208,7 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
> 
>  		/* Deliver packet to local host only */
>  		if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
> -			    NULL, br_handle_local_finish)) {
> +			    NULL, br_handle_local_finish) != 1) {
>  			return RX_HANDLER_CONSUMED; /* consumed by filter */
>  		} else {
>  			*pskb = skb;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

I think the input code was incorrectly assuming the model of that receive logic
should return NET_RX_SUCCESS(0) or NET_RX_DROP(1). (Note DECNET has same bug).

Rather than hardcode in 0 and 1. why not use NF_DROP, NF_ACCEPT instead?


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html