2018-01-15 15:49:53

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH net-next 1/2] netfilter: nf_defrag: mark xt_table structures 'const' again

As a side-effect of adding the module option, we now get a section
mismatch warning:

WARNING: net/ipv4/netfilter/iptable_raw.o(.data+0x1c): Section mismatch in reference from the variable packet_raw to the function .init.text:iptable_raw_table_init()
The variable packet_raw references
the function __init iptable_raw_table_init()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

Apparently it's ok to link to a __net_init function from .rodata but not
from .data. We can address this by rearranging the logic so that the
structure is read-only again. Instead of writing to the .priority field
later, we have an extra copies of the structure with that flag. An added
advantage is that that we don't have writable function pointers with this
approach.

Fixes: 902d6a4c2a4f ("netfilter: nf_defrag: Skip defrag if NOTRACK is set")
Signed-off-by: Arnd Bergmann <[email protected]>
---
This might not be the best fix for the issue, please have a look if you
can come up with something nicer, or just apply this version.
---
net/ipv4/netfilter/iptable_raw.c | 24 +++++++++++++++++++-----
net/ipv6/netfilter/ip6table_raw.c | 24 +++++++++++++++++++-----
2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/netfilter/iptable_raw.c b/net/ipv4/netfilter/iptable_raw.c
index 29b64d3024e0..960625aabf04 100644
--- a/net/ipv4/netfilter/iptable_raw.c
+++ b/net/ipv4/netfilter/iptable_raw.c
@@ -17,7 +17,7 @@ static bool raw_before_defrag __read_mostly;
MODULE_PARM_DESC(raw_before_defrag, "Enable raw table before defrag");
module_param(raw_before_defrag, bool, 0000);

-static struct xt_table packet_raw = {
+static const struct xt_table packet_raw = {
.name = "raw",
.valid_hooks = RAW_VALID_HOOKS,
.me = THIS_MODULE,
@@ -26,6 +26,15 @@ static struct xt_table packet_raw = {
.table_init = iptable_raw_table_init,
};

+static const struct xt_table packet_raw_before_defrag = {
+ .name = "raw",
+ .valid_hooks = RAW_VALID_HOOKS,
+ .me = THIS_MODULE,
+ .af = NFPROTO_IPV4,
+ .priority = NF_IP_PRI_RAW_BEFORE_DEFRAG,
+ .table_init = iptable_raw_table_init,
+};
+
/* The work comes in here from netfilter.c. */
static unsigned int
iptable_raw_hook(void *priv, struct sk_buff *skb,
@@ -39,15 +48,19 @@ static struct nf_hook_ops *rawtable_ops __read_mostly;
static int __net_init iptable_raw_table_init(struct net *net)
{
struct ipt_replace *repl;
+ const struct xt_table *table = &packet_raw;
int ret;

+ if (raw_before_defrag)
+ table = &packet_raw_before_defrag;
+
if (net->ipv4.iptable_raw)
return 0;

- repl = ipt_alloc_initial_table(&packet_raw);
+ repl = ipt_alloc_initial_table(table);
if (repl == NULL)
return -ENOMEM;
- ret = ipt_register_table(net, &packet_raw, repl, rawtable_ops,
+ ret = ipt_register_table(net, table, repl, rawtable_ops,
&net->ipv4.iptable_raw);
kfree(repl);
return ret;
@@ -68,14 +81,15 @@ static struct pernet_operations iptable_raw_net_ops = {
static int __init iptable_raw_init(void)
{
int ret;
+ const struct xt_table *table = &packet_raw;

if (raw_before_defrag) {
- packet_raw.priority = NF_IP_PRI_RAW_BEFORE_DEFRAG;
+ table = &packet_raw_before_defrag;

pr_info("Enabling raw table before defrag\n");
}

- rawtable_ops = xt_hook_ops_alloc(&packet_raw, iptable_raw_hook);
+ rawtable_ops = xt_hook_ops_alloc(table, iptable_raw_hook);
if (IS_ERR(rawtable_ops))
return PTR_ERR(rawtable_ops);

diff --git a/net/ipv6/netfilter/ip6table_raw.c b/net/ipv6/netfilter/ip6table_raw.c
index 3df7383f96d0..710fa0806c37 100644
--- a/net/ipv6/netfilter/ip6table_raw.c
+++ b/net/ipv6/netfilter/ip6table_raw.c
@@ -16,7 +16,7 @@ static bool raw_before_defrag __read_mostly;
MODULE_PARM_DESC(raw_before_defrag, "Enable raw table before defrag");
module_param(raw_before_defrag, bool, 0000);

-static struct xt_table packet_raw = {
+static const struct xt_table packet_raw = {
.name = "raw",
.valid_hooks = RAW_VALID_HOOKS,
.me = THIS_MODULE,
@@ -25,6 +25,15 @@ static struct xt_table packet_raw = {
.table_init = ip6table_raw_table_init,
};

+static const struct xt_table packet_raw_before_defrag = {
+ .name = "raw",
+ .valid_hooks = RAW_VALID_HOOKS,
+ .me = THIS_MODULE,
+ .af = NFPROTO_IPV6,
+ .priority = NF_IP6_PRI_RAW_BEFORE_DEFRAG,
+ .table_init = ip6table_raw_table_init,
+};
+
/* The work comes in here from netfilter.c. */
static unsigned int
ip6table_raw_hook(void *priv, struct sk_buff *skb,
@@ -38,15 +47,19 @@ static struct nf_hook_ops *rawtable_ops __read_mostly;
static int __net_init ip6table_raw_table_init(struct net *net)
{
struct ip6t_replace *repl;
+ const struct xt_table *table = &packet_raw;
int ret;

+ if (raw_before_defrag)
+ table = &packet_raw_before_defrag;
+
if (net->ipv6.ip6table_raw)
return 0;

- repl = ip6t_alloc_initial_table(&packet_raw);
+ repl = ip6t_alloc_initial_table(table);
if (repl == NULL)
return -ENOMEM;
- ret = ip6t_register_table(net, &packet_raw, repl, rawtable_ops,
+ ret = ip6t_register_table(net, table, repl, rawtable_ops,
&net->ipv6.ip6table_raw);
kfree(repl);
return ret;
@@ -67,15 +80,16 @@ static struct pernet_operations ip6table_raw_net_ops = {
static int __init ip6table_raw_init(void)
{
int ret;
+ const struct xt_table *table = &packet_raw;

if (raw_before_defrag) {
- packet_raw.priority = NF_IP6_PRI_RAW_BEFORE_DEFRAG;
+ table = &packet_raw_before_defrag;

pr_info("Enabling raw table before defrag\n");
}

/* Register hooks */
- rawtable_ops = xt_hook_ops_alloc(&packet_raw, ip6table_raw_hook);
+ rawtable_ops = xt_hook_ops_alloc(table, ip6table_raw_hook);
if (IS_ERR(rawtable_ops))
return PTR_ERR(rawtable_ops);

--
2.9.0


2018-01-15 15:50:04

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH net-next 2/2] netfilter: nf_defrag: move NF_CONNTRACK bits into #ifdef

We cannot access the skb->_nfct field when CONFIG_NF_CONNTRACK is
disabled:

net/ipv4/netfilter/nf_defrag_ipv4.c: In function 'ipv4_conntrack_defrag':
net/ipv4/netfilter/nf_defrag_ipv4.c:83:9: error: 'struct sk_buff' has no member named '_nfct'
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c: In function 'ipv6_defrag':
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68:9: error: 'struct sk_buff' has no member named '_nfct'

Both functions already have an #ifdef for this, so let's move the
check in there.

Fixes: 902d6a4c2a4f ("netfilter: nf_defrag: Skip defrag if NOTRACK is set")
Signed-off-by: Arnd Bergmann <[email protected]>
---
Please double-check what the right behavior for !CONFIG_NF_CONNTRACK
should be, I was only guessing here.
---
net/ipv4/netfilter/nf_defrag_ipv4.c | 4 +++-
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/nf_defrag_ipv4.c b/net/ipv4/netfilter/nf_defrag_ipv4.c
index cbd987f6b1f8..a0d3ad60a411 100644
--- a/net/ipv4/netfilter/nf_defrag_ipv4.c
+++ b/net/ipv4/netfilter/nf_defrag_ipv4.c
@@ -78,9 +78,11 @@ static unsigned int ipv4_conntrack_defrag(void *priv,
if (skb_nfct(skb) && !nf_ct_is_template((struct nf_conn *)skb_nfct(skb)))
return NF_ACCEPT;
#endif
+ if (skb->_nfct == IP_CT_UNTRACKED)
+ return NF_ACCEPT;
#endif
/* Gather fragments. */
- if (skb->_nfct != IP_CT_UNTRACKED && ip_is_fragment(ip_hdr(skb))) {
+ if (ip_is_fragment(ip_hdr(skb))) {
enum ip_defrag_users user =
nf_ct_defrag_user(state->hook, skb);

diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
index 87b503a8f5ef..c87b48359e8f 100644
--- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
+++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
@@ -63,10 +63,10 @@ static unsigned int ipv6_defrag(void *priv,
/* Previously seen (loopback)? */
if (skb_nfct(skb) && !nf_ct_is_template((struct nf_conn *)skb_nfct(skb)))
return NF_ACCEPT;
-#endif

if (skb->_nfct == IP_CT_UNTRACKED)
return NF_ACCEPT;
+#endif

err = nf_ct_frag6_gather(state->net, skb,
nf_ct6_defrag_user(state->hook, skb));
--
2.9.0

2018-01-16 00:48:14

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH net-next 1/2] netfilter: nf_defrag: mark xt_table structures 'const' again

On Mon, Jan 15, 2018 at 04:49:05PM +0100, Arnd Bergmann wrote:
> As a side-effect of adding the module option, we now get a section
> mismatch warning:
>
> WARNING: net/ipv4/netfilter/iptable_raw.o(.data+0x1c): Section mismatch in reference from the variable packet_raw to the function .init.text:iptable_raw_table_init()
> The variable packet_raw references
> the function __init iptable_raw_table_init()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

Applied, thanks Arnd.

2018-01-16 00:48:25

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH net-next 2/2] netfilter: nf_defrag: move NF_CONNTRACK bits into #ifdef

On Mon, Jan 15, 2018 at 04:49:06PM +0100, Arnd Bergmann wrote:
> We cannot access the skb->_nfct field when CONFIG_NF_CONNTRACK is
> disabled:
>
> net/ipv4/netfilter/nf_defrag_ipv4.c: In function 'ipv4_conntrack_defrag':
> net/ipv4/netfilter/nf_defrag_ipv4.c:83:9: error: 'struct sk_buff' has no member named '_nfct'
> net/ipv6/netfilter/nf_defrag_ipv6_hooks.c: In function 'ipv6_defrag':
> net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68:9: error: 'struct sk_buff' has no member named '_nfct'
>
> Both functions already have an #ifdef for this, so let's move the
> check in there.

Also applied, thanks.