Thread (2 messages) 2 messages, 2 authors, 2018-09-27

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

From: Johannes Berg <hidden>
Date: 2018-09-26 20:35:27
Also in: linux-wireless

Possibly related (same subject, not in this thread)

On Wed, 2018-09-26 at 22:17 +0200, Johannes Berg wrote:
On Wed, 2018-09-26 at 22:06 +0200, Johannes Berg wrote:
quoted
From: Johannes Berg <redacted>

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help