Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [nf PATCH] netfilter: nft_set_rbtree: Add missing expired checks
@ 2020-05-06 11:11 Phil Sutter
  2020-05-10 22:03 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 3+ messages in thread
From: Phil Sutter @ 2020-05-06 11:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Expired intervals would still match and be dumped to user space until
garbage collection wiped them out. Make sure they stop matching and
disappear (from users' perspective) as soon as they expire.

Fixes: 8d8540c4f5e03 ("netfilter: nft_set_rbtree: add timeout support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/netfilter/nft_set_rbtree.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 3ffef454d4699..8efcea03a4cbb 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -75,7 +75,8 @@ static bool __nft_rbtree_lookup(const struct net *net, const struct nft_set *set
 		} else if (d > 0)
 			parent = rcu_dereference_raw(parent->rb_right);
 		else {
-			if (!nft_set_elem_active(&rbe->ext, genmask)) {
+			if (!nft_set_elem_active(&rbe->ext, genmask) ||
+			    nft_set_elem_expired(&rbe->ext)) {
 				parent = rcu_dereference_raw(parent->rb_left);
 				continue;
 			}
@@ -94,6 +95,7 @@ static bool __nft_rbtree_lookup(const struct net *net, const struct nft_set *set
 
 	if (set->flags & NFT_SET_INTERVAL && interval != NULL &&
 	    nft_set_elem_active(&interval->ext, genmask) &&
+	    !nft_set_elem_expired(&interval->ext) &&
 	    nft_rbtree_interval_start(interval)) {
 		*ext = &interval->ext;
 		return true;
@@ -149,7 +151,8 @@ static bool __nft_rbtree_get(const struct net *net, const struct nft_set *set,
 			if (flags & NFT_SET_ELEM_INTERVAL_END)
 				interval = rbe;
 		} else {
-			if (!nft_set_elem_active(&rbe->ext, genmask)) {
+			if (!nft_set_elem_active(&rbe->ext, genmask) ||
+			    nft_set_elem_expired(&rbe->ext)) {
 				parent = rcu_dereference_raw(parent->rb_left);
 				continue;
 			}
@@ -170,6 +173,7 @@ static bool __nft_rbtree_get(const struct net *net, const struct nft_set *set,
 
 	if (set->flags & NFT_SET_INTERVAL && interval != NULL &&
 	    nft_set_elem_active(&interval->ext, genmask) &&
+	    !nft_set_elem_expired(&interval->ext) &&
 	    ((!nft_rbtree_interval_end(interval) &&
 	      !(flags & NFT_SET_ELEM_INTERVAL_END)) ||
 	     (nft_rbtree_interval_end(interval) &&
@@ -418,6 +422,8 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
 
 		if (iter->count < iter->skip)
 			goto cont;
+		if (nft_set_elem_expired(&rbe->ext))
+			goto cont;
 		if (!nft_set_elem_active(&rbe->ext, iter->genmask))
 			goto cont;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [nf PATCH] netfilter: nft_set_rbtree: Add missing expired checks
  2020-05-06 11:11 [nf PATCH] netfilter: nft_set_rbtree: Add missing expired checks Phil Sutter
@ 2020-05-10 22:03 ` Pablo Neira Ayuso
  2020-05-11 13:31   ` Phil Sutter
  0 siblings, 1 reply; 3+ messages in thread
From: Pablo Neira Ayuso @ 2020-05-10 22:03 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel

Hi Phil,

On Wed, May 06, 2020 at 01:11:07PM +0200, Phil Sutter wrote:
> Expired intervals would still match and be dumped to user space until
> garbage collection wiped them out. Make sure they stop matching and
> disappear (from users' perspective) as soon as they expire.
> 
> Fixes: 8d8540c4f5e03 ("netfilter: nft_set_rbtree: add timeout support")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  net/netfilter/nft_set_rbtree.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
> index 3ffef454d4699..8efcea03a4cbb 100644
> --- a/net/netfilter/nft_set_rbtree.c
> +++ b/net/netfilter/nft_set_rbtree.c
> @@ -75,7 +75,8 @@ static bool __nft_rbtree_lookup(const struct net *net, const struct nft_set *set
>  		} else if (d > 0)
>  			parent = rcu_dereference_raw(parent->rb_right);
>  		else {
> -			if (!nft_set_elem_active(&rbe->ext, genmask)) {
> +			if (!nft_set_elem_active(&rbe->ext, genmask) ||
> +			    nft_set_elem_expired(&rbe->ext)) {

It seems _insert() does not allow for duplicates. I think it's better
if you just:

        return false;

in case in case the element has expired, right?

Same thing in _get.

Thanks.

>  				parent = rcu_dereference_raw(parent->rb_left);
>  				continue;
>  			}
> @@ -94,6 +95,7 @@ static bool __nft_rbtree_lookup(const struct net *net, const struct nft_set *set
>  
>  	if (set->flags & NFT_SET_INTERVAL && interval != NULL &&
>  	    nft_set_elem_active(&interval->ext, genmask) &&
> +	    !nft_set_elem_expired(&interval->ext) &&
>  	    nft_rbtree_interval_start(interval)) {
>  		*ext = &interval->ext;
>  		return true;
> @@ -149,7 +151,8 @@ static bool __nft_rbtree_get(const struct net *net, const struct nft_set *set,
>  			if (flags & NFT_SET_ELEM_INTERVAL_END)
>  				interval = rbe;
>  		} else {
> -			if (!nft_set_elem_active(&rbe->ext, genmask)) {
> +			if (!nft_set_elem_active(&rbe->ext, genmask) ||
> +			    nft_set_elem_expired(&rbe->ext)) {
>  				parent = rcu_dereference_raw(parent->rb_left);
>  				continue;
>  			}
> @@ -170,6 +173,7 @@ static bool __nft_rbtree_get(const struct net *net, const struct nft_set *set,
>  
>  	if (set->flags & NFT_SET_INTERVAL && interval != NULL &&
>  	    nft_set_elem_active(&interval->ext, genmask) &&
> +	    !nft_set_elem_expired(&interval->ext) &&
>  	    ((!nft_rbtree_interval_end(interval) &&
>  	      !(flags & NFT_SET_ELEM_INTERVAL_END)) ||
>  	     (nft_rbtree_interval_end(interval) &&
> @@ -418,6 +422,8 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
>  
>  		if (iter->count < iter->skip)
>  			goto cont;
> +		if (nft_set_elem_expired(&rbe->ext))
> +			goto cont;
>  		if (!nft_set_elem_active(&rbe->ext, iter->genmask))
>  			goto cont;
>  
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [nf PATCH] netfilter: nft_set_rbtree: Add missing expired checks
  2020-05-10 22:03 ` Pablo Neira Ayuso
@ 2020-05-11 13:31   ` Phil Sutter
  0 siblings, 0 replies; 3+ messages in thread
From: Phil Sutter @ 2020-05-11 13:31 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Hi Pablo,

On Mon, May 11, 2020 at 12:03:56AM +0200, Pablo Neira Ayuso wrote:
> On Wed, May 06, 2020 at 01:11:07PM +0200, Phil Sutter wrote:
> > Expired intervals would still match and be dumped to user space until
> > garbage collection wiped them out. Make sure they stop matching and
> > disappear (from users' perspective) as soon as they expire.
> > 
> > Fixes: 8d8540c4f5e03 ("netfilter: nft_set_rbtree: add timeout support")
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  net/netfilter/nft_set_rbtree.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
> > index 3ffef454d4699..8efcea03a4cbb 100644
> > --- a/net/netfilter/nft_set_rbtree.c
> > +++ b/net/netfilter/nft_set_rbtree.c
> > @@ -75,7 +75,8 @@ static bool __nft_rbtree_lookup(const struct net *net, const struct nft_set *set
> >  		} else if (d > 0)
> >  			parent = rcu_dereference_raw(parent->rb_right);
> >  		else {
> > -			if (!nft_set_elem_active(&rbe->ext, genmask)) {
> > +			if (!nft_set_elem_active(&rbe->ext, genmask) ||
> > +			    nft_set_elem_expired(&rbe->ext)) {
> 
> It seems _insert() does not allow for duplicates. I think it's better
> if you just:
> 
>         return false;
> 
> in case in case the element has expired, right?

Ah yes, thanks. I'll send a v2.

Thanks, Phil

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 11:11 [nf PATCH] netfilter: nft_set_rbtree: Add missing expired checks Phil Sutter
2020-05-10 22:03 ` Pablo Neira Ayuso
2020-05-11 13:31   ` Phil Sutter

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git