Re: [PATCH] netlink: add policy attribute range validation

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2018-09-26 at 22:17 +0200, Johannes Berg wrote:
> On Wed, 2018-09-26 at 22:06 +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > Without further bloating the policy structs, we can overload
> > the `validation_data' pointer with a struct of s16 min, max
> > and use those to validate ranges in NLA_{U,S}{8,16,32,64}
> > attributes.
> > 
> > It may sound strange to validate NLA_U32 with a s16 max, but
> > in many cases NLA_U32 is used for enums etc. since there's no
> > size benefit in using a smaller attribute width anyway, due
> > to netlink attribute alignment; in cases like that it's still
> > useful, particularly when the attribute really transports an
> > enum value.
> 
> That said, I did find a few places where we could benefit from a larger
> type here - e.g. having a NLA_U16 that must be non-zero cannot be
> represented in the policy as is, since you can't set max to 65535.

We could also fix that, btw, by taking two bits out of the "type" field,
and letting those indicate "check_min" and "check_max". That would also
fix the other thing I noted regarding the union, I suppose.

I didn't really like that too much because it makes the whole thing far
more complex, but perhaps if we hide it behind macros like

#define NLA_POLICY_RANGE(tp, _min, _max) {
	.type = tp,
	.min = _min, .check_min = 1,
	.max = _max, .check_max = 1,
}

#define NLA_POLICY_MIN(tp, _min) {
	.type = tp,
	.min = _min, .check_min = 1,
}

#define NLA_POLICY_MAX(tp, _max) {
	.type = tp,
	.max = _max, .check_max = 1,
}

it becomes more palatable?

johannes