Re: [PATCH nf] netfilter: xt_hashlimit: fix a possible memory leak in htable_create()
On Fri, Nov 16, 2018 at 09:32:35PM +0900, Taehee Yoo wrote: > In the htable_create(), hinfo is allocated by vmalloc() > So that if error occurred, hinfo should be freed. Applied, thanks Taehee.
[PATCH iptables 4/4] xtables: constify struct builtin_table and struct builtin_chain
These definitions should be const, propagate this to all existing users.
Signed-off-by: Pablo Neira Ayuso
---
iptables/nft.c | 42 +-
iptables/nft.h | 14 +++---
iptables/xtables-restore.c | 4 ++--
iptables/xtables-save.c | 2 +-
iptables/xtables-translate.c | 2 +-
5 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/iptables/nft.c b/iptables/nft.c
index 618171e3208a..0223c0ed1000 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -376,7 +376,7 @@ static int batch_rule_add(struct nft_handle *h, enum
obj_update_type type,
return batch_add(h, type, r);
}
-struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
+const struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
[NFT_TABLE_RAW] = {
.name = "raw",
.type = NFT_TABLE_RAW,
@@ -513,7 +513,7 @@ struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
#include
-struct builtin_table xtables_arp[NFT_TABLE_MAX] = {
+const struct builtin_table xtables_arp[NFT_TABLE_MAX] = {
[NFT_TABLE_FILTER] = {
.name = "filter",
.type = NFT_TABLE_FILTER,
@@ -536,7 +536,7 @@ struct builtin_table xtables_arp[NFT_TABLE_MAX] = {
#include
-struct builtin_table xtables_bridge[NFT_TABLE_MAX] = {
+const struct builtin_table xtables_bridge[NFT_TABLE_MAX] = {
[NFT_TABLE_FILTER] = {
.name = "filter",
.type = NFT_TABLE_FILTER,
@@ -594,7 +594,7 @@ static bool nft_table_initialized(const struct nft_handle
*h,
}
static int nft_table_builtin_add(struct nft_handle *h,
-struct builtin_table *_t)
+const struct builtin_table *_t)
{
struct nftnl_table *t;
int ret;
@@ -614,8 +614,8 @@ static int nft_table_builtin_add(struct nft_handle *h,
}
static struct nftnl_chain *
-nft_chain_builtin_alloc(struct builtin_table *table,
- struct builtin_chain *chain, int policy)
+nft_chain_builtin_alloc(const struct builtin_table *table,
+ const struct builtin_chain *chain, int policy)
{
struct nftnl_chain *c;
@@ -634,8 +634,8 @@ nft_chain_builtin_alloc(struct builtin_table *table,
}
static void nft_chain_builtin_add(struct nft_handle *h,
- struct builtin_table *table,
- struct builtin_chain *chain)
+ const struct builtin_table *table,
+ const struct builtin_chain *chain)
{
struct nftnl_chain *c;
@@ -647,7 +647,7 @@ static void nft_chain_builtin_add(struct nft_handle *h,
}
/* find if built-in table already exists */
-struct builtin_table *
+const struct builtin_table *
nft_table_builtin_find(struct nft_handle *h, const char *table)
{
int i;
@@ -668,8 +668,8 @@ nft_table_builtin_find(struct nft_handle *h, const char
*table)
}
/* find if built-in chain already exists */
-struct builtin_chain *
-nft_chain_builtin_find(struct builtin_table *t, const char *chain)
+const struct builtin_chain *
+nft_chain_builtin_find(const struct builtin_table *t, const char *chain)
{
int i;
bool found = false;
@@ -685,7 +685,7 @@ nft_chain_builtin_find(struct builtin_table *t, const char
*chain)
}
static void nft_chain_builtin_init(struct nft_handle *h,
- struct builtin_table *table)
+ const struct builtin_table *table)
{
struct nftnl_chain_list *list = nft_chain_list_get(h, table->name);
struct nftnl_chain *c;
@@ -707,7 +707,7 @@ static void nft_chain_builtin_init(struct nft_handle *h,
static int nft_xt_builtin_init(struct nft_handle *h, const char *table)
{
- struct builtin_table *t;
+ const struct builtin_table *t;
t = nft_table_builtin_find(h, table);
if (t == NULL)
@@ -750,7 +750,7 @@ static int nft_restart(struct nft_handle *h)
return 0;
}
-int nft_init(struct nft_handle *h, struct builtin_table *t)
+int nft_init(struct nft_handle *h, const struct builtin_table *t)
{
h->nl = mnl_socket_open(NETLINK_NETFILTER);
if (h->nl == NULL)
@@ -852,8 +852,8 @@ static struct nftnl_chain *nft_chain_new(struct nft_handle
*h,
const struct xt_counters *counters)
{
struct nftnl_chain *c;
- struct builtin_table *_t;
- struct builtin_chain *_c;
+ const struct builtin_table *_t;
+ const struct builtin_chain *_c;
_t = nft_table_builtin_find(h, table);
if (!_t) {
@@ -1294,7 +1294,7 @@ nft_rule_print_save(const struct nftnl_rule *r, enum
nft_rule_print type,
static int nftnl_chain_list_cb(const struct nlmsghdr *nlh, void *data)
{
struct nft_handle *h = data;
- struct builtin_table *t;
+ const struct builtin_table *t;
struct nftnl_chain *c
[PATCH iptables 1/4] nft: add type field to builtin_table
Use enum nft_table_type to set the new type field in the structure that
define tables.
---
iptables/nft.c | 8
iptables/nft.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/iptables/nft.c b/iptables/nft.c
index 5e55ec13d0da..db86f97c6d29 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -379,6 +379,7 @@ static int batch_rule_add(struct nft_handle *h, enum
obj_update_type type,
struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
[NFT_TABLE_RAW] = {
.name = "raw",
+ .type = NFT_TABLE_RAW,
.chains = {
{
.name = "PREROUTING",
@@ -396,6 +397,7 @@ struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
},
[NFT_TABLE_MANGLE] = {
.name = "mangle",
+ .type = NFT_TABLE_MANGLE,
.chains = {
{
.name = "PREROUTING",
@@ -431,6 +433,7 @@ struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
},
[NFT_TABLE_FILTER] = {
.name = "filter",
+ .type = NFT_TABLE_FILTER,
.chains = {
{
.name = "INPUT",
@@ -454,6 +457,7 @@ struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
},
[NFT_TABLE_SECURITY] = {
.name = "security",
+ .type = NFT_TABLE_SECURITY,
.chains = {
{
.name = "INPUT",
@@ -477,6 +481,7 @@ struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
},
[NFT_TABLE_NAT] = {
.name = "nat",
+ .type = NFT_TABLE_NAT,
.chains = {
{
.name = "PREROUTING",
@@ -511,6 +516,7 @@ struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
struct builtin_table xtables_arp[NFT_TABLE_MAX] = {
[NFT_TABLE_FILTER] = {
.name = "filter",
+ .type = NFT_TABLE_FILTER,
.chains = {
{
.name = "INPUT",
@@ -533,6 +539,7 @@ struct builtin_table xtables_arp[NFT_TABLE_MAX] = {
struct builtin_table xtables_bridge[NFT_TABLE_MAX] = {
[NFT_TABLE_FILTER] = {
.name = "filter",
+ .type = NFT_TABLE_FILTER,
.chains = {
{
.name = "INPUT",
@@ -556,6 +563,7 @@ struct builtin_table xtables_bridge[NFT_TABLE_MAX] = {
},
[NFT_TABLE_NAT] = {
.name = "nat",
+ .type = NFT_TABLE_NAT,
.chains = {
{
.name = "PREROUTING",
diff --git a/iptables/nft.h b/iptables/nft.h
index 980b38dcce1e..e582a6afcc8f 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -23,6 +23,7 @@ struct builtin_chain {
struct builtin_table {
const char *name;
+ enum nft_table_type type;
struct builtin_chain chains[NF_INET_NUMHOOKS];
bool initialized;
struct nftnl_chain_list *chain_cache;
--
2.11.0
[PATCH iptables 2/4] nft: move chain_cache back to struct nft_handle
Place this back into the structure that stores the state information.
Signed-off-by: Pablo Neira Ayuso
---
iptables/nft.c | 26 +-
iptables/nft.h | 4 +++-
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/iptables/nft.c b/iptables/nft.c
index db86f97c6d29..6852def381dd 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -809,14 +809,14 @@ static void flush_chain_cache(struct nft_handle *h, const
char *tablename)
if (tablename && strcmp(h->tables[i].name, tablename))
continue;
- if (h->tables[i].chain_cache) {
+ if (h->table[i].chain_cache) {
if (tablename) {
-
nftnl_chain_list_foreach(h->tables[i].chain_cache,
+
nftnl_chain_list_foreach(h->table[i].chain_cache,
__flush_chain_cache,
NULL);
break;
} else {
- nftnl_chain_list_free(h->tables[i].chain_cache);
- h->tables[i].chain_cache = NULL;
+ nftnl_chain_list_free(h->table[i].chain_cache);
+ h->table[i].chain_cache = NULL;
}
}
}
@@ -1303,13 +1303,13 @@ static int nftnl_chain_list_cb(const struct nlmsghdr
*nlh, void *data)
if (!t)
goto out;
- if (!t->chain_cache) {
- t->chain_cache = nftnl_chain_list_alloc();
- if (!t->chain_cache)
+ if (!h->table[t->type].chain_cache) {
+ h->table[t->type].chain_cache = nftnl_chain_list_alloc();
+ if (!h->table[t->type].chain_cache)
goto out;
}
- nftnl_chain_list_add_tail(c, t->chain_cache);
+ nftnl_chain_list_add_tail(c, h->table[t->type].chain_cache);
return MNL_CB_OK;
out:
@@ -1330,8 +1330,8 @@ struct nftnl_chain_list *nft_chain_list_get(struct
nft_handle *h,
if (!t)
return NULL;
- if (t->chain_cache)
- return t->chain_cache;
+ if (h->table[t->type].chain_cache)
+ return h->table[t->type].chain_cache;
retry:
nlh = nftnl_chain_nlmsg_build_hdr(buf, NFT_MSG_GETCHAIN, h->family,
NLM_F_DUMP, h->seq);
@@ -1342,10 +1342,10 @@ retry:
goto retry;
}
- if (!t->chain_cache)
- t->chain_cache = nftnl_chain_list_alloc();
+ if (!h->table[t->type].chain_cache)
+ h->table[t->type].chain_cache = nftnl_chain_list_alloc();
- return t->chain_cache;
+ return h->table[t->type].chain_cache;
}
static const char *policy_name[NF_ACCEPT+1] = {
diff --git a/iptables/nft.h b/iptables/nft.h
index e582a6afcc8f..8cacae7394a3 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -26,7 +26,6 @@ struct builtin_table {
enum nft_table_type type;
struct builtin_chain chains[NF_INET_NUMHOOKS];
bool initialized;
- struct nftnl_chain_list *chain_cache;
};
struct nft_handle {
@@ -40,6 +39,9 @@ struct nft_handle {
struct list_headerr_list;
struct nft_family_ops *ops;
struct builtin_table*tables;
+ struct {
+ struct nftnl_chain_list *chain_cache;
+ } table[NFT_TABLE_MAX];
struct nftnl_rule_list *rule_cache;
boolrestore;
int8_t config_done;
--
2.11.0
[PATCH iptables 3/4] nft: move initialize to struct nft_handle
Move this to the structure that stores, stateful information. Introduce
nft_table_initialized() and use it.
Signed-off-by: Pablo Neira Ayuso
---
iptables/nft.c | 14 ++
iptables/nft.h | 2 +-
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/iptables/nft.c b/iptables/nft.c
index 6852def381dd..618171e3208a 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -587,13 +587,19 @@ struct builtin_table xtables_bridge[NFT_TABLE_MAX] = {
},
};
+static bool nft_table_initialized(const struct nft_handle *h,
+ enum nft_table_type type)
+{
+ return h->table[type].initialized;
+}
+
static int nft_table_builtin_add(struct nft_handle *h,
struct builtin_table *_t)
{
struct nftnl_table *t;
int ret;
- if (_t->initialized)
+ if (nft_table_initialized(h, _t->type))
return 0;
t = nftnl_table_alloc();
@@ -707,7 +713,7 @@ static int nft_xt_builtin_init(struct nft_handle *h, const
char *table)
if (t == NULL)
return -1;
- if (t->initialized)
+ if (nft_table_initialized(h, t->type))
return 0;
if (nft_table_builtin_add(h, t) < 0)
@@ -715,7 +721,7 @@ static int nft_xt_builtin_init(struct nft_handle *h, const
char *table)
nft_chain_builtin_init(h, t);
- t->initialized = true;
+ h->table[t->type].initialized = true;
return 0;
}
@@ -1902,7 +1908,7 @@ static int __nft_table_flush(struct nft_handle *h, const
char *table)
_t = nft_table_builtin_find(h, table);
assert(_t);
- _t->initialized = false;
+ h->table[_t->type].initialized = false;
flush_chain_cache(h, table);
flush_rule_cache(h, table);
diff --git a/iptables/nft.h b/iptables/nft.h
index 8cacae7394a3..9fe83ad134da 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -25,7 +25,6 @@ struct builtin_table {
const char *name;
enum nft_table_type type;
struct builtin_chain chains[NF_INET_NUMHOOKS];
- bool initialized;
};
struct nft_handle {
@@ -41,6 +40,7 @@ struct nft_handle {
struct builtin_table*tables;
struct {
struct nftnl_chain_list *chain_cache;
+ boolinitialized;
} table[NFT_TABLE_MAX];
struct nftnl_rule_list *rule_cache;
boolrestore;
--
2.11.0
Re: [iptables PATCH v2] xtables: Introduce per table chain caches
On Thu, Nov 15, 2018 at 02:53:02PM +0100, Phil Sutter wrote: > Being able to omit the previously obligatory table name check when > iterating over the chain cache might help restore performance with large > rulesets in xtables-save and -restore. > > There is one subtle quirk in the code: flush_chain_cache() did free the > global chain cache if not called with a table name but didn't if a table > name was given even if it emptied the chain cache. In other places, > chain_cache being non-NULL prevented a cache update from happening, so > this patch establishes the same behaviour (for each individual chain > cache) since otherwise unexpected cache updates lead to weird problems. Applied, thanks Phil. I sent a few follow up patches on top of this. My idea is to constify the builtin_table and builtin_cache definitions, so we keep all ongoing internal states into the struct nft_handle. Thanks.
Re: [PATCH iptables] include: fix build with kernel headers before 4.2
Hi Baruch, On Fri, Nov 16, 2018 at 09:30:33AM +0200, Baruch Siach wrote: > Commit 672accf1530 (include: update kernel netfilter header files) > updated linux/netfilter.h and brought with it the update from kernel > commit a263653ed798 (netfilter: don't pull include/linux/netfilter.h > from netns headers). This triggers conflict of headers that is fixed in > kernel commit 279c6c7fa64f (api: fix compatibility of linux/in.h with > netinet/in.h) included in kernel version 4.2. For earlier kernel headers > we need a workaround that prevents the headers conflict. > > Fixes the following build failure: > > In file included from .../sysroot/usr/include/netinet/ip.h:25:0, > from ../include/libiptc/ipt_kernel_headers.h:8, > from ../include/libiptc/libiptc.h:6, > from libip4tc.c:29: > .../sysroot/usr/include/linux/in.h:26:3: error: redeclaration of enumerator > ‘IPPROTO_IP’ >IPPROTO_IP = 0, /* Dummy protocol for TCP */ >^ > .../sysroot/usr/include/netinet/in.h:33:5: note: previous definition of > ‘IPPROTO_IP’ was here > IPPROTO_IP = 0,/* Dummy protocol for TCP. */ > ^~ > > Cc: Florian Westphal > Signed-off-by: Baruch Siach > --- > include/linux/netfilter.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h > index c3f087ac680c..bacf8cd92116 100644 > --- a/include/linux/netfilter.h > +++ b/include/linux/netfilter.h > @@ -3,7 +3,9 @@ > > #include > > +#ifndef _NETINET_IN_H > #include > +#endif This is updating a cached copy of the kernel headers, we basically copy kernel headers and place in the userspace tree to make sure that iptables compiles standalone, without the need for kernel-headers to be installed in the system in order to simplify building process. I would like we don't have to modify this cached copy, so if you can find a way to update the userspace C files without touching the cached copy of the kernel header, that would be great. My concern is that this little tweak will go away once we update the cached copy anytime soon in the future. Thanks.
[PATCH iptables] xtables-monitor: fix build with musl libc
Commit 7c8791edac3 ("xtables-monitor: fix build with older glibc")
changed the code to use GNU style tcphdr fields. Unfortunately, musl
libc requires _GNU_SOURCE definition to expose these fields.
Fix the following build failure:
xtables-monitor.c: In function ‘trace_print_packet’:
xtables-monitor.c:406:43: error: ‘const struct tcphdr’ has no member named
‘source’
printf("SPORT=%d DPORT=%d ", ntohs(tcph->source), ntohs(tcph->dest));
^~
xtables-monitor.c:406:64: error: ‘const struct tcphdr’ has no member named
‘dest’
printf("SPORT=%d DPORT=%d ", ntohs(tcph->source), ntohs(tcph->dest));
^~
...
Cc: Florian Westphal
Signed-off-by: Baruch Siach
---
iptables/xtables-monitor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
index 5d1611122df5..f835c5e503e0 100644
--- a/iptables/xtables-monitor.c
+++ b/iptables/xtables-monitor.c
@@ -9,6 +9,7 @@
* This software has been sponsored by Sophos Astaro
*/
+#define _GNU_SOURCE
#include
#include
#include
--
2.19.1
Re: [PATCH iptables] xtables-monitor: fix build with musl libc
Baruch Siach wrote:
> Commit 7c8791edac3 ("xtables-monitor: fix build with older glibc")
> changed the code to use GNU style tcphdr fields. Unfortunately, musl
> libc requires _GNU_SOURCE definition to expose these fields.
>
> Fix the following build failure:
Applied, thanks.
Re: [PATCH iptables] include: fix build with kernel headers before 4.2
Hi Pablo,
Pablo Neira Ayuso writes:
> On Fri, Nov 16, 2018 at 09:30:33AM +0200, Baruch Siach wrote:
>> Commit 672accf1530 (include: update kernel netfilter header files)
>> updated linux/netfilter.h and brought with it the update from kernel
>> commit a263653ed798 (netfilter: don't pull include/linux/netfilter.h
>> from netns headers). This triggers conflict of headers that is fixed in
>> kernel commit 279c6c7fa64f (api: fix compatibility of linux/in.h with
>> netinet/in.h) included in kernel version 4.2. For earlier kernel headers
>> we need a workaround that prevents the headers conflict.
>>
>> Fixes the following build failure:
>>
>> In file included from .../sysroot/usr/include/netinet/ip.h:25:0,
>> from ../include/libiptc/ipt_kernel_headers.h:8,
>> from ../include/libiptc/libiptc.h:6,
>> from libip4tc.c:29:
>> .../sysroot/usr/include/linux/in.h:26:3: error: redeclaration of enumerator
>> ‘IPPROTO_IP’
>>IPPROTO_IP = 0, /* Dummy protocol for TCP */
>>^
>> .../sysroot/usr/include/netinet/in.h:33:5: note: previous definition of
>> ‘IPPROTO_IP’ was here
>> IPPROTO_IP = 0,/* Dummy protocol for TCP. */
>> ^~
>>
>> Cc: Florian Westphal
>> Signed-off-by: Baruch Siach
>> ---
>> include/linux/netfilter.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
>> index c3f087ac680c..bacf8cd92116 100644
>> --- a/include/linux/netfilter.h
>> +++ b/include/linux/netfilter.h
>> @@ -3,7 +3,9 @@
>>
>> #include
>>
>> +#ifndef _NETINET_IN_H
>> #include
>> +#endif
>
> This is updating a cached copy of the kernel headers, we basically
> copy kernel headers and place in the userspace tree to make sure that
> iptables compiles standalone, without the need for kernel-headers to
> be installed in the system in order to simplify building process.
>
> I would like we don't have to modify this cached copy, so if you can
> find a way to update the userspace C files without touching the cached
> copy of the kernel header, that would be great. My concern is that
> this little tweak will go away once we update the cached copy anytime
> soon in the future.
>
> Thanks.
I can't think of any better solution.
A possible alternative would be to add '#define _LINUX_IN_H' in every
file that include netinet/in.h to suppress the kernel headern. This is a
bigger change, although is doesn't touch any cached kernel header as far
as I can see.
Do you like this solution better?
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}ooO--U--Ooo{=
- bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
[PATCH nf] netfilter: nfnetlink_cttimeout: fetch timeouts for udplite and gre, too
syzbot was able to trigger the WARN in cttimeout_default_get() by
passing UDPLITE as l4protocol. Alias UDPLITE to UDP, both use
same timeout values.
Furthermore, also fetch GRE timeouts. GRE is a bit more complicated,
as it still can be a module and its netns_proto_gre struct layout isn't
visible outside of the gre module.
Work around this by forcing the timeouts to be the first structure
member, then use plain net_generic().
A followup nf-next patch could make gre tracker be built-in as well
if needed, its not that large.
Last, make the WARN() mention the missing protocol value in case
anything else is missing.
Reported-by: syzbot+2fae8fa157dd92618...@syzkaller.appspotmail.com
Fixes: 8866df9264a3 ("netfilter: nfnetlink_cttimeout: pass default timeout
policy to obj_to_nlattr")
Signed-off-by: Florian Westphal
---
net/netfilter/nf_conntrack_proto_gre.c | 4 +++-
net/netfilter/nfnetlink_cttimeout.c| 11 +--
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nf_conntrack_proto_gre.c
b/net/netfilter/nf_conntrack_proto_gre.c
index 9b48dc8b4b88..dd8db7fbc437 100644
--- a/net/netfilter/nf_conntrack_proto_gre.c
+++ b/net/netfilter/nf_conntrack_proto_gre.c
@@ -56,10 +56,10 @@ static const unsigned int gre_timeouts[GRE_CT_MAX] = {
static unsigned int proto_gre_net_id __read_mostly;
struct netns_proto_gre {
+ unsigned intgre_timeouts[GRE_CT_MAX];
struct nf_proto_net nf;
rwlock_tkeymap_lock;
struct list_headkeymap_list;
- unsigned intgre_timeouts[GRE_CT_MAX];
};
static inline struct netns_proto_gre *gre_pernet(struct net *net)
@@ -402,6 +402,8 @@ static int __init nf_ct_proto_gre_init(void)
{
int ret;
+ BUILD_BUG_ON(offsetof(struct netns_proto_gre, gre_timeouts));
+
ret = register_pernet_subsys(&proto_gre_net_ops);
if (ret < 0)
goto out_pernet;
diff --git a/net/netfilter/nfnetlink_cttimeout.c
b/net/netfilter/nfnetlink_cttimeout.c
index a518eb162344..1643faa35f56 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -455,7 +455,8 @@ static int cttimeout_default_get(struct net *net, struct
sock *ctnl,
case IPPROTO_TCP:
timeouts = nf_tcp_pernet(net)->timeouts;
break;
- case IPPROTO_UDP:
+ case IPPROTO_UDP: /* fallthrough */
+ case IPPROTO_UDPLITE:
timeouts = nf_udp_pernet(net)->timeouts;
break;
case IPPROTO_DCCP:
@@ -469,13 +470,19 @@ static int cttimeout_default_get(struct net *net, struct
sock *ctnl,
case IPPROTO_SCTP:
#ifdef CONFIG_NF_CT_PROTO_SCTP
timeouts = nf_sctp_pernet(net)->timeouts;
+#endif
+ break;
+ case IPPROTO_GRE:
+#ifdef CONFIG_NF_CT_PROTO_GRE
+ if (l4proto->net_id)
+ timeouts = net_generic(net, *l4proto->net_id);
#endif
break;
case 255:
timeouts = &nf_generic_pernet(net)->timeout;
break;
default:
- WARN_ON_ONCE(1);
+ WARN_ONCE(1, "Missing timeouts for proto %d", l4proto->l4proto);
break;
}
--
2.18.1
Re: [PATCHv2 net] ipvs: call ip_vs_dst_notifier earlier than ipv6_dev_notf
On Fri, Nov 16, 2018 at 06:37:19AM -0800, Simon Horman wrote:
> On Fri, Nov 16, 2018 at 09:10:16AM +0200, Julian Anastasov wrote:
> >
> > Hello,
> >
> > On Thu, 15 Nov 2018, Xin Long wrote:
> >
> > > ip_vs_dst_event is supposed to clean up all dst used in ipvs'
> > > destinations when a net dev is going down. But it works only
> > > when the dst's dev is the same as the dev from the event.
> > >
> > > Now with the same priority but late registration,
> > > ip_vs_dst_notifier is always called later than ipv6_dev_notf
> > > where the dst's dev is set to lo for NETDEV_DOWN event.
> > >
> > > As the dst's dev lo is not the same as the dev from the event
> > > in ip_vs_dst_event, ip_vs_dst_notifier doesn't actually work.
> > > Also as these dst have to wait for dest_trash_timer to clean
> > > them up. It would cause some non-permanent kernel warnings:
> > >
> > > unregister_netdevice: waiting for br0 to become free. Usage count = 3
> > >
> > > To fix it, call ip_vs_dst_notifier earlier than ipv6_dev_notf
> > > by increasing its priority to ADDRCONF_NOTIFY_PRIORITY + 5.
> > >
> > > Note that for ipv4 route fib_netdev_notifier doesn't set dst's
> > > dev to lo in NETDEV_DOWN event, so this fix is only needed when
> > > IP_VS_IPV6 is defined.
> > >
> > > v1->v2:
> > > - apply it only when CONFIG_IP_VS_IPV6 is defined.
> > >
> > > Fixes: 7a4f0761fce3 ("IPVS: init and cleanup restructuring")
> > > Reported-by: Li Shuang
> > > Signed-off-by: Xin Long
> >
> > Acked-by: Julian Anastasov
>
> Thanks,
>
> Pablo, could you consider this for nf?
>
> Acked-by: Simon Horman
Applied, thanks Simon.
Re: [PATCH nf] netfilter: nfnetlink_cttimeout: fetch timeouts for udplite and gre, too
On Sat, Nov 17, 2018 at 11:32:29AM +0100, Florian Westphal wrote: > syzbot was able to trigger the WARN in cttimeout_default_get() by > passing UDPLITE as l4protocol. Alias UDPLITE to UDP, both use > same timeout values. > > Furthermore, also fetch GRE timeouts. GRE is a bit more complicated, > as it still can be a module and its netns_proto_gre struct layout isn't > visible outside of the gre module. > > Work around this by forcing the timeouts to be the first structure > member, then use plain net_generic(). > > A followup nf-next patch could make gre tracker be built-in as well > if needed, its not that large. > > Last, make the WARN() mention the missing protocol value in case > anything else is missing. Applied, thanks Florian.
Re: [PATCHv2 net] ipvs: call ip_vs_dst_notifier earlier than ipv6_dev_notf
On Sat, Nov 17, 2018 at 8:15 PM Pablo Neira Ayuso wrote:
>
> On Fri, Nov 16, 2018 at 06:37:19AM -0800, Simon Horman wrote:
> > On Fri, Nov 16, 2018 at 09:10:16AM +0200, Julian Anastasov wrote:
> > >
> > > Hello,
> > >
> > > On Thu, 15 Nov 2018, Xin Long wrote:
> > >
> > > > ip_vs_dst_event is supposed to clean up all dst used in ipvs'
> > > > destinations when a net dev is going down. But it works only
> > > > when the dst's dev is the same as the dev from the event.
> > > >
> > > > Now with the same priority but late registration,
> > > > ip_vs_dst_notifier is always called later than ipv6_dev_notf
> > > > where the dst's dev is set to lo for NETDEV_DOWN event.
> > > >
> > > > As the dst's dev lo is not the same as the dev from the event
> > > > in ip_vs_dst_event, ip_vs_dst_notifier doesn't actually work.
> > > > Also as these dst have to wait for dest_trash_timer to clean
> > > > them up. It would cause some non-permanent kernel warnings:
> > > >
> > > > unregister_netdevice: waiting for br0 to become free. Usage count = 3
> > > >
> > > > To fix it, call ip_vs_dst_notifier earlier than ipv6_dev_notf
> > > > by increasing its priority to ADDRCONF_NOTIFY_PRIORITY + 5.
> > > >
> > > > Note that for ipv4 route fib_netdev_notifier doesn't set dst's
> > > > dev to lo in NETDEV_DOWN event, so this fix is only needed when
> > > > IP_VS_IPV6 is defined.
> > > >
> > > > v1->v2:
> > > > - apply it only when CONFIG_IP_VS_IPV6 is defined.
> > > >
> > > > Fixes: 7a4f0761fce3 ("IPVS: init and cleanup restructuring")
> > > > Reported-by: Li Shuang
> > > > Signed-off-by: Xin Long
> > >
> > > Acked-by: Julian Anastasov
> >
> > Thanks,
> >
> > Pablo, could you consider this for nf?
> >
> > Acked-by: Simon Horman
>
> Applied, thanks Simon.
Hi Pablo,
The one you just applied is the v1, I'm afraid you need
to revert and apply the v2, which fixed a build error
when IPv6 is disabled.
Re: [PATCHv2 net] ipvs: call ip_vs_dst_notifier earlier than ipv6_dev_notf
On Sat, Nov 17, 2018 at 09:19:52PM +0900, Xin Long wrote:
> On Sat, Nov 17, 2018 at 8:15 PM Pablo Neira Ayuso wrote:
> >
> > On Fri, Nov 16, 2018 at 06:37:19AM -0800, Simon Horman wrote:
> > > On Fri, Nov 16, 2018 at 09:10:16AM +0200, Julian Anastasov wrote:
> > > >
> > > > Hello,
> > > >
> > > > On Thu, 15 Nov 2018, Xin Long wrote:
> > > >
> > > > > ip_vs_dst_event is supposed to clean up all dst used in ipvs'
> > > > > destinations when a net dev is going down. But it works only
> > > > > when the dst's dev is the same as the dev from the event.
> > > > >
> > > > > Now with the same priority but late registration,
> > > > > ip_vs_dst_notifier is always called later than ipv6_dev_notf
> > > > > where the dst's dev is set to lo for NETDEV_DOWN event.
> > > > >
> > > > > As the dst's dev lo is not the same as the dev from the event
> > > > > in ip_vs_dst_event, ip_vs_dst_notifier doesn't actually work.
> > > > > Also as these dst have to wait for dest_trash_timer to clean
> > > > > them up. It would cause some non-permanent kernel warnings:
> > > > >
> > > > > unregister_netdevice: waiting for br0 to become free. Usage count =
> > > > > 3
> > > > >
> > > > > To fix it, call ip_vs_dst_notifier earlier than ipv6_dev_notf
> > > > > by increasing its priority to ADDRCONF_NOTIFY_PRIORITY + 5.
> > > > >
> > > > > Note that for ipv4 route fib_netdev_notifier doesn't set dst's
> > > > > dev to lo in NETDEV_DOWN event, so this fix is only needed when
> > > > > IP_VS_IPV6 is defined.
> > > > >
> > > > > v1->v2:
> > > > > - apply it only when CONFIG_IP_VS_IPV6 is defined.
> > > > >
> > > > > Fixes: 7a4f0761fce3 ("IPVS: init and cleanup restructuring")
> > > > > Reported-by: Li Shuang
> > > > > Signed-off-by: Xin Long
> > > >
> > > > Acked-by: Julian Anastasov
> > >
> > > Thanks,
> > >
> > > Pablo, could you consider this for nf?
> > >
> > > Acked-by: Simon Horman
> >
> > Applied, thanks Simon.
> Hi Pablo,
>
> The one you just applied is the v1, I'm afraid you need
> to revert and apply the v2, which fixed a build error
> when IPv6 is disabled.
Fixed, thanks!
