netdev - Re: [PATCH net-next v5] rtnetlink: add new RTM

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]

Message-ID: <1461158228.2176.18.camel@sipsolutions.net>
Date:	Wed, 20 Apr 2016 15:17:08 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Jiri Benc <jbenc@...hat.com>
Cc:	David Ahern <dsa@...ulusnetworks.com>,
	David Miller <davem@...emloft.net>, eric.dumazet@...il.com,
	roopa@...ulusnetworks.com, netdev@...r.kernel.org,
	jhs@...atatu.com, tgraf@...g.ch, nicolas.dichtel@...nd.com,
	egrumbach@...il.com
Subject: Re: [PATCH net-next v5] rtnetlink: add new RTM_GETSTATS message to
 dump link stats

On Wed, 2016-04-20 at 14:48 +0200, Jiri Benc wrote:
> On Wed, 20 Apr 2016 09:32:20 +0200, Johannes Berg wrote:
> > 
> > 2) Use the new attribute flag with some required attribute for
> >    existing commands, so that older kernel will not find the
> > required
> >    attribute and will reject the operation entirely.
> >    May or may not fall back to trying the operation again without
> > the
> >    flag.
> This is basically what I submitted half a year ago. See:
> http://thread.gmane.org/gmane.linux.network/382850
> 

That looks like a *huge* patchset though - whereas my proposal really
required only what Emmanuel sent in this thread. It did make some
assumptions, for example that any attribute lower than the "maxtype"
argument to nla_parse() was understood. [1]

Looks like you have this on a per-message basis. I thought it was
better on an attribute basis because that's really where the issue is.

You can still detect it with the per-attribute flag approach as I
described in (2) - if, for your lwtunnel example, you could specify the
flag on the RTA_ENCAP attribute, without which no lwtunnel can be
created (if I understand the code correctly.)

johannes

[1] for example, if I have three attributes:
enum attrs {__unused, A, B, C};

and the policy

policy = {
	[A] = { .type = NLA_U32 },
	[C] = { .type = NLA_U8 },
}

and then do

nla_parse(tb, 3, msg, msg_len, &policy)

it would assume that "B" is valid. Since this policy is equivalent to
the policy with
	[B] = { .type = NLA_BINARY }

(minimum length 0) we could also reject anything that has type=len=0 in
the policy, if the NLA_F_NET_MUST_PARSE flag is set in the nla_type.

This would likely be the right approach for most netlink families,
since they usually don't have holes that they actually care about -
I've yet to see any attribute that's not specified at all in the policy
but used anyway, normally you want some level of checking, and indicate
that by using { .type = NLA_BINARY } - but other things are possible.

johannes