Hi,
On Wed, May 18, 2022 at 12:08:42PM +0200, Pablo Neira Ayuso wrote:
> Either userspace or kernelspace need to pre-fetch keys inconditionally
> before comparisons for this to work. Otherwise, register tracking data
> is misleading and it might result in reducing expressions which are not
> yet registers.
>
> First expression is guaranteed to be evaluated always, therefore, keep
> tracking registers and restrict reduction to first expression.
>
> Fixes: b2d306542ff9 ("netfilter: nf_tables: do not reduce read-only expressions")
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> @Phil, you mentioned about a way to simplify this patch, I don't see how,
> just let me know.
Not a big one. Instead of:
| if (nft_expr_reduce(&track, expr)) {
| if (reduce) {
| reduce = false;
| expr = track.cur;
| continue;
| }
| } else if (reduce) {
| reduce = false;
| }
One could do:
| if (nft_expr_reduce(&track, expr) && reduce) {
| reduce = false;
| expr = track.cur;
| continue;
| }
| reduce = false;
Regarding later pre-fetching, one should distinguish between expressions
that (may) set verdict register and those that don't. There are pitfalls
though, e.g. error conditions handled that way.
Maybe introduce a new nft_expr_type field and set reduce like so:
| reduce = reduce && expr->ops->type->reduce;
Cheers, Phil
