Re: [PATCH v6 -next 2/4] netfilter: nftables: add connlabel set support

[Patrick McHardy <kaber@trash.net>]

On 25.04, Florian Westphal wrote:
> Patrick McHardy <kaber@trash.net> wrote:
> 
> > The alternative to internally handling it would be to some propagating
> > validation to immediates / sets which invoke the actual user of the data.
> > So in the case of helpers, we could replace the name by references to
> > the helper structures and reverse this during dumping.
> > 
> > Regarding connlabels this doesn't really apply though. We expect userspace
> > to create a reasonable ruleset and anything that does not cause critical
> > errors is validated in userspace.
> 
> Yes.  So we have three choices here (pseudo-code)
> 
> memcpy(ct->labels, regs->data[priv->sreg], sizeof(reg));
> vs.
> set_bit(priv->imm, ct->labels);
> 
> The latter is what the iptables module does, I do not mind if we
> go for #1 (treat the label area just like an 128bit register and
> replace it completely with whatever is in the source register).
> 
> My only problem is that Pablo suggested #2 whereas you recommend #1.
> 
> I don't want to resubmit until there is consensus as to what the
> preferred solution is.
> 
> We could go for a 3rd alternative, namely:
> 
> u16 bit = regs->data[priv->sreg];
> set_bit(bit, ct->labels);
> 
> i.e. have userspace place the _bit_ that we want to set in the
> source register.
> 
> If we go for sreg that would be my favored solution.
> 
> The only drawback vs #1 is that get and set work differently
> (get places all labels into dreg, set expects bit to set).

That seems like a problem. I agree that #3 would generally be fine, but
we should also really have "ct labels set ct labels" not change the labels,
that would be highly counterintuitive.