Re: [PATCH nf 0/2] nft_set_pipapo: Fix crash due to dangling entries in mapping table

[Stefano Brivio <sbrivio@xxxxxxxxxx>]

On Wed, 26 Feb 2020 12:29:35 +0100
Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:

> From a17f22eac1dfd599ff97bb262fc97d64333b06fe Mon Sep 17 00:00:00 2001
> From: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> Date: Wed, 26 Feb 2020 12:11:53 +0100
> Subject: [PATCH] netfilter: nf_tables: report ENOTEMPTY on element
>  intersection
> 
> The set backend uses ENOTEMPTY to report an intersection between two
> range elements.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
>  net/netfilter/nf_tables_api.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index d1318bdf49ca..48ad273a273e 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -5059,7 +5059,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>  	ext->genmask = nft_genmask_cur(ctx->net) | NFT_SET_ELEM_BUSY_MASK;
>  	err = set->ops->insert(ctx->net, set, &elem, &ext2);
>  	if (err) {
> -		if (err == -EEXIST) {
> +		if (err == -EEXIST || err == -ENOTEMPTY) {
>  			if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA) ^
>  			    nft_set_ext_exists(ext2, NFT_SET_EXT_DATA) ||
>  			    nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF) ^
> @@ -5073,10 +5073,17 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
>  				    nft_set_ext_data(ext2), set->dlen) != 0) ||
>  			    (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF) &&
>  			     nft_set_ext_exists(ext2, NFT_SET_EXT_OBJREF) &&
> -			     *nft_set_ext_obj(ext) != *nft_set_ext_obj(ext2)))
> +			     *nft_set_ext_obj(ext) != *nft_set_ext_obj(ext2))) {
>  				err = -EBUSY;
> -			else if (!(nlmsg_flags & NLM_F_EXCL))
> -				err = 0;
> +			} else {
> +				/* ENOTEMPTY reports an intersection between
> +				 * this element and an existing one.
> +				 */
> +				if (err == -ENOTEMPTY)
> +					err = -EEXIST;
> +				else if (!(nlmsg_flags & NLM_F_EXCL))
> +					err = 0;
> +			}
>  		}
>  		goto err_element_clash;
>  	}

I haven't tested it, but isn't:

@@ -5077,6 +5077,11 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
                                err = -EBUSY;
                        else if (!(nlmsg_flags & NLM_F_EXCL))
                                err = 0;
+               } else if (err == -ENOTEMPTY) {
+                       /* ENOTEMPTY reports overlapping between this element
+                        * and an existing one.
+                        */
+                       err = -EEXIST;
                }
                goto err_element_clash;
        }

simpler and equivalent?

-- 
Stefano