* [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 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
* 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
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).