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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 26 Sep 2018 22:35:27 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     linux-wireless@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH] netlink: add policy attribute range validation

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@...el.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