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]

Date:	Tue, 19 Apr 2016 13:48:16 +0300
From:	Emmanuel Grumbach <egrumbach@...il.com>
To:	Johannes Berg <johannes@...solutions.net>
Cc:	David Miller <davem@...emloft.net>,
	Eric Dumazet <eric.dumazet@...il.com>,
	roopa@...ulusnetworks.com,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	jhs@...atatu.com, tgraf@...g.ch, nicolas.dichtel@...nd.com
Subject: Re: [PATCH net-next v5] rtnetlink: add new RTM_GETSTATS message to
 dump link stats

On Tue, Apr 19, 2016 at 1:09 PM, Johannes Berg
<johannes@...solutions.net> wrote:
> On Mon, 2016-04-18 at 23:52 -0400, David Miller wrote:
>> From: David Miller <davem@...emloft.net>
>> Date: Mon, 18 Apr 2016 21:48:51 -0400 (EDT)
>>
>> > I think the time has probably come to have a new netlink attribute
>> > format that doesn't have this multi-decade old problem.
>>  ...
>>
>> Scratch this whole idea, I think the padding attribute works a lot
>> better and is backwards compatible with every properly written
>> netlink application.
>
> This talk about attribute flags reminded me about something else.
>
> At netconf, we talked about how awkward it can be that one doesn't know
> if an attribute was accepted by the kernel or simply ignored because
> it's not supported (older kernel version).
>
> I considered (and Emmanuel has started to cook up a patch for it)
> adding a flag here meaning "reject if not parsed" (or so).
>
> Now, I realize that with all the existing netlink commands one can't
> actually rely on that (since it requires some infrastructure), but new
> commands in existing families and also entirely new generic netlink
> families would allow let userspace rely on such a thing.
>
> Thoughts?


FWIW:

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 2a5dbcc..2dfd46d 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -663,6 +663,15 @@ static inline int nla_type(const struct nlattr *nla)
 }

 /**
+ * nla_must_parse - returns true if the attribute must be parsed
+ * @nla: netlink attribute
+ */
+static inline bool nla_must_parse(const struct nlattr *nla)
+{
+       return nla->nla_type & NLA_F_NET_MUST_PARSE;
+}
+
+/**
  * nla_data - head of payload
  * @nla: netlink attribute
  */
diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index 1a85940..e81a8d4 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -165,17 +165,21 @@ struct nlattr {

 /*
  * nla_type (16 bits)
- * +---+---+-------------------------------+
- * | N | O | Attribute Type                |
- * +---+---+-------------------------------+
+ * +---+---+---+----------------------------+
+ * | N | O | M | Attribute Type             |
+ * +---+---+---+----------------------------+
  * N := Carries nested attributes
  * O := Payload stored in network byte order
+ * M := Attribute can't be ignored
  *
  * Note: The N and O flag are mutually exclusive.
  */
 #define NLA_F_NESTED           (1 << 15)
 #define NLA_F_NET_BYTEORDER    (1 << 14)
-#define NLA_TYPE_MASK          ~(NLA_F_NESTED | NLA_F_NET_BYTEORDER)
+#define NLA_F_NET_MUST_PARSE   (1 << 13)
+#define NLA_TYPE_MASK          ~(NLA_F_NESTED |        \
+                                 NLA_F_NET_BYTEORDER | \
+                                 NLA_F_NET_MUST_PARSE)

 #define NLA_ALIGNTO            4
 #define NLA_ALIGN(len)         (((len) + NLA_ALIGNTO - 1) & ~(NLA_ALIGNTO - 1))
diff --git a/lib/nlattr.c b/lib/nlattr.c
index f5907d2..fa15e6f 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -198,6 +198,10 @@ int nla_parse(struct nlattr **tb, int maxtype,
const struct nlattr *head,
                        }

                        tb[type] = (struct nlattr *)nla;
+               } else if (nla_must_parse(nla)) {
+                       err = -EINVAL;
+                       goto errout;
                }
        }