Re: [PATCH] netfilter: nft_set_pipapo: fix stack out-of-bounds read in pipapo_drop()

[Florian Westphal <fw@strlen.de>]

Jenny Guanni Qu <qguanni@gmail.com> wrote:
> pipapo_drop() passes rulemap[i + 1].n to pipapo_unmap() as the
> to_offset argument on every iteration, including the last one where
> i == m->field_count - 1. This reads one element past the end of the
> stack-allocated rulemap array (declared as rulemap[NFT_PIPAPO_MAX_FIELDS]
> with NFT_PIPAPO_MAX_FIELDS == 16).

Thanks, patch looks correct to me.

> diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
> index 7ef4b44471d3..9fb83fc05848 100644
> --- a/net/netfilter/nft_set_pipapo.c
> +++ b/net/netfilter/nft_set_pipapo.c
> @@ -1659,7 +1659,8 @@ static void pipapo_drop(struct nft_pipapo_match *m,
>  		}
>  
>  		pipapo_unmap(f->mt, f->rules, rulemap[i].to, rulemap[i].n,
> -			     rulemap[i + 1].n, i == m->field_count - 1);
> +			     i == m->field_count - 1 ? 0 : rulemap[i + 1].n,
> +			     i == m->field_count - 1);

Small nit, could you add

	bool last = i == m->field_count - 1;

and then use 'last ? 0 : ..., last) ?

This idiom is used elsewhere in the file as well and I think it makes
this sligthly more readable.

Thanks!