Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp5614417imu; Mon, 26 Nov 2018 03:02:19 -0800 (PST) X-Google-Smtp-Source: AFSGD/VB3/CzUqxXlV20fxUhzfVPqMQ78zlG+FEOnGyxLAkPDkvxnShzCaKZTSOXDXPuyxcscjuY X-Received: by 2002:a17:902:c05:: with SMTP id 5mr18878709pls.155.1543230139533; Mon, 26 Nov 2018 03:02:19 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1543230139; cv=none; d=google.com; s=arc-20160816; b=FILGiSZxEjovQxy2QWhn7vuuV+lp19GR3b8b8mpVX8fTmpE0h9Q0URa7nSc98tIQRy 1G3Jy6MI5Q9OlMDymmSxQpyielzJogR99YIdALJdDH5gR3I7kfDDCYszpGrYiUXPYbGi uF3UyhEnWNjcKSMkkodpkEYiLdQR5EpJ3PIiE+EVboz0K8ApFqaMJLmhhPx0gECi1Q/K 3YasvQWD3i6zPi4zWbBdMeCaTJ8c9TuW0hL4l9rbVIAm6GscUHNj0lds8PYRUZj4pnHx rqXtomBug5BHVr0re1fTSBTR3XqiSotiBvCTcyJ8IRYWR1hNwRdW+pzh0mhNwuu6CNol BUmQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=ZBoReBsxCKkBBRo2qV+sQO0E/dYjenvbG3qeSSZU08k=; b=ML6LDyO6e/Y1QwfKXftvPmtWekNnV1kVn0K2FrROrsQQroNutRaGuIN+x8qUR5Bs1l A9cHB+BpP8tF91co46528HZX7IFR89WyC68q3lDHtMKke4o1hXlv2eYEjpvajsWD+DnL x/dhzCIHg3eWmX+P+Vfd2Pashy1FHzT8dADstEVo+2hecnPI72KXN+5J3KtPAxcKOy1a 0AHjIDknv6G1gHl4aHfQ6gS23CxCPQtZQXaaOh5f5vrMVwIQmIsMp8wKpBQb6tx1SakD oikEY6CBDkuIUxCTVDAQ2ipryN487s709gQWaUuYwMVgE3prlRA8gEQEU7fv7WsRwO0e XZUw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=OXUm6QU9; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id a23si30345593plm.334.2018.11.26.03.02.00; Mon, 26 Nov 2018 03:02:19 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=OXUm6QU9; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729541AbeKZVxN (ORCPT + 99 others); Mon, 26 Nov 2018 16:53:13 -0500 Received: from mail.kernel.org ([198.145.29.99]:34890 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727875AbeKZVxN (ORCPT ); Mon, 26 Nov 2018 16:53:13 -0500 Received: from localhost (5356596B.cm-6-7b.dynamic.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 2E80420817; Mon, 26 Nov 2018 10:59:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1543229967; bh=SrF/1Pfky7jYe5NIUzTFYvZOS8g7FhMMHX/jQtVMjfg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OXUm6QU9Sh8qwY71iNl4sT0gJR6fZ8Pk/50T+Nnvz6thbOklXA0gOm/hd7/0d7K4D j9eE/FDpWCzJ/fKCSjoMpV9+juV6umXun894Wh1JBAO2S2UUfwyubrqOltBu3u/Scj EYh/crGowtmNQOOmWHHK4TtE4itvuc4o/I64AZjw= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Li Shuang , Stefano Brivio , Jozsef Kadlecsik , Pablo Neira Ayuso , Sasha Levin Subject: [PATCH 4.14 21/62] netfilter: ipset: list:set: Decrease refcount synchronously on deletion and replace Date: Mon, 26 Nov 2018 11:51:02 +0100 Message-Id: <20181126105052.590398742@linuxfoundation.org> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20181126105050.592727680@linuxfoundation.org> References: <20181126105050.592727680@linuxfoundation.org> User-Agent: quilt/0.65 X-stable: review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.14-stable review patch. If anyone has any objections, please let me know. ------------------ [ Upstream commit 439cd39ea136d2c026805264d58a91f36b6b64ca ] Commit 45040978c899 ("netfilter: ipset: Fix set:list type crash when flush/dump set in parallel") postponed decreasing set reference counters to the RCU callback. An 'ipset del' command can terminate before the RCU grace period is elapsed, and if sets are listed before then, the reference counter shown in userspace will be wrong: # ipset create h hash:ip; ipset create l list:set; ipset add l # ipset del l h; ipset list h Name: h Type: hash:ip Revision: 4 Header: family inet hashsize 1024 maxelem 65536 Size in memory: 88 References: 1 Number of entries: 0 Members: # sleep 1; ipset list h Name: h Type: hash:ip Revision: 4 Header: family inet hashsize 1024 maxelem 65536 Size in memory: 88 References: 0 Number of entries: 0 Members: Fix this by making the reference count update synchronous again. As a result, when sets are listed, ip_set_name_byindex() might now fetch a set whose reference count is already zero. Instead of relying on the reference count to protect against concurrent set renaming, grab ip_set_ref_lock as reader and copy the name, while holding the same lock in ip_set_rename() as writer instead. Reported-by: Li Shuang Fixes: 45040978c899 ("netfilter: ipset: Fix set:list type crash when flush/dump set in parallel") Signed-off-by: Stefano Brivio Signed-off-by: Jozsef Kadlecsik Signed-off-by: Pablo Neira Ayuso Signed-off-by: Sasha Levin --- include/linux/netfilter/ipset/ip_set.h | 2 +- net/netfilter/ipset/ip_set_core.c | 23 +++++++++++------------ net/netfilter/ipset/ip_set_list_set.c | 17 +++++++++++------ 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h index 8e42253e5d4d..91a533bd3eb1 100644 --- a/include/linux/netfilter/ipset/ip_set.h +++ b/include/linux/netfilter/ipset/ip_set.h @@ -312,7 +312,7 @@ enum { extern ip_set_id_t ip_set_get_byname(struct net *net, const char *name, struct ip_set **set); extern void ip_set_put_byindex(struct net *net, ip_set_id_t index); -extern const char *ip_set_name_byindex(struct net *net, ip_set_id_t index); +extern void ip_set_name_byindex(struct net *net, ip_set_id_t index, char *name); extern ip_set_id_t ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t index); extern void ip_set_nfnl_put(struct net *net, ip_set_id_t index); diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c index 9d2ce1459cec..a3f1dc7cf538 100644 --- a/net/netfilter/ipset/ip_set_core.c +++ b/net/netfilter/ipset/ip_set_core.c @@ -668,21 +668,20 @@ ip_set_put_byindex(struct net *net, ip_set_id_t index) EXPORT_SYMBOL_GPL(ip_set_put_byindex); /* Get the name of a set behind a set index. - * We assume the set is referenced, so it does exist and - * can't be destroyed. The set cannot be renamed due to - * the referencing either. - * + * Set itself is protected by RCU, but its name isn't: to protect against + * renaming, grab ip_set_ref_lock as reader (see ip_set_rename()) and copy the + * name. */ -const char * -ip_set_name_byindex(struct net *net, ip_set_id_t index) +void +ip_set_name_byindex(struct net *net, ip_set_id_t index, char *name) { - const struct ip_set *set = ip_set_rcu_get(net, index); + struct ip_set *set = ip_set_rcu_get(net, index); BUG_ON(!set); - BUG_ON(set->ref == 0); - /* Referenced, so it's safe */ - return set->name; + read_lock_bh(&ip_set_ref_lock); + strncpy(name, set->name, IPSET_MAXNAMELEN); + read_unlock_bh(&ip_set_ref_lock); } EXPORT_SYMBOL_GPL(ip_set_name_byindex); @@ -1128,7 +1127,7 @@ static int ip_set_rename(struct net *net, struct sock *ctnl, if (!set) return -ENOENT; - read_lock_bh(&ip_set_ref_lock); + write_lock_bh(&ip_set_ref_lock); if (set->ref != 0) { ret = -IPSET_ERR_REFERENCED; goto out; @@ -1145,7 +1144,7 @@ static int ip_set_rename(struct net *net, struct sock *ctnl, strncpy(set->name, name2, IPSET_MAXNAMELEN); out: - read_unlock_bh(&ip_set_ref_lock); + write_unlock_bh(&ip_set_ref_lock); return ret; } diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c index 178d4eba013b..75d52aed6fdb 100644 --- a/net/netfilter/ipset/ip_set_list_set.c +++ b/net/netfilter/ipset/ip_set_list_set.c @@ -156,9 +156,7 @@ __list_set_del_rcu(struct rcu_head * rcu) { struct set_elem *e = container_of(rcu, struct set_elem, rcu); struct ip_set *set = e->set; - struct list_set *map = set->data; - ip_set_put_byindex(map->net, e->id); ip_set_ext_destroy(set, e); kfree(e); } @@ -166,15 +164,21 @@ __list_set_del_rcu(struct rcu_head * rcu) static inline void list_set_del(struct ip_set *set, struct set_elem *e) { + struct list_set *map = set->data; + set->elements--; list_del_rcu(&e->list); + ip_set_put_byindex(map->net, e->id); call_rcu(&e->rcu, __list_set_del_rcu); } static inline void -list_set_replace(struct set_elem *e, struct set_elem *old) +list_set_replace(struct ip_set *set, struct set_elem *e, struct set_elem *old) { + struct list_set *map = set->data; + list_replace_rcu(&old->list, &e->list); + ip_set_put_byindex(map->net, old->id); call_rcu(&old->rcu, __list_set_del_rcu); } @@ -306,7 +310,7 @@ list_set_uadd(struct ip_set *set, void *value, const struct ip_set_ext *ext, INIT_LIST_HEAD(&e->list); list_set_init_extensions(set, ext, e); if (n) - list_set_replace(e, n); + list_set_replace(set, e, n); else if (next) list_add_tail_rcu(&e->list, &next->list); else if (prev) @@ -497,6 +501,7 @@ list_set_list(const struct ip_set *set, const struct list_set *map = set->data; struct nlattr *atd, *nested; u32 i = 0, first = cb->args[IPSET_CB_ARG0]; + char name[IPSET_MAXNAMELEN]; struct set_elem *e; int ret = 0; @@ -515,8 +520,8 @@ list_set_list(const struct ip_set *set, nested = ipset_nest_start(skb, IPSET_ATTR_DATA); if (!nested) goto nla_put_failure; - if (nla_put_string(skb, IPSET_ATTR_NAME, - ip_set_name_byindex(map->net, e->id))) + ip_set_name_byindex(map->net, e->id, name); + if (nla_put_string(skb, IPSET_ATTR_NAME, name)) goto nla_put_failure; if (ip_set_put_extensions(skb, set, e, true)) goto nla_put_failure; -- 2.17.1