| 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
| ||
|
Date: Thu, 14 Feb 2019 12:10:03 +0100
From: Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
To: Petr Machata <petrm@...lanox.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuznet@....inr.ac.ru" <kuznet@....inr.ac.ru>,
"yoshfuji@...ux-ipv6.org" <yoshfuji@...ux-ipv6.org>,
lucien.xin@...il.com
Subject: Re: [PATCH net-next] net: ip6_gre: Give ERSPAN a fill_info link op
of its own
> In commit c706863bc890 ("net: ip6_gre: always reports o_key to
> userspace"), ip6gre and ip6gretap tunnels started reporting a TUNNEL_KEY
> output flag even if one was not configured at the device.
>
> When an okey-less ip6gre or ip6gretap netdevice is created, it initially
> encapsulates the packets without okey. But any configuration change
> (even a non-change such as setting TOS to an already-configured value)
> then causes the okey flag from the reported configuration to be
> circulated back to actual configuration. From that point on, the device
> encapsulates packets with output key of 0.
>
> The intention was to implement this behavior for ERSPAN devices, not for
> all ip6gre devices. The ERSPAN netdevice should really have its own
> fill_info callback. Add one.
Hi Petr,
I was assuming erspan_ver is set just for erspan tunnels. In particular I guess
the issue is due to the default erspan_ver configuration done in
ip6gre_netlink_parms (commit 84581bdae9587).
What about adding a routine to set erspan_ver and moving it in
ip6erspan_newlink/ip6erspan_changelink? In this way erspan_ver will be
defined just for erspan tunnels.
Moreover do we have a similar issue for IFLA_GRE_ERSPAN_INDEX in
ip6gre_fill_info?
Something like:
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 65a4f96dc462..bb525abd860e 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1719,6 +1719,24 @@ static int ip6erspan_tap_validate(struct nlattr *tb[], struct nlattr *data[],
return 0;
}
+static void ip6erspan_set_version(struct nlattr *data[],
+ struct __ip6_tnl_parm *parms)
+{
+ parms->erspan_ver = 1;
+ if (data[IFLA_GRE_ERSPAN_VER])
+ parms->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]);
+
+ if (parms->erspan_ver == 1) {
+ if (data[IFLA_GRE_ERSPAN_INDEX])
+ parms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]);
+ } else if (parms->erspan_ver == 2) {
+ if (data[IFLA_GRE_ERSPAN_DIR])
+ parms->dir = nla_get_u8(data[IFLA_GRE_ERSPAN_DIR]);
+ if (data[IFLA_GRE_ERSPAN_HWID])
+ parms->hwid = nla_get_u16(data[IFLA_GRE_ERSPAN_HWID]);
+ }
+}
+
static void ip6gre_netlink_parms(struct nlattr *data[],
struct __ip6_tnl_parm *parms)
{
@@ -1767,20 +1785,6 @@ static void ip6gre_netlink_parms(struct nlattr *data[],
if (data[IFLA_GRE_COLLECT_METADATA])
parms->collect_md = true;
-
- parms->erspan_ver = 1;
- if (data[IFLA_GRE_ERSPAN_VER])
- parms->erspan_ver = nla_get_u8(data[IFLA_GRE_ERSPAN_VER]);
-
- if (parms->erspan_ver == 1) {
- if (data[IFLA_GRE_ERSPAN_INDEX])
- parms->index = nla_get_u32(data[IFLA_GRE_ERSPAN_INDEX]);
- } else if (parms->erspan_ver == 2) {
- if (data[IFLA_GRE_ERSPAN_DIR])
- parms->dir = nla_get_u8(data[IFLA_GRE_ERSPAN_DIR]);
- if (data[IFLA_GRE_ERSPAN_HWID])
- parms->hwid = nla_get_u16(data[IFLA_GRE_ERSPAN_HWID]);
- }
}
static int ip6gre_tap_init(struct net_device *dev)
@@ -2203,6 +2207,7 @@ static int ip6erspan_newlink(struct net *src_net, struct net_device *dev,
int err;
ip6gre_netlink_parms(data, &nt->parms);
+ ip6erspan_set_version(data, &nt->parms);
ign = net_generic(net, ip6gre_net_id);
if (nt->parms.collect_md) {
@@ -2248,6 +2253,7 @@ static int ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[],
if (IS_ERR(t))
return PTR_ERR(t);
+ ip6erspan_set_version(data, &p);
ip6gre_tunnel_unlink_md(ign, t);
ip6gre_tunnel_unlink(ign, t);
ip6erspan_tnl_change(t, &p, !tb[IFLA_MTU]);
Does it fix reported issue?
Regards,
Lorenzo
>
> Fixes: c706863bc890 ("net: ip6_gre: always reports o_key to userspace")
> CC: Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
> Signed-off-by: Petr Machata <petrm@...lanox.com>
> ---
> net/ipv6/ip6_gre.c | 31 ++++++++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 65a4f96dc462..0a6087cffe54 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -2094,15 +2094,13 @@ static size_t ip6gre_get_size(const struct net_device *dev)
> 0;
> }
>
> -static int ip6gre_fill_info(struct sk_buff *skb, const struct net_device *dev)
> +static int __ip6gre_fill_info(struct sk_buff *skb,
> + const struct net_device *dev,
> + __be16 base_o_flags)
> {
> struct ip6_tnl *t = netdev_priv(dev);
> struct __ip6_tnl_parm *p = &t->parms;
> - __be16 o_flags = p->o_flags;
> -
> - if ((p->erspan_ver == 1 || p->erspan_ver == 2) &&
> - !p->collect_md)
> - o_flags |= TUNNEL_KEY;
> + __be16 o_flags = p->o_flags | base_o_flags;
>
> if (nla_put_u32(skb, IFLA_GRE_LINK, p->link) ||
> nla_put_be16(skb, IFLA_GRE_IFLAGS,
> @@ -2155,6 +2153,11 @@ static int ip6gre_fill_info(struct sk_buff *skb, const struct net_device *dev)
> return -EMSGSIZE;
> }
>
> +static int ip6gre_fill_info(struct sk_buff *skb, const struct net_device *dev)
> +{
> + return __ip6gre_fill_info(skb, dev, 0);
> +}
> +
> static const struct nla_policy ip6gre_policy[IFLA_GRE_MAX + 1] = {
> [IFLA_GRE_LINK] = { .type = NLA_U32 },
> [IFLA_GRE_IFLAGS] = { .type = NLA_U16 },
> @@ -2256,6 +2259,20 @@ static int ip6erspan_changelink(struct net_device *dev, struct nlattr *tb[],
> return 0;
> }
>
> +static int ip6erspan_fill_info(struct sk_buff *skb,
> + const struct net_device *dev)
> +{
> + struct ip6_tnl *t = netdev_priv(dev);
> + struct __ip6_tnl_parm *p = &t->parms;
> + __be16 base_o_flags = 0;
> +
> + if ((p->erspan_ver == 1 || p->erspan_ver == 2) &&
> + !p->collect_md)
> + base_o_flags |= TUNNEL_KEY;
> +
> + return __ip6gre_fill_info(skb, dev, base_o_flags);
> +}
> +
> static struct rtnl_link_ops ip6gre_link_ops __read_mostly = {
> .kind = "ip6gre",
> .maxtype = IFLA_GRE_MAX,
> @@ -2295,7 +2312,7 @@ static struct rtnl_link_ops ip6erspan_tap_ops __read_mostly = {
> .newlink = ip6erspan_newlink,
> .changelink = ip6erspan_changelink,
> .get_size = ip6gre_get_size,
> - .fill_info = ip6gre_fill_info,
> + .fill_info = ip6erspan_fill_info,
> .get_link_net = ip6_tnl_get_link_net,
> };
>
> --
> 2.4.11
>
Powered by blists - more mailing lists