lkml.org 
[lkml]   [2020]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 024/309] netfilter: ipset: fix suspicious RCU usage in find_set_and_id
    Date
    From: Kadlecsik József <kadlec@blackhole.kfki.hu>

    commit 5038517119d50ed0240059b1d7fc2faa92371c08 upstream.

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    net/netfilter/ipset/ip_set_core.c | 41 +++++++++++++++++++-------------------
    1 file changed, 21 insertions(+), 20 deletions(-)

    --- a/net/netfilter/ipset/ip_set_core.c
    +++ b/net/netfilter/ipset/ip_set_core.c
    @@ -1293,31 +1293,34 @@ ip_set_dump_policy[IPSET_ATTR_CMD_MAX +
    };

    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 {
    @@ -1333,10 +1336,17 @@ dump_init(struct netlink_callback *cb, s
    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;
    @@ -1347,18 +1357,8 @@ ip_set_dump_start(struct sk_buff *skb, s
    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;
    @@ -1494,7 +1494,8 @@ static int ip_set_dump(struct net *net,

    {
    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);

    \
     
     \ /
      Last update: 2020-02-10 14:27    [W:4.157 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site