[PATCH 1/1] netfilter: ebtables: make broute table work again

[PATCH 1/1] netfilter: ebtables: make broute table work again

[Florian Westphal]

broute table init hook sets up the "br_should_route_hook" pointer,
which then gets called from br_input.

commit a386f99025f13b32502fe5dedf223c20d7283826
(bridge: add proper RCU annotation to should_route_hook)
introduced a typedef, and then changed this to:

br_should_route_hook_t *rhook;
[..]
rhook = rcu_dereference(br_should_route_hook);
if (*rhook(skb))

problem is that "br_should_route_hook" contains the address of the function,
so calling *rhook() results in kernel panic.

Can't simply use "if (rhook(skb)", because br_should_route_hook_t *rhook
is not a function.

Signed-off-by: Florian Westphal <fw@strlen.de>
---

 include/linux/if_bridge.h |    2 +-
 net/bridge/br_input.c     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index f7e73c3..dd3f201 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -103,7 +103,7 @@ struct __fdb_entry {
 
 extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __user *));
 
-typedef int (*br_should_route_hook_t)(struct sk_buff *skb);
+typedef int br_should_route_hook_t(struct sk_buff *skb);
 extern br_should_route_hook_t __rcu *br_should_route_hook;
 
 #endif
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 6f6d8e1..1d677bf 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -178,7 +178,7 @@ forward:
 	case BR_STATE_FORWARDING:
 		rhook = rcu_dereference(br_should_route_hook);
 		if (rhook) {
-			if ((*rhook)(skb))
+			if (rhook(skb))
 				return skb;
 			dest = eth_hdr(skb)->h_dest;
 		}
-- 
1.7.2.2

Re: [PATCH 1/1] netfilter: ebtables: make broute table work again

[Changli Gao]

On Fri, Dec 24, 2010 at 8:27 AM, Florian Westphal <fw@strlen.de> wrote:
> broute table init hook sets up the "br_should_route_hook" pointer,
> which then gets called from br_input.
>
> commit a386f99025f13b32502fe5dedf223c20d7283826
> (bridge: add proper RCU annotation to should_route_hook)
> introduced a typedef, and then changed this to:
>
> br_should_route_hook_t *rhook;
> [..]
> rhook = rcu_dereference(br_should_route_hook);
> if (*rhook(skb))
>
> problem is that "br_should_route_hook" contains the address of the function,
> so calling *rhook() results in kernel panic.
>
> Can't simply use "if (rhook(skb)", because br_should_route_hook_t *rhook
> is not a function.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>
>  include/linux/if_bridge.h |    2 +-
>  net/bridge/br_input.c     |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index f7e73c3..dd3f201 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -103,7 +103,7 @@ struct __fdb_entry {
>
>  extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __user *));
>
> -typedef int (*br_should_route_hook_t)(struct sk_buff *skb);
> +typedef int br_should_route_hook_t(struct sk_buff *skb);
>  extern br_should_route_hook_t __rcu *br_should_route_hook;
>
>  #endif
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 6f6d8e1..1d677bf 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -178,7 +178,7 @@ forward:
>        case BR_STATE_FORWARDING:
>                rhook = rcu_dereference(br_should_route_hook);
>                if (rhook) {
> -                       if ((*rhook)(skb))
> +                       if (rhook(skb))

I don't think this change is necessary. Would you like to remove this
change and test again?

>                                return skb;
>                        dest = eth_hdr(skb)->h_dest;
>                }




-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

Re: [PATCH 1/1] netfilter: ebtables: make broute table work again

[Florian Westphal]

Changli Gao <xiaosuo@gmail.com> wrote:
> > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> > index 6f6d8e1..1d677bf 100644
> > --- a/net/bridge/br_input.c
> > +++ b/net/bridge/br_input.c
> > @@ -178,7 +178,7 @@ forward:
> >        case BR_STATE_FORWARDING:
> >                rhook = rcu_dereference(br_should_route_hook);
> >                if (rhook) {
> > -                       if ((*rhook)(skb))
> > +                       if (rhook(skb))
> 
> I don't think this change is necessary. Would you like to remove this
> change and test again?

Its not needed, but i consider it less confusing.
I do not have a strong preference, though.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html