Message ID | 1416994537-1592-1-git-send-email-rupran@einserver.de |
---|---|
State | Not Applicable |
Delegated to: | Pablo Neira |
Headers | show |
Andreas Ruprecht <rupran@einserver.de> wrote: > Additionally it is necessary to provide stubs for the > nf_nat_redirect_ipv{4,6} functions in case the header is included but > the corresponding Kconfig feature is not enabled. Hmmm, not following. Can you elaborate? Under which circumstances do we have a call to nf_nat_redirect_ipv4() (i.e., linker error) but can safely do a noop operation instead of the requested nat redirect...? -- 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
Sure. When the file is compiled, i.e. CONFIG_NETFILTER_XT_TARGET_REDIRECT is selected, all headers will be included and all functions inside the file will be compiled, regardless of other Kconfig options. This means redirect_tg6 and redirect_tg4 will be compiled (which doesn't necessarily mean they will be _called_) but the linker needs to resolve nf_nat_redirect_ipv4() due to the compilation of the redirect_tg4() function. nf_nat_redirect_ip4() is defined in net/ipv4/netfilter/nf_nat_redirect_ipv4.c but this file is only included into the build when CONFIG_NF_NAT_REDIRECT_IPV4 is enabled. Now when a kernel config enables CONFIG_NETFILTER_XT_TARGET_REDIRECT but _not_ CONFIG_NF_NAT_REDIRECT_IPV4, the declaration of nf_nat_redirect_ipv4() from the header <net/netfilter/ipv4/nf_nat_redirect.h> will have no definition (i.e., no implementation), causing the linker to report an "undefined reference". Same logic goes for nf_nat_redirect_ipv6(). Hope this helps, Andreas On 26.11.2014 11:24, Florian Westphal wrote: > Andreas Ruprecht <rupran@einserver.de> wrote: >> Additionally it is necessary to provide stubs for the >> nf_nat_redirect_ipv{4,6} functions in case the header is included but >> the corresponding Kconfig feature is not enabled. > > Hmmm, not following. > > Can you elaborate? > > Under which circumstances do we have a call to nf_nat_redirect_ipv4() > (i.e., linker error) but can safely do a noop operation instead of the > requested nat redirect...? > -- 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
On Wed, Nov 26, 2014 at 11:33:19AM +0100, Andreas Ruprecht wrote: > Sure. > > When the file is compiled, i.e. CONFIG_NETFILTER_XT_TARGET_REDIRECT is > selected, all headers will be included and all functions inside the file > will be compiled, regardless of other Kconfig options. > > This means redirect_tg6 and redirect_tg4 will be compiled (which doesn't > necessarily mean they will be _called_) but the linker needs to resolve > nf_nat_redirect_ipv4() due to the compilation of the redirect_tg4() > function. > > nf_nat_redirect_ip4() is defined in > net/ipv4/netfilter/nf_nat_redirect_ipv4.c but this file is only included > into the build when CONFIG_NF_NAT_REDIRECT_IPV4 is enabled. > > Now when a kernel config enables CONFIG_NETFILTER_XT_TARGET_REDIRECT but > _not_ CONFIG_NF_NAT_REDIRECT_IPV4, the declaration of > nf_nat_redirect_ipv4() from the header > <net/netfilter/ipv4/nf_nat_redirect.h> will have no definition (i.e., no > implementation), causing the linker to report an "undefined reference". > > Same logic goes for nf_nat_redirect_ipv6(). I'd suggest alternatives to resolve this problem: 1) Split xt_REDIRECT into ipt_REDIRECT and ip6t_REDIRECT, so we restore the state of how this was back in 2012. The main motivation behind that change was to reduce memory consumption by combining both modules. In other modules, these combinations have been causing us problems specifically when IPv6 symbols are used and it's not that clean since IPv6 specific code remains there unused in the module even if CONFIG_IPV6=n. 2) Merge nf_nat_redirect_ipv4 and nf_nat_redirect_ipv6 into nf_nat_redirect, so we inconditionally build IPv6 redirect code, thus xt_REDIRECT always finds the IPv6 symbol that needs even if it doesn't use it. 3) Add #ifdef to xt_REDIRECT.c to make IPv6 specific code, this should be a simple and small patch, but it results in #ifdef pollution. Comments? Thanks. -- 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
Andreas Ruprecht <rupran@einserver.de> wrote: > When the file is compiled, i.e. CONFIG_NETFILTER_XT_TARGET_REDIRECT is > selected, all headers will be included and all functions inside the file > will be compiled, regardless of other Kconfig options. > > This means redirect_tg6 and redirect_tg4 will be compiled (which doesn't > necessarily mean they will be _called_) but the linker needs to resolve > nf_nat_redirect_ipv4() due to the compilation of the redirect_tg4() > function. So the problem is eg. CONFIG_NETFILTER_XT_TARGET_REDIRECT=y CONFIG_NF_NAT_IPV4=n which yields undefined reference to `nf_nat_redirect_ipv4' adding stub fixes this; we will still register a netfilter target for ipv4 redirect, but it cannot be called ever since that target is resticted to 'nat' table, which doesn't exist for ipv4 in the above config. To me it seem cleaner to put IS_ENABLED(NF_NAT_REDIRECT_IPV4) into xt_REDIRECT.c instead of building not-working-but-never-called ipv4 redirect. Pablo, if you disagree I am fine with the patch, although the stubs should get a 'static inline' prefix to not cause sym clash in case we ever gain another call site. -- 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
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > 2) Merge nf_nat_redirect_ipv4 and nf_nat_redirect_ipv6 into > nf_nat_redirect, so we inconditionally build IPv6 redirect code, thus > xt_REDIRECT always finds the IPv6 symbol that needs even if it doesn't > use it. Seems this is the best solution since it would also reduce the kconfig option symbol count. afaics there is nothing ipv6 specific in nf_nat_redirect_ipv6 so this should not cause any of the ususal IPV6=m dependency issues. -- 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
diff --git a/include/net/netfilter/ipv4/nf_nat_redirect.h b/include/net/netfilter/ipv4/nf_nat_redirect.h index 19e1df3a0a4d..56a506dd55cc 100644 --- a/include/net/netfilter/ipv4/nf_nat_redirect.h +++ b/include/net/netfilter/ipv4/nf_nat_redirect.h @@ -1,9 +1,23 @@ #ifndef _NF_NAT_REDIRECT_IPV4_H_ #define _NF_NAT_REDIRECT_IPV4_H_ +#include <linux/netfilter.h> + +#ifdef CONFIG_NF_NAT_REDIRECT_IPV4 unsigned int nf_nat_redirect_ipv4(struct sk_buff *skb, const struct nf_nat_ipv4_multi_range_compat *mr, unsigned int hooknum); +#else /* CONFIG_NF_NAT_REDIRECT_IPV4 */ + +unsigned int +nf_nat_redirect_ipv4(struct sk_buff *skb, + const struct nf_nat_ipv4_multi_range_compat *mr, + unsigned int hooknum) +{ + return NF_ACCEPT; +} +#endif /* CONFIG_NF_NAT_REDIRECT_IPV4 */ + #endif /* _NF_NAT_REDIRECT_IPV4_H_ */ diff --git a/include/net/netfilter/ipv6/nf_nat_redirect.h b/include/net/netfilter/ipv6/nf_nat_redirect.h index 1ebdffc461cc..4db9351120ec 100644 --- a/include/net/netfilter/ipv6/nf_nat_redirect.h +++ b/include/net/netfilter/ipv6/nf_nat_redirect.h @@ -1,8 +1,21 @@ #ifndef _NF_NAT_REDIRECT_IPV6_H_ #define _NF_NAT_REDIRECT_IPV6_H_ +#include <linux/netfilter.h> + +#ifdef CONFIG_NF_NAT_REDIRECT_IPV6 unsigned int nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range *range, unsigned int hooknum); +#else /* CONFIG_NF_NAT_REDIRECT_IPV6 */ + +unsigned int +nf_nat_redirect_ipv6(struct sk_buff *skb, const struct nf_nat_range *range, + unsigned int hooknum) +{ + return NF_ACCEPT; +} +#endif /* CONFIG_NF_NAT_REDIRECT_IPV6 */ + #endif /* _NF_NAT_REDIRECT_IPV6_H_ */ diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig index be8db270aa77..0972851cce03 100644 --- a/net/netfilter/Kconfig +++ b/net/netfilter/Kconfig @@ -844,7 +844,7 @@ config NETFILTER_XT_TARGET_RATEEST config NETFILTER_XT_TARGET_REDIRECT tristate "REDIRECT target support" - depends on NF_NAT + depends on NF_NAT_IPV4 || NF_NAT_IPV6 select NF_NAT_REDIRECT_IPV4 if NF_NAT_IPV4 select NF_NAT_REDIRECT_IPV6 if NF_NAT_IPV6 ---help---
In a configuration with CONFIG_NFT_NAT and CONFIG_NETFILTER_XT_TARGET_REDIRECT enabled, undefined references to nf_nat_redirect_ipv{4,6}() can occur, when the corresponding options CONFIG_NF_NAT_REDIRECT_IPV4 or CONFIG_NF_NAT_REDIRECT_IPV6 are not enabled. net/built-in.o: In function `redirect_tg4': xt_REDIRECT.c:(.text+0x6d001): undefined reference to `nf_nat_redirect_ipv4' net/built-in.o: In function `redirect_tg6': xt_REDIRECT.c:(.text+0x6d021): undefined reference to `nf_nat_redirect_ipv6' This is because the file xt_REDIRECT.c is compiled when CONFIG_NETFILTER_XT_TARGET_REDIRECT is enabled, which only depends on CONFIG_NF_NAT. This option is invisible and can only be selected by other Kconfig options. In this particular case, it is selected by CONFIG_NFT_NAT. This patch changes the dependency for CONFIG_NETFILTER_XT_TARGET_REDIRECT to only make it visible if at least one of {CONFIG_NF_NAT_REDIRECT_IPV4, CONFIG_NF_NAT_REDIRECT_IPV6} are enabled. Additionally it is necessary to provide stubs for the nf_nat_redirect_ipv{4,6} functions in case the header is included but the corresponding Kconfig feature is not enabled. Changes: v2: Correct capitalization for CONFIG_NF_NAT_REDIRECT_IPV4 in comment. Signed-off-by: Andreas Ruprecht <rupran@einserver.de> --- include/net/netfilter/ipv4/nf_nat_redirect.h | 14 ++++++++++++++ include/net/netfilter/ipv6/nf_nat_redirect.h | 13 +++++++++++++ net/netfilter/Kconfig | 2 +- 3 files changed, 28 insertions(+), 1 deletion(-)