* [PATCH nf 1/2] netfilter: nfnetlink: avoid recurrent netns lookups in call_batch
@ 2015-12-09 12:12 Pablo Neira Ayuso
2015-12-09 12:12 ` [PATCH nf 2/2] nfnetlink: fix splat due to incorrect socket memory accounting in skbuff clones Pablo Neira Ayuso
2015-12-10 8:38 ` [PATCH nf 1/2] netfilter: nfnetlink: avoid recurrent netns lookups in call_batch Arturo Borrero Gonzalez
0 siblings, 2 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2015-12-09 12:12 UTC (permalink / raw)
To: netfilter-devel; +Cc: arturo.borrero.glez, ben
Pass the net pointer to the call_batch callback functions so we can skip
recurrent lookups.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
I would really appreciate to get a Tested-by: tag from you on this.
include/linux/netfilter/nfnetlink.h | 2 +-
net/netfilter/nf_tables_api.c | 96 +++++++++++++++++--------------------
net/netfilter/nfnetlink.c | 2 +-
3 files changed, 47 insertions(+), 53 deletions(-)
diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index 249d1bb..5646b24 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -14,7 +14,7 @@ struct nfnl_callback {
int (*call_rcu)(struct sock *nl, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const cda[]);
- int (*call_batch)(struct sock *nl, struct sk_buff *skb,
+ int (*call_batch)(struct net *net, struct sock *nl, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const cda[]);
const struct nla_policy *policy; /* netlink attribute policy */
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 428b90a..dd9405b 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -89,6 +89,7 @@ nf_tables_afinfo_lookup(struct net *net, int family, bool autoload)
}
static void nft_ctx_init(struct nft_ctx *ctx,
+ struct net *net,
const struct sk_buff *skb,
const struct nlmsghdr *nlh,
struct nft_af_info *afi,
@@ -96,7 +97,7 @@ static void nft_ctx_init(struct nft_ctx *ctx,
struct nft_chain *chain,
const struct nlattr * const *nla)
{
- ctx->net = sock_net(skb->sk);
+ ctx->net = net;
ctx->afi = afi;
ctx->table = table;
ctx->chain = chain;
@@ -672,15 +673,14 @@ err:
return ret;
}
-static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_newtable(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
const struct nlattr *name;
struct nft_af_info *afi;
struct nft_table *table;
- struct net *net = sock_net(skb->sk);
int family = nfmsg->nfgen_family;
u32 flags = 0;
struct nft_ctx ctx;
@@ -706,7 +706,7 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_REPLACE)
return -EOPNOTSUPP;
- nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, afi, table, NULL, nla);
return nf_tables_updtable(&ctx);
}
@@ -730,7 +730,7 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
INIT_LIST_HEAD(&table->sets);
table->flags = flags;
- nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, afi, table, NULL, nla);
err = nft_trans_table_add(&ctx, NFT_MSG_NEWTABLE);
if (err < 0)
goto err3;
@@ -810,18 +810,17 @@ out:
return err;
}
-static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_deltable(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
struct nft_af_info *afi;
struct nft_table *table;
- struct net *net = sock_net(skb->sk);
int family = nfmsg->nfgen_family;
struct nft_ctx ctx;
- nft_ctx_init(&ctx, skb, nlh, NULL, NULL, NULL, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, NULL, NULL, NULL, nla);
if (family == AF_UNSPEC || nla[NFTA_TABLE_NAME] == NULL)
return nft_flush(&ctx, family);
@@ -1221,8 +1220,8 @@ static void nf_tables_chain_destroy(struct nft_chain *chain)
}
}
-static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_newchain(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
@@ -1232,7 +1231,6 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
struct nft_chain *chain;
struct nft_base_chain *basechain = NULL;
struct nlattr *ha[NFTA_HOOK_MAX + 1];
- struct net *net = sock_net(skb->sk);
int family = nfmsg->nfgen_family;
struct net_device *dev = NULL;
u8 policy = NF_ACCEPT;
@@ -1313,7 +1311,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
return PTR_ERR(stats);
}
- nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
trans = nft_trans_alloc(&ctx, NFT_MSG_NEWCHAIN,
sizeof(struct nft_trans_chain));
if (trans == NULL) {
@@ -1461,7 +1459,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
if (err < 0)
goto err1;
- nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
err = nft_trans_chain_add(&ctx, NFT_MSG_NEWCHAIN);
if (err < 0)
goto err2;
@@ -1476,15 +1474,14 @@ err1:
return err;
}
-static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_delchain(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
struct nft_af_info *afi;
struct nft_table *table;
struct nft_chain *chain;
- struct net *net = sock_net(skb->sk);
int family = nfmsg->nfgen_family;
struct nft_ctx ctx;
@@ -1506,7 +1503,7 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
if (chain->use > 0)
return -EBUSY;
- nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
return nft_delchain(&ctx);
}
@@ -2010,13 +2007,12 @@ static void nf_tables_rule_destroy(const struct nft_ctx *ctx,
static struct nft_expr_info *info;
-static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_newrule(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
struct nft_af_info *afi;
- struct net *net = sock_net(skb->sk);
struct nft_table *table;
struct nft_chain *chain;
struct nft_rule *rule, *old_rule = NULL;
@@ -2075,7 +2071,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
return PTR_ERR(old_rule);
}
- nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
n = 0;
size = 0;
@@ -2176,13 +2172,12 @@ err1:
return err;
}
-static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_delrule(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
struct nft_af_info *afi;
- struct net *net = sock_net(skb->sk);
struct nft_table *table;
struct nft_chain *chain = NULL;
struct nft_rule *rule;
@@ -2205,7 +2200,7 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
return PTR_ERR(chain);
}
- nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
if (chain) {
if (nla[NFTA_RULE_HANDLE]) {
@@ -2344,12 +2339,11 @@ static const struct nla_policy nft_set_desc_policy[NFTA_SET_DESC_MAX + 1] = {
[NFTA_SET_DESC_SIZE] = { .type = NLA_U32 },
};
-static int nft_ctx_init_from_setattr(struct nft_ctx *ctx,
+static int nft_ctx_init_from_setattr(struct nft_ctx *ctx, struct net *net,
const struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
- struct net *net = sock_net(skb->sk);
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
struct nft_af_info *afi = NULL;
struct nft_table *table = NULL;
@@ -2371,7 +2365,7 @@ static int nft_ctx_init_from_setattr(struct nft_ctx *ctx,
return -ENOENT;
}
- nft_ctx_init(ctx, skb, nlh, afi, table, NULL, nla);
+ nft_ctx_init(ctx, net, skb, nlh, afi, table, NULL, nla);
return 0;
}
@@ -2623,6 +2617,7 @@ static int nf_tables_getset(struct sock *nlsk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
+ struct net *net = sock_net(skb->sk);
const struct nft_set *set;
struct nft_ctx ctx;
struct sk_buff *skb2;
@@ -2630,7 +2625,7 @@ static int nf_tables_getset(struct sock *nlsk, struct sk_buff *skb,
int err;
/* Verify existence before starting dump */
- err = nft_ctx_init_from_setattr(&ctx, skb, nlh, nla);
+ err = nft_ctx_init_from_setattr(&ctx, net, skb, nlh, nla);
if (err < 0)
return err;
@@ -2693,14 +2688,13 @@ static int nf_tables_set_desc_parse(const struct nft_ctx *ctx,
return 0;
}
-static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_newset(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
const struct nft_set_ops *ops;
struct nft_af_info *afi;
- struct net *net = sock_net(skb->sk);
struct nft_table *table;
struct nft_set *set;
struct nft_ctx ctx;
@@ -2798,7 +2792,7 @@ static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
if (IS_ERR(table))
return PTR_ERR(table);
- nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
+ nft_ctx_init(&ctx, net, skb, nlh, afi, table, NULL, nla);
set = nf_tables_set_lookup(table, nla[NFTA_SET_NAME]);
if (IS_ERR(set)) {
@@ -2882,8 +2876,8 @@ static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set
nft_set_destroy(set);
}
-static int nf_tables_delset(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_delset(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
@@ -2896,7 +2890,7 @@ static int nf_tables_delset(struct sock *nlsk, struct sk_buff *skb,
if (nla[NFTA_SET_TABLE] == NULL)
return -EINVAL;
- err = nft_ctx_init_from_setattr(&ctx, skb, nlh, nla);
+ err = nft_ctx_init_from_setattr(&ctx, net, skb, nlh, nla);
if (err < 0)
return err;
@@ -3024,7 +3018,7 @@ static const struct nla_policy nft_set_elem_list_policy[NFTA_SET_ELEM_LIST_MAX +
[NFTA_SET_ELEM_LIST_SET_ID] = { .type = NLA_U32 },
};
-static int nft_ctx_init_from_elemattr(struct nft_ctx *ctx,
+static int nft_ctx_init_from_elemattr(struct nft_ctx *ctx, struct net *net,
const struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nla[],
@@ -3033,7 +3027,6 @@ static int nft_ctx_init_from_elemattr(struct nft_ctx *ctx,
const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
struct nft_af_info *afi;
struct nft_table *table;
- struct net *net = sock_net(skb->sk);
afi = nf_tables_afinfo_lookup(net, nfmsg->nfgen_family, false);
if (IS_ERR(afi))
@@ -3045,7 +3038,7 @@ static int nft_ctx_init_from_elemattr(struct nft_ctx *ctx,
if (!trans && (table->flags & NFT_TABLE_INACTIVE))
return -ENOENT;
- nft_ctx_init(ctx, skb, nlh, afi, table, NULL, nla);
+ nft_ctx_init(ctx, net, skb, nlh, afi, table, NULL, nla);
return 0;
}
@@ -3135,6 +3128,7 @@ static int nf_tables_dump_setelem(const struct nft_ctx *ctx,
static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
{
+ struct net *net = sock_net(skb->sk);
const struct nft_set *set;
struct nft_set_dump_args args;
struct nft_ctx ctx;
@@ -3150,8 +3144,8 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
if (err < 0)
return err;
- err = nft_ctx_init_from_elemattr(&ctx, cb->skb, cb->nlh, (void *)nla,
- false);
+ err = nft_ctx_init_from_elemattr(&ctx, net, cb->skb, cb->nlh,
+ (void *)nla, false);
if (err < 0)
return err;
@@ -3212,11 +3206,12 @@ static int nf_tables_getsetelem(struct sock *nlsk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
+ struct net *net = sock_net(skb->sk);
const struct nft_set *set;
struct nft_ctx ctx;
int err;
- err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla, false);
+ err = nft_ctx_init_from_elemattr(&ctx, net, skb, nlh, nla, false);
if (err < 0)
return err;
@@ -3528,11 +3523,10 @@ err1:
return err;
}
-static int nf_tables_newsetelem(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_newsetelem(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
- struct net *net = sock_net(skb->sk);
const struct nlattr *attr;
struct nft_set *set;
struct nft_ctx ctx;
@@ -3541,7 +3535,7 @@ static int nf_tables_newsetelem(struct sock *nlsk, struct sk_buff *skb,
if (nla[NFTA_SET_ELEM_LIST_ELEMENTS] == NULL)
return -EINVAL;
- err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla, true);
+ err = nft_ctx_init_from_elemattr(&ctx, net, skb, nlh, nla, true);
if (err < 0)
return err;
@@ -3623,8 +3617,8 @@ err1:
return err;
}
-static int nf_tables_delsetelem(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
+static int nf_tables_delsetelem(struct net *net, struct sock *nlsk,
+ struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
const struct nlattr *attr;
@@ -3635,7 +3629,7 @@ static int nf_tables_delsetelem(struct sock *nlsk, struct sk_buff *skb,
if (nla[NFTA_SET_ELEM_LIST_ELEMENTS] == NULL)
return -EINVAL;
- err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla, false);
+ err = nft_ctx_init_from_elemattr(&ctx, net, skb, nlh, nla, false);
if (err < 0)
return err;
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 46453ab..445590f 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -381,7 +381,7 @@ replay:
goto ack;
if (nc->call_batch) {
- err = nc->call_batch(net->nfnl, skb, nlh,
+ err = nc->call_batch(net, net->nfnl, skb, nlh,
(const struct nlattr **)cda);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH nf 2/2] nfnetlink: fix splat due to incorrect socket memory accounting in skbuff clones
2015-12-09 12:12 [PATCH nf 1/2] netfilter: nfnetlink: avoid recurrent netns lookups in call_batch Pablo Neira Ayuso
@ 2015-12-09 12:12 ` Pablo Neira Ayuso
2015-12-10 8:39 ` Arturo Borrero Gonzalez
2015-12-10 8:38 ` [PATCH nf 1/2] netfilter: nfnetlink: avoid recurrent netns lookups in call_batch Arturo Borrero Gonzalez
1 sibling, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2015-12-09 12:12 UTC (permalink / raw)
To: netfilter-devel; +Cc: arturo.borrero.glez, ben
If we attach the sk to the skb, netlink_skb_destructor() will underflow
the socket receive memory counter and we get warning splat when
releasing the socket.
$ cat /proc/net/netlink
sk Eth Pid Groups Rmem Wmem Dump Locks Drops Inode
ffff8800ca903000 12 0 00000000 -54144 0 0 2 0 17942
^^^^^^
Rmem above shows an underflow.
And here below the warning splat:
[ 1363.815976] WARNING: CPU: 2 PID: 1356 at net/netlink/af_netlink.c:958 netlink_sock_destruct+0x80/0xb9()
[...]
[ 1363.816152] CPU: 2 PID: 1356 Comm: kworker/u16:1 Tainted: G W 4.4.0-rc1+ #153
[ 1363.816155] Hardware name: LENOVO 23259H1/23259H1, BIOS G2ET32WW (1.12 ) 05/30/2012
[ 1363.816160] Workqueue: netns cleanup_net
[ 1363.816163] 0000000000000000 ffff880119203dd0 ffffffff81240204 0000000000000000
[ 1363.816169] ffff880119203e08 ffffffff8104db4b ffffffff813d49a1 ffff8800ca771000
[ 1363.816174] ffffffff81a42b00 0000000000000000 ffff8800c0afe1e0 ffff880119203e18
[ 1363.816179] Call Trace:
[ 1363.816181] <IRQ> [<ffffffff81240204>] dump_stack+0x4e/0x79
[ 1363.816193] [<ffffffff8104db4b>] warn_slowpath_common+0x9a/0xb3
[ 1363.816197] [<ffffffff813d49a1>] ? netlink_sock_destruct+0x80/0xb9
skb->sk was only needed to lookup for the netns, however we don't need
this anymore since ("netfilter: nfnetlink: avoid recurrent netns lookups
in call_batch"), so this patch removes this manual socket assignment.
Reported-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
I would really appreciate to get a Tested-by: tag from you on this.
net/netfilter/nfnetlink.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 445590f..77afe91 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -295,8 +295,6 @@ replay:
if (!skb)
return netlink_ack(oskb, nlh, -ENOMEM);
- skb->sk = oskb->sk;
-
nfnl_lock(subsys_id);
ss = rcu_dereference_protected(table[subsys_id].subsys,
lockdep_is_held(&table[subsys_id].mutex));
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH nf 2/2] nfnetlink: fix splat due to incorrect socket memory accounting in skbuff clones
2015-12-09 12:12 ` [PATCH nf 2/2] nfnetlink: fix splat due to incorrect socket memory accounting in skbuff clones Pablo Neira Ayuso
@ 2015-12-10 8:39 ` Arturo Borrero Gonzalez
2015-12-10 12:51 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Arturo Borrero Gonzalez @ 2015-12-10 8:39 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Netfilter Development Mailing list, Ben Hutchings
On 9 December 2015 at 13:12, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> If we attach the sk to the skb, netlink_skb_destructor() will underflow
> the socket receive memory counter and we get warning splat when
> releasing the socket.
>
> $ cat /proc/net/netlink
> sk Eth Pid Groups Rmem Wmem Dump Locks Drops Inode
> ffff8800ca903000 12 0 00000000 -54144 0 0 2 0 17942
> ^^^^^^
>
> Rmem above shows an underflow.
>
> And here below the warning splat:
>
> [ 1363.815976] WARNING: CPU: 2 PID: 1356 at net/netlink/af_netlink.c:958 netlink_sock_destruct+0x80/0xb9()
> [...]
> [ 1363.816152] CPU: 2 PID: 1356 Comm: kworker/u16:1 Tainted: G W 4.4.0-rc1+ #153
> [ 1363.816155] Hardware name: LENOVO 23259H1/23259H1, BIOS G2ET32WW (1.12 ) 05/30/2012
> [ 1363.816160] Workqueue: netns cleanup_net
> [ 1363.816163] 0000000000000000 ffff880119203dd0 ffffffff81240204 0000000000000000
> [ 1363.816169] ffff880119203e08 ffffffff8104db4b ffffffff813d49a1 ffff8800ca771000
> [ 1363.816174] ffffffff81a42b00 0000000000000000 ffff8800c0afe1e0 ffff880119203e18
> [ 1363.816179] Call Trace:
> [ 1363.816181] <IRQ> [<ffffffff81240204>] dump_stack+0x4e/0x79
> [ 1363.816193] [<ffffffff8104db4b>] warn_slowpath_common+0x9a/0xb3
> [ 1363.816197] [<ffffffff813d49a1>] ? netlink_sock_destruct+0x80/0xb9
>
> skb->sk was only needed to lookup for the netns, however we don't need
> this anymore since ("netfilter: nfnetlink: avoid recurrent netns lookups
> in call_batch"), so this patch removes this manual socket assignment.
>
> Reported-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> Reported-by: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> I would really appreciate to get a Tested-by: tag from you on this.
>
> net/netfilter/nfnetlink.c | 2 --
> 1 file changed, 2 deletions(-)
thanks, the problem seems to be fixed now.
Tested-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
--
Arturo Borrero González
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH nf 2/2] nfnetlink: fix splat due to incorrect socket memory accounting in skbuff clones
2015-12-10 8:39 ` Arturo Borrero Gonzalez
@ 2015-12-10 12:51 ` Pablo Neira Ayuso
0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2015-12-10 12:51 UTC (permalink / raw)
To: Arturo Borrero Gonzalez; +Cc: Netfilter Development Mailing list, Ben Hutchings
On Thu, Dec 10, 2015 at 09:39:28AM +0100, Arturo Borrero Gonzalez wrote:
> On 9 December 2015 at 13:12, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > If we attach the sk to the skb, netlink_skb_destructor() will underflow
> > the socket receive memory counter and we get warning splat when
> > releasing the socket.
> >
> > $ cat /proc/net/netlink
> > sk Eth Pid Groups Rmem Wmem Dump Locks Drops Inode
> > ffff8800ca903000 12 0 00000000 -54144 0 0 2 0 17942
> > ^^^^^^
> >
> > Rmem above shows an underflow.
> >
> > And here below the warning splat:
> >
> > [ 1363.815976] WARNING: CPU: 2 PID: 1356 at net/netlink/af_netlink.c:958 netlink_sock_destruct+0x80/0xb9()
> > [...]
> > [ 1363.816152] CPU: 2 PID: 1356 Comm: kworker/u16:1 Tainted: G W 4.4.0-rc1+ #153
> > [ 1363.816155] Hardware name: LENOVO 23259H1/23259H1, BIOS G2ET32WW (1.12 ) 05/30/2012
> > [ 1363.816160] Workqueue: netns cleanup_net
> > [ 1363.816163] 0000000000000000 ffff880119203dd0 ffffffff81240204 0000000000000000
> > [ 1363.816169] ffff880119203e08 ffffffff8104db4b ffffffff813d49a1 ffff8800ca771000
> > [ 1363.816174] ffffffff81a42b00 0000000000000000 ffff8800c0afe1e0 ffff880119203e18
> > [ 1363.816179] Call Trace:
> > [ 1363.816181] <IRQ> [<ffffffff81240204>] dump_stack+0x4e/0x79
> > [ 1363.816193] [<ffffffff8104db4b>] warn_slowpath_common+0x9a/0xb3
> > [ 1363.816197] [<ffffffff813d49a1>] ? netlink_sock_destruct+0x80/0xb9
> >
> > skb->sk was only needed to lookup for the netns, however we don't need
> > this anymore since ("netfilter: nfnetlink: avoid recurrent netns lookups
> > in call_batch"), so this patch removes this manual socket assignment.
> >
> > Reported-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> > Reported-by: Ben Hutchings <ben@decadent.org.uk>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> > I would really appreciate to get a Tested-by: tag from you on this.
> >
> > net/netfilter/nfnetlink.c | 2 --
> > 1 file changed, 2 deletions(-)
>
> thanks, the problem seems to be fixed now.
>
> Tested-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
Thanks for testing Arturo!
It would be good to give a another testing given this is related to
netns as well: http://patchwork.ozlabs.org/patch/554791/. What I could
test here showed no problems.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH nf 1/2] netfilter: nfnetlink: avoid recurrent netns lookups in call_batch
2015-12-09 12:12 [PATCH nf 1/2] netfilter: nfnetlink: avoid recurrent netns lookups in call_batch Pablo Neira Ayuso
2015-12-09 12:12 ` [PATCH nf 2/2] nfnetlink: fix splat due to incorrect socket memory accounting in skbuff clones Pablo Neira Ayuso
@ 2015-12-10 8:38 ` Arturo Borrero Gonzalez
1 sibling, 0 replies; 5+ messages in thread
From: Arturo Borrero Gonzalez @ 2015-12-10 8:38 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Netfilter Development Mailing list, Ben Hutchings
On 9 December 2015 at 13:12, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Pass the net pointer to the call_batch callback functions so we can skip
> recurrent lookups.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> I would really appreciate to get a Tested-by: tag from you on this.
>
> include/linux/netfilter/nfnetlink.h | 2 +-
> net/netfilter/nf_tables_api.c | 96 +++++++++++++++++--------------------
> net/netfilter/nfnetlink.c | 2 +-
> 3 files changed, 47 insertions(+), 53 deletions(-)
>
thanks, the problem seems to be fixed now.
Tested-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
--
Arturo Borrero González
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-10 12:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-09 12:12 [PATCH nf 1/2] netfilter: nfnetlink: avoid recurrent netns lookups in call_batch Pablo Neira Ayuso
2015-12-09 12:12 ` [PATCH nf 2/2] nfnetlink: fix splat due to incorrect socket memory accounting in skbuff clones Pablo Neira Ayuso
2015-12-10 8:39 ` Arturo Borrero Gonzalez
2015-12-10 12:51 ` Pablo Neira Ayuso
2015-12-10 8:38 ` [PATCH nf 1/2] netfilter: nfnetlink: avoid recurrent netns lookups in call_batch Arturo Borrero Gonzalez
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.