netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / 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 related	[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, other threads:[~2020-05-11 13:31 UTC | newest]

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).