Re: [PATCH v2] macvlan: validate flags

From: John Fastabend
Date: Thu Aug 01 2013 - 13:24:25 EST

On 8/1/2013 9:09 AM, Michael S. Tsirkin wrote:
commit df8ef8f3aaa6692970a436204c4429210addb23a
macvlan: add FDB bridge ops and macvlan flags
added a flags field to macvlan, which can be
controlled from userspace.
The idea is to make the interface future-proof
so we can add flags and not new fields.

However, flags value isn't validated, as a result,
userspace can't detect which flags are supported.

Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
Cc: John Fastabend <john.r.fastabend@xxxxxxxxx>
Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>

Changes from v1:
tweaked commit message
no code changes

Please consider this patch for -stable.

The idea is by the time we add more flags,
everyone has updated to a kernel that
detects errors, so userspace will be able
to detect supported flags cleanly.

Agreed and because we haven't added more flags yet this shouldn't
break uapi. Thanks for catching this.

drivers/net/macvlan.c | 7 +++++++
1 file changed, 7 insertions(+)

By the same logic should we also add the check to macvlan_changelink()?

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 18373b6..8445a94 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -736,6 +736,10 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])

+ if (data && data[IFLA_MACVLAN_FLAGS] &&
+ return -EINVAL;
if (data && data[IFLA_MACVLAN_MODE]) {
switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
@@ -809,6 +813,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
if (data && data[IFLA_MACVLAN_FLAGS])
vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);

+ if (vlan->flags & ~MACVLAN_FLAG_NOPROMISC)
+ return -EINVAL;

Is there really a case where newlink is called without first calling
validate? I don't think there is so the snippet here in newlink could
be dropped.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at