On Wed, Sep 26, 2018 at 10:35:27PM +0200, Johannes Berg wrote:
> 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@xxxxxxxxx>
> > >
> > > 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?
The overloading still feels a bit complicated. Perhaps we could rather
use validation_data in the natural way, i.e. as a pointer to validation
data. That would be a struct (maybe array) of two values of the
corresponding type. It would mean a bit more data and a bit more writing
but it would be IMHO more straightforward.
Michal Kubecek
