* [PATCH net] bridge: br_handle_local_finish should not return zero
@ 2013-12-14 20:52 Martin Faecknitz
2013-12-17 21:13 ` David Miller
2013-12-17 21:55 ` Stephen Hemminger
0 siblings, 2 replies; 3+ messages in thread
From: Martin Faecknitz @ 2013-12-14 20:52 UTC (permalink / raw)
To: netdev
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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] bridge: br_handle_local_finish should not return zero
2013-12-14 20:52 [PATCH net] bridge: br_handle_local_finish should not return zero Martin Faecknitz
@ 2013-12-17 21:13 ` David Miller
2013-12-17 21:55 ` Stephen Hemminger
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2013-12-17 21:13 UTC (permalink / raw)
To: faecknitz; +Cc: netdev
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] bridge: br_handle_local_finish should not return zero
2013-12-14 20:52 [PATCH net] bridge: br_handle_local_finish should not return zero Martin Faecknitz
2013-12-17 21:13 ` David Miller
@ 2013-12-17 21:55 ` Stephen Hemminger
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2013-12-17 21:55 UTC (permalink / raw)
To: Martin Faecknitz; +Cc: netdev
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?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-12-17 21:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-14 20:52 [PATCH net] bridge: br_handle_local_finish should not return zero Martin Faecknitz
2013-12-17 21:13 ` David Miller
2013-12-17 21:55 ` Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).