* [PATCH] ip/link_vti6.c: Fix local/remote any handling
@ 2017-08-07 6:41 Christian Langrock
2017-08-07 15:12 ` Stephen Hemminger
0 siblings, 1 reply; 3+ messages in thread
From: Christian Langrock @ 2017-08-07 6:41 UTC (permalink / raw)
To: netdev
[-- Attachment #1.1.1: Type: text/plain, Size: 2034 bytes --]
According to the IPv4 behavior of 'ip' it should be possible to omit the
arguments for local and remote address.
Without this patch omitting these parameters would lead to
uninitialized memory being interpreted as IPv6 addresses.
Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
---
ip/link_vti6.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index be4e33c..220b7df 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -60,7 +60,9 @@ static int vti6_parse_opt(struct link_util *lu, int
argc, char **argv,
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
struct in6_addr saddr;
+ bool use_saddr = false;
struct in6_addr daddr;
+ bool use_daddr = false;
unsigned int ikey = 0;
unsigned int okey = 0;
unsigned int link = 0;
@@ -167,6 +169,7 @@ get_failed:
get_prefix(&addr, *argv, AF_INET6);
memcpy(&daddr, addr.data, addr.bytelen);
+ use_daddr = true;
}
} else if (!matches(*argv, "local")) {
NEXT_ARG();
@@ -178,6 +181,7 @@ get_failed:
get_prefix(&addr, *argv, AF_INET6);
memcpy(&saddr, addr.data, addr.bytelen);
+ use_saddr = true;
}
} else if (!matches(*argv, "dev")) {
NEXT_ARG();
@@ -195,8 +199,10 @@ get_failed:
addattr32(n, 1024, IFLA_VTI_IKEY, ikey);
addattr32(n, 1024, IFLA_VTI_OKEY, okey);
- addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
- addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
+ if (use_saddr)
+ addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
+ if (use_daddr)
+ addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
addattr32(n, 1024, IFLA_VTI_FWMARK, fwmark);
if (link)
addattr32(n, 1024, IFLA_VTI_LINK, link);
--
2.7.4
[-- Attachment #1.1.2: 0x82EB6B5E.asc --]
[-- Type: application/pgp-keys, Size: 1758 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ip/link_vti6.c: Fix local/remote any handling
2017-08-07 6:41 [PATCH] ip/link_vti6.c: Fix local/remote any handling Christian Langrock
@ 2017-08-07 15:12 ` Stephen Hemminger
2017-08-08 7:53 ` Christian Langrock
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Hemminger @ 2017-08-07 15:12 UTC (permalink / raw)
To: Christian Langrock; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]
On Mon, 7 Aug 2017 08:41:23 +0200
Christian Langrock <christian.langrock@secunet.com> wrote:
> According to the IPv4 behavior of 'ip' it should be possible to omit the
> arguments for local and remote address.
> Without this patch omitting these parameters would lead to
> uninitialized memory being interpreted as IPv6 addresses.
>
> Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
I don't like extra flag values. Why not just:
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index be4e33cee606..6ea1fc2306ce 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -59,8 +59,8 @@ static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
struct rtattr *tb[IFLA_MAX + 1];
struct rtattr *linkinfo[IFLA_INFO_MAX+1];
struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
- struct in6_addr saddr;
- struct in6_addr daddr;
+ struct in6_addr saddr = IN6ADDR_ANY_INIT;
+ struct in6_addr daddr = IN6ADDR_ANY_INIT;
unsigned int ikey = 0;
unsigned int okey = 0;
unsigned int link = 0;
@@ -195,8 +195,11 @@ get_failed:
addattr32(n, 1024, IFLA_VTI_IKEY, ikey);
addattr32(n, 1024, IFLA_VTI_OKEY, okey);
- addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
- addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
+
+ if (memcmp(&saddr, &in6addr_any, sizeof(in6addr_any)))
+ addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
+ if (memcmp(&daddr, &in6addr_any, sizeof(in6addr_any)))
+ addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
addattr32(n, 1024, IFLA_VTI_FWMARK, fwmark);
if (link)
addattr32(n, 1024, IFLA_VTI_LINK, link);
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ip/link_vti6.c: Fix local/remote any handling
2017-08-07 15:12 ` Stephen Hemminger
@ 2017-08-08 7:53 ` Christian Langrock
0 siblings, 0 replies; 3+ messages in thread
From: Christian Langrock @ 2017-08-08 7:53 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
[-- Attachment #1.1.1: Type: text/plain, Size: 1908 bytes --]
This seems to be the better solution, at least because i missed the case
where the function is called to alter a VTI.
BR,
Christian
Am 07.08.2017 um 17:12 schrieb Stephen Hemminger:
> On Mon, 7 Aug 2017 08:41:23 +0200
> Christian Langrock <christian.langrock@secunet.com> wrote:
>
>> According to the IPv4 behavior of 'ip' it should be possible to omit the
>> arguments for local and remote address.
>> Without this patch omitting these parameters would lead to
>> uninitialized memory being interpreted as IPv6 addresses.
>>
>> Signed-off-by: Christian Langrock <christian.langrock@secunet.com>
> I don't like extra flag values. Why not just:
>
> diff --git a/ip/link_vti6.c b/ip/link_vti6.c
> index be4e33cee606..6ea1fc2306ce 100644
> --- a/ip/link_vti6.c
> +++ b/ip/link_vti6.c
> @@ -59,8 +59,8 @@ static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
> struct rtattr *tb[IFLA_MAX + 1];
> struct rtattr *linkinfo[IFLA_INFO_MAX+1];
> struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
> - struct in6_addr saddr;
> - struct in6_addr daddr;
> + struct in6_addr saddr = IN6ADDR_ANY_INIT;
> + struct in6_addr daddr = IN6ADDR_ANY_INIT;
> unsigned int ikey = 0;
> unsigned int okey = 0;
> unsigned int link = 0;
> @@ -195,8 +195,11 @@ get_failed:
>
> addattr32(n, 1024, IFLA_VTI_IKEY, ikey);
> addattr32(n, 1024, IFLA_VTI_OKEY, okey);
> - addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
> - addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
> +
> + if (memcmp(&saddr, &in6addr_any, sizeof(in6addr_any)))
> + addattr_l(n, 1024, IFLA_VTI_LOCAL, &saddr, sizeof(saddr));
> + if (memcmp(&daddr, &in6addr_any, sizeof(in6addr_any)))
> + addattr_l(n, 1024, IFLA_VTI_REMOTE, &daddr, sizeof(daddr));
> addattr32(n, 1024, IFLA_VTI_FWMARK, fwmark);
> if (link)
> addattr32(n, 1024, IFLA_VTI_LINK, link);
[-- Attachment #1.1.2: 0x82EB6B5E.asc --]
[-- Type: application/pgp-keys, Size: 1758 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-08 7:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-07 6:41 [PATCH] ip/link_vti6.c: Fix local/remote any handling Christian Langrock
2017-08-07 15:12 ` Stephen Hemminger
2017-08-08 7:53 ` Christian Langrock
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).