Skip to content

Commit

Permalink
netfilter: nfnetlink: nfnetlink_unicast() reports EAGAIN instead of E…
Browse files Browse the repository at this point in the history 
…NOBUFS

Frontend callback reports EAGAIN to nfnetlink to retry a command, this
is used to signal that module autoloading is required. Unfortunately,
nlmsg_unicast() reports EAGAIN in case the receiver socket buffer gets
full, so it enters a busy-loop.

This patch updates nfnetlink_unicast() to turn EAGAIN into ENOBUFS and
to use nlmsg_unicast(). Remove the flags field in nfnetlink_unicast()
since this is always MSG_DONTWAIT in the existing code which is exactly
what nlmsg_unicast() passes to netlink_unicast() as parameter.

Fixes: 9651851 ("netfilter: add nftables")
Reported-by: Phil Sutter <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
  • Loading branch information
@ummakynes
ummakynes committed Aug 28, 2020
1 parent 4b7ddc5 commit ee92118
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 40 deletions.
3 changes: 1 addition & 2 deletions include/linux/netfilter/nfnetlink.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ int nfnetlink_has_listeners(struct net *net, unsigned int group);
int nfnetlink_send(struct sk_buff *skb, struct net *net, u32 portid,
unsigned int group, int echo, gfp_t flags);
int nfnetlink_set_err(struct net *net, u32 portid, u32 group, int error);
int nfnetlink_unicast(struct sk_buff *skb, struct net *net, u32 portid,
int flags);
int nfnetlink_unicast(struct sk_buff *skb, struct net *net, u32 portid);

static inline u16 nfnl_msg_type(u8 subsys, u8 msg_type)
{
Expand Down
61 changes: 29 additions & 32 deletions net/netfilter/nf_tables_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -815,11 +815,11 @@ static int nf_tables_gettable(struct net *net, struct sock *nlsk,
nlh->nlmsg_seq, NFT_MSG_NEWTABLE, 0,
family, table);
if (err < 0)
goto err;
goto err_fill_table_info;

return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);

err:
err_fill_table_info:
kfree_skb(skb2);
return err;
}
Expand Down Expand Up @@ -1563,11 +1563,11 @@ static int nf_tables_getchain(struct net *net, struct sock *nlsk,
nlh->nlmsg_seq, NFT_MSG_NEWCHAIN, 0,
family, table, chain);
if (err < 0)
goto err;
goto err_fill_chain_info;

return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);

err:
err_fill_chain_info:
kfree_skb(skb2);
return err;
}
Expand Down Expand Up @@ -3008,11 +3008,11 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0,
family, table, chain, rule, NULL);
if (err < 0)
goto err;
goto err_fill_rule_info;

return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);

err:
err_fill_rule_info:
kfree_skb(skb2);
return err;
}
Expand Down Expand Up @@ -3968,11 +3968,11 @@ static int nf_tables_getset(struct net *net, struct sock *nlsk,

err = nf_tables_fill_set(skb2, &ctx, set, NFT_MSG_NEWSET, 0);
if (err < 0)
goto err;
goto err_fill_set_info;

return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);

err:
err_fill_set_info:
kfree_skb(skb2);
return err;
}
Expand Down Expand Up @@ -4860,24 +4860,18 @@ static int nft_get_set_elem(struct nft_ctx *ctx, struct nft_set *set,
err = -ENOMEM;
skb = nlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC);
if (skb == NULL)
goto err1;
return err;

err = nf_tables_fill_setelem_info(skb, ctx, ctx->seq, ctx->portid,
NFT_MSG_NEWSETELEM, 0, set, &elem);
if (err < 0)
goto err2;
goto err_fill_setelem;

err = nfnetlink_unicast(skb, ctx->net, ctx->portid, MSG_DONTWAIT);
/* This avoids a loop in nfnetlink. */
if (err < 0)
goto err1;
return nfnetlink_unicast(skb, ctx->net, ctx->portid);

return 0;
err2:
err_fill_setelem:
kfree_skb(skb);
err1:
/* this avoids a loop in nfnetlink. */
return err == -EAGAIN ? -ENOBUFS : err;
return err;
}

/* called with rcu_read_lock held */
Expand Down Expand Up @@ -6182,10 +6176,11 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0,
family, table, obj, reset);
if (err < 0)
goto err;
goto err_fill_obj_info;

return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
err:
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);

err_fill_obj_info:
kfree_skb(skb2);
return err;
}
Expand Down Expand Up @@ -7045,10 +7040,11 @@ static int nf_tables_getflowtable(struct net *net, struct sock *nlsk,
NFT_MSG_NEWFLOWTABLE, 0, family,
flowtable, &flowtable->hook_list);
if (err < 0)
goto err;
goto err_fill_flowtable_info;

return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
err:
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);

err_fill_flowtable_info:
kfree_skb(skb2);
return err;
}
Expand Down Expand Up @@ -7234,10 +7230,11 @@ static int nf_tables_getgen(struct net *net, struct sock *nlsk,
err = nf_tables_fill_gen_info(skb2, net, NETLINK_CB(skb).portid,
nlh->nlmsg_seq);
if (err < 0)
goto err;
goto err_fill_gen_info;

return nlmsg_unicast(nlsk, skb2, NETLINK_CB(skb).portid);
err:
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);

err_fill_gen_info:
kfree_skb(skb2);
return err;
}
Expand Down
11 changes: 8 additions & 3 deletions net/netfilter/nfnetlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,15 @@ int nfnetlink_set_err(struct net *net, u32 portid, u32 group, int error)
}
EXPORT_SYMBOL_GPL(nfnetlink_set_err);

int nfnetlink_unicast(struct sk_buff *skb, struct net *net, u32 portid,
int flags)
int nfnetlink_unicast(struct sk_buff *skb, struct net *net, u32 portid)
{
return netlink_unicast(net->nfnl, skb, portid, flags);
int err;

err = nlmsg_unicast(net->nfnl, skb, portid);
if (err == -EAGAIN)
err = -ENOBUFS;

return err;
}
EXPORT_SYMBOL_GPL(nfnetlink_unicast);

Expand Down
3 changes: 1 addition & 2 deletions net/netfilter/nfnetlink_log.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,7 @@ __nfulnl_send(struct nfulnl_instance *inst)
goto out;
}
}
nfnetlink_unicast(inst->skb, inst->net, inst->peer_portid,
MSG_DONTWAIT);
nfnetlink_unicast(inst->skb, inst->net, inst->peer_portid);
out:
inst->qlen = 0;
inst->skb = NULL;
Expand Down
2 changes: 1 addition & 1 deletion net/netfilter/nfnetlink_queue.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ __nfqnl_enqueue_packet(struct net *net, struct nfqnl_instance *queue,
*packet_id_ptr = htonl(entry->id);

/* nfnetlink_unicast will either free the nskb or add it to a socket */
err = nfnetlink_unicast(nskb, net, queue->peer_portid, MSG_DONTWAIT);
err = nfnetlink_unicast(nskb, net, queue->peer_portid);
if (err < 0) {
if (queue->flags & NFQA_CFG_F_FAIL_OPEN) {
failopen = 1;
Expand Down

0 comments on commit ee92118

Please sign in to comment.