[PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre
@ 2017-12-20  7:57 Serhey Popovych
  2017-12-20  7:57 ` [PATCH iproute2 1/2] utils: ll_addr: Handle ARPHRD_IP6GRE in ll_addr_n2a() Serhey Popovych
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Serhey Popovych @ 2017-12-20  7:57 UTC (permalink / raw)
  To: netdev

In this series I present next set of improvements/fixes:

  1) If rtnl_talk(), called with answer != NULL fails it
     never returns with allocated answer: no need to
     initialize answer and free() it in the caller on error.

  2) Correct ip6gre print endpoint address printing with
     ll_addr_n2a(): endpoints are IPv6 addresses, not hex
     dump.

See individual patch description message for details.

Thanks,
Serhii

Serhey Popovych (2):
  utils: ll_addr: Handle ARPHRD_IP6GRE in ll_addr_n2a()
  ip/tunnel: No need to free answer after rtnl_talk() on error

 ip/link_gre.c    |    3 +--
 ip/link_gre6.c   |    3 +--
 ip/link_ip6tnl.c |    3 +--
 ip/link_iptnl.c  |    3 +--
 ip/link_vti.c    |    3 +--
 ip/link_vti6.c   |    3 +--
 lib/ll_addr.c    |    3 ++-
 7 files changed, 8 insertions(+), 13 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH iproute2 1/2] utils: ll_addr: Handle ARPHRD_IP6GRE in ll_addr_n2a()
  2017-12-20  7:57 [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre Serhey Popovych
@ 2017-12-20  7:57 ` Serhey Popovych
  2017-12-20  7:57 ` [PATCH iproute2 2/2] ip/tunnel: No need to free answer after rtnl_talk() on error Serhey Popovych
  2017-12-26 17:08 ` [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre Stephen Hemminger
  2 siblings, 0 replies; 4+ messages in thread
From: Serhey Popovych @ 2017-12-20  7:57 UTC (permalink / raw)
  To: netdev

ll_addr_n2a() correctly prints tunnel endpoints for gre, ipip, sit
and ip6tnl, but not for ip6gre. Fix this by adding ARPHRD_IP6GRE to
IPv6 tunnel endpoing address conversion.

Before:
-------

$ ip link show
...
18: ip6tnl0: <NOARP> mtu 1452 qdisc noop state DOWN mode DEFAULT group default
    link/tunnel6 :: brd ::
19: ip6gre0: <NOARP> mtu 1456 qdisc noop state DOWN mode DEFAULT group default
    link/gre6 00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00 brd \
00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00

After:
------

$ ip link show
...
18: ip6tnl0: <NOARP> mtu 1452 qdisc noop state DOWN mode DEFAULT group default
    link/tunnel6 :: brd ::
19: ip6gre0: <NOARP> mtu 1456 qdisc noop state DOWN mode DEFAULT group default
    link/gre6 :: brd ::

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 lib/ll_addr.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/ll_addr.c b/lib/ll_addr.c
index c03523d..84de64e 100644
--- a/lib/ll_addr.c
+++ b/lib/ll_addr.c
@@ -36,7 +36,8 @@ const char *ll_addr_n2a(const unsigned char *addr, int alen, int type, char *buf
 	    (type == ARPHRD_TUNNEL || type == ARPHRD_SIT || type == ARPHRD_IPGRE)) {
 		return inet_ntop(AF_INET, addr, buf, blen);
 	}
-	if (alen == 16 && type == ARPHRD_TUNNEL6) {
+	if (alen == 16 &&
+	    (type == ARPHRD_TUNNEL6 || type == ARPHRD_IP6GRE)) {
 		return inet_ntop(AF_INET6, addr, buf, blen);
 	}
 	snprintf(buf, blen, "%02x", addr[0]);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH iproute2 2/2] ip/tunnel: No need to free answer after rtnl_talk() on error
  2017-12-20  7:57 [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre Serhey Popovych
  2017-12-20  7:57 ` [PATCH iproute2 1/2] utils: ll_addr: Handle ARPHRD_IP6GRE in ll_addr_n2a() Serhey Popovych
@ 2017-12-20  7:57 ` Serhey Popovych
  2017-12-26 17:08 ` [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre Stephen Hemminger
  2 siblings, 0 replies; 4+ messages in thread
From: Serhey Popovych @ 2017-12-20  7:57 UTC (permalink / raw)
  To: netdev

Since rtnl_talk() never returns with answer buffer allocated
on error we do not need to release it manually. After this
initializing answer with NULL before rtnl_talk() is useless.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre.c    |    3 +--
 ip/link_gre6.c   |    3 +--
 ip/link_ip6tnl.c |    3 +--
 ip/link_iptnl.c  |    3 +--
 ip/link_vti.c    |    3 +--
 ip/link_vti6.c   |    3 +--
 6 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/ip/link_gre.c b/ip/link_gre.c
index 2397920..f55c40c 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -75,7 +75,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 		.i.ifi_family = preferred_family,
 		.i.ifi_index = ifi->ifi_index,
 	};
-	struct nlmsghdr *answer = NULL;
+	struct nlmsghdr *answer;
 	struct rtattr *tb[IFLA_MAX + 1];
 	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
 	struct rtattr *greinfo[IFLA_GRE_MAX + 1];
@@ -104,7 +104,6 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 get_failed:
 			fprintf(stderr,
 				"Failed to get existing tunnel info.\n");
-			free(answer);
 			return -1;
 		}
 
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7190ada..a3e8e08 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -86,7 +86,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 		.i.ifi_family = preferred_family,
 		.i.ifi_index = ifi->ifi_index,
 	};
-	struct nlmsghdr *answer = NULL;
+	struct nlmsghdr *answer;
 	struct rtattr *tb[IFLA_MAX + 1];
 	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
 	struct rtattr *greinfo[IFLA_GRE_MAX + 1];
@@ -114,7 +114,6 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 get_failed:
 			fprintf(stderr,
 				"Failed to get existing tunnel info.\n");
-			free(answer);
 			return -1;
 		}
 
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index f11ddd5..0a471c2 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -83,7 +83,7 @@ static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
 		.i.ifi_family = preferred_family,
 		.i.ifi_index = ifi->ifi_index,
 	};
-	struct nlmsghdr *answer = NULL;
+	struct nlmsghdr *answer;
 	struct rtattr *tb[IFLA_MAX + 1];
 	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
 	struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
@@ -108,7 +108,6 @@ static int ip6tunnel_parse_opt(struct link_util *lu, int argc, char **argv,
 get_failed:
 			fprintf(stderr,
 				"Failed to get existing tunnel info.\n");
-			free(answer);
 			return -1;
 		}
 
diff --git a/ip/link_iptnl.c b/ip/link_iptnl.c
index 5e4dffa..8a8f5dd 100644
--- a/ip/link_iptnl.c
+++ b/ip/link_iptnl.c
@@ -83,7 +83,7 @@ static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
 		.i.ifi_family = preferred_family,
 		.i.ifi_index = ifi->ifi_index,
 	};
-	struct nlmsghdr *answer = NULL;
+	struct nlmsghdr *answer;
 	struct rtattr *tb[IFLA_MAX + 1];
 	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
 	struct rtattr *iptuninfo[IFLA_IPTUN_MAX + 1];
@@ -112,7 +112,6 @@ static int iptunnel_parse_opt(struct link_util *lu, int argc, char **argv,
 get_failed:
 			fprintf(stderr,
 				"Failed to get existing tunnel info.\n");
-			free(answer);
 			return -1;
 		}
 
diff --git a/ip/link_vti.c b/ip/link_vti.c
index 6c5469f..2b0fab2 100644
--- a/ip/link_vti.c
+++ b/ip/link_vti.c
@@ -60,7 +60,7 @@ static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
 		.i.ifi_family = preferred_family,
 		.i.ifi_index = ifi->ifi_index,
 	};
-	struct nlmsghdr *answer = NULL;
+	struct nlmsghdr *answer;
 	struct rtattr *tb[IFLA_MAX + 1];
 	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
 	struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
@@ -77,7 +77,6 @@ static int vti_parse_opt(struct link_util *lu, int argc, char **argv,
 get_failed:
 			fprintf(stderr,
 				"Failed to get existing tunnel info.\n");
-			free(answer);
 			return -1;
 		}
 
diff --git a/ip/link_vti6.c b/ip/link_vti6.c
index 4136b0e..74c246d 100644
--- a/ip/link_vti6.c
+++ b/ip/link_vti6.c
@@ -55,7 +55,7 @@ static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
 		.i.ifi_family = preferred_family,
 		.i.ifi_index = ifi->ifi_index,
 	};
-	struct nlmsghdr *answer = NULL;
+	struct nlmsghdr *answer;
 	struct rtattr *tb[IFLA_MAX + 1];
 	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
 	struct rtattr *vtiinfo[IFLA_VTI_MAX + 1];
@@ -72,7 +72,6 @@ static int vti6_parse_opt(struct link_util *lu, int argc, char **argv,
 get_failed:
 			fprintf(stderr,
 				"Failed to get existing tunnel info.\n");
-			free(answer);
 			return -1;
 		}
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre
  2017-12-20  7:57 [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre Serhey Popovych
  2017-12-20  7:57 ` [PATCH iproute2 1/2] utils: ll_addr: Handle ARPHRD_IP6GRE in ll_addr_n2a() Serhey Popovych
  2017-12-20  7:57 ` [PATCH iproute2 2/2] ip/tunnel: No need to free answer after rtnl_talk() on error Serhey Popovych
@ 2017-12-26 17:08 ` Stephen Hemminger
  2 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2017-12-26 17:08 UTC (permalink / raw)
  To: Serhey Popovych; +Cc: netdev

On Wed, 20 Dec 2017 09:57:08 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:

> In this series I present next set of improvements/fixes:
> 
>   1) If rtnl_talk(), called with answer != NULL fails it
>      never returns with allocated answer: no need to
>      initialize answer and free() it in the caller on error.
> 
>   2) Correct ip6gre print endpoint address printing with
>      ll_addr_n2a(): endpoints are IPv6 addresses, not hex
>      dump.
> 
> See individual patch description message for details.
> 
> Thanks,
> Serhii
> 
> Serhey Popovych (2):
>   utils: ll_addr: Handle ARPHRD_IP6GRE in ll_addr_n2a()
>   ip/tunnel: No need to free answer after rtnl_talk() on error
> 
>  ip/link_gre.c    |    3 +--
>  ip/link_gre6.c   |    3 +--
>  ip/link_ip6tnl.c |    3 +--
>  ip/link_iptnl.c  |    3 +--
>  ip/link_vti.c    |    3 +--
>  ip/link_vti6.c   |    3 +--
>  lib/ll_addr.c    |    3 ++-
>  7 files changed, 8 insertions(+), 13 deletions(-)
> 

Applied, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-12-26 17:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-20  7:57 [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre Serhey Popovych
2017-12-20  7:57 ` [PATCH iproute2 1/2] utils: ll_addr: Handle ARPHRD_IP6GRE in ll_addr_n2a() Serhey Popovych
2017-12-20  7:57 ` [PATCH iproute2 2/2] ip/tunnel: No need to free answer after rtnl_talk() on error Serhey Popovych
2017-12-26 17:08 ` [PATCH iproute2 0/2] ip/tunnel: Do not free() answer on rtnl_talk() error, correct ip6gre Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.