Skip to content

Commit

Permalink
netfilter: ipset: fix suspicious RCU usage in find_set_and_id
Browse files Browse the repository at this point in the history
find_set_and_id() is called when the NFNL_SUBSYS_IPSET mutex is held.
However, in the error path there can be a follow-up recvmsg() without
the mutex held. Use the start() function of struct netlink_dump_control
instead of dump() to verify and report if the specified set does not
exist.

Thanks to Pablo Neira Ayuso for helping me to understand the subleties
of the netlink protocol.

Reported-by: syzbot+fc69d7cb21258ab4ae4d@syzkaller.appspotmail.com
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  • Loading branch information
Kadlecsik József authored and ummakynes committed Jan 29, 2020
1 parent 44efc78 commit 5038517
Showing 1 changed file with 21 additions and 20 deletions.
41 changes: 21 additions & 20 deletions net/netfilter/ipset/ip_set_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1483,31 +1483,34 @@ ip_set_dump_policy[IPSET_ATTR_CMD_MAX + 1] = {
};

static int
dump_init(struct netlink_callback *cb, struct ip_set_net *inst)
ip_set_dump_start(struct netlink_callback *cb)
{
struct nlmsghdr *nlh = nlmsg_hdr(cb->skb);
int min_len = nlmsg_total_size(sizeof(struct nfgenmsg));
struct nlattr *cda[IPSET_ATTR_CMD_MAX + 1];
struct nlattr *attr = (void *)nlh + min_len;
struct sk_buff *skb = cb->skb;
struct ip_set_net *inst = ip_set_pernet(sock_net(skb->sk));
u32 dump_type;
ip_set_id_t index;
int ret;

ret = nla_parse(cda, IPSET_ATTR_CMD_MAX, attr,
nlh->nlmsg_len - min_len,
ip_set_dump_policy, NULL);
if (ret)
return ret;
goto error;

cb->args[IPSET_CB_PROTO] = nla_get_u8(cda[IPSET_ATTR_PROTOCOL]);
if (cda[IPSET_ATTR_SETNAME]) {
ip_set_id_t index;
struct ip_set *set;

set = find_set_and_id(inst, nla_data(cda[IPSET_ATTR_SETNAME]),
&index);
if (!set)
return -ENOENT;

if (!set) {
ret = -ENOENT;
goto error;
}
dump_type = DUMP_ONE;
cb->args[IPSET_CB_INDEX] = index;
} else {
Expand All @@ -1523,10 +1526,17 @@ dump_init(struct netlink_callback *cb, struct ip_set_net *inst)
cb->args[IPSET_CB_DUMP] = dump_type;

return 0;

error:
/* We have to create and send the error message manually :-( */
if (nlh->nlmsg_flags & NLM_F_ACK) {
netlink_ack(cb->skb, nlh, ret, NULL);
}
return ret;
}

static int
ip_set_dump_start(struct sk_buff *skb, struct netlink_callback *cb)
ip_set_dump_do(struct sk_buff *skb, struct netlink_callback *cb)
{
ip_set_id_t index = IPSET_INVALID_ID, max;
struct ip_set *set = NULL;
Expand All @@ -1537,18 +1547,8 @@ ip_set_dump_start(struct sk_buff *skb, struct netlink_callback *cb)
bool is_destroyed;
int ret = 0;

if (!cb->args[IPSET_CB_DUMP]) {
ret = dump_init(cb, inst);
if (ret < 0) {
nlh = nlmsg_hdr(cb->skb);
/* We have to create and send the error message
* manually :-(
*/
if (nlh->nlmsg_flags & NLM_F_ACK)
netlink_ack(cb->skb, nlh, ret, NULL);
return ret;
}
}
if (!cb->args[IPSET_CB_DUMP])
return -EINVAL;

if (cb->args[IPSET_CB_INDEX] >= inst->ip_set_max)
goto out;
Expand Down Expand Up @@ -1684,7 +1684,8 @@ static int ip_set_dump(struct net *net, struct sock *ctnl, struct sk_buff *skb,

{
struct netlink_dump_control c = {
.dump = ip_set_dump_start,
.start = ip_set_dump_start,
.dump = ip_set_dump_do,
.done = ip_set_dump_done,
};
return netlink_dump_start(ctnl, skb, nlh, &c);
Expand Down

0 comments on commit 5038517

Please sign in to comment.