[PATCH] inet: fixed the check of inet_pton return value
--- src/inet.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/inet.c b/src/inet.c index cd220ff..fae36a0 100644 --- a/src/inet.c +++ b/src/inet.c @@ -634,17 +634,15 @@ int connman_inet_add_ipv6_network_route(int index, const char *host, rt.rtmsg_dst_len = prefix_len; - if (inet_pton(AF_INET6, host, &rt.rtmsg_dst) < 0) { + if (inet_pton(AF_INET6, host, &rt.rtmsg_dst) < 1) { err = -errno; goto out; } rt.rtmsg_flags = RTF_UP | RTF_HOST; - if (gateway) { + if (gateway && inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) > 0) rt.rtmsg_flags |= RTF_GATEWAY; - inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway); - } rt.rtmsg_metric = 1; rt.rtmsg_ifindex = index; @@ -686,7 +684,7 @@ int connman_inet_clear_ipv6_gateway_address(int index, const char *gateway) memset(&rt, 0, sizeof(rt)); - if (inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) < 0) { + if (inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) < 1) { err = -errno; goto out; } -- 1.8.3.2 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] inet: fixed the check of inet_pton return value
On Wed, 2015-04-01 at 22:32 +0300, Slava Monich wrote: > --- Now there are more changes than just a inet_pton retur value. Please write a proper commit message! > src/inet.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/src/inet.c b/src/inet.c > index cd220ff..fae36a0 100644 > --- a/src/inet.c > +++ b/src/inet.c > @@ -634,17 +634,15 @@ int connman_inet_add_ipv6_network_route(int index, > const char *host, > > rt.rtmsg_dst_len = prefix_len; > > - if (inet_pton(AF_INET6, host, &rt.rtmsg_dst) < 0) { > + if (inet_pton(AF_INET6, host, &rt.rtmsg_dst) < 1) { > err = -errno; > goto out; > } > > rt.rtmsg_flags = RTF_UP | RTF_HOST; > > - if (gateway) { > + if (gateway && inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) > 0) > rt.rtmsg_flags |= RTF_GATEWAY; > - inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway); > - } At this point if inet_pton fails, why are we continuing with the rt.rtmsg? Apparently there was a gateway with an unusable IPv6 address so shouldn't we return error instead? > rt.rtmsg_metric = 1; > rt.rtmsg_ifindex = index; > @@ -686,7 +684,7 @@ int connman_inet_clear_ipv6_gateway_address(int index, > const char *gateway) > > memset(&rt, 0, sizeof(rt)); > > - if (inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) < 0) { > + if (inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) < 1) { > err = -errno; > goto out; > } Cheers, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] inet: fixed the check of inet_pton return value
On 02/04/15 09:14, Patrik Flykt wrote: > @@ -634,17 +634,15 @@ int connman_inet_add_ipv6_network_route(int index, > const char *host, > > rt.rtmsg_dst_len = prefix_len; > > - if (inet_pton(AF_INET6, host, &rt.rtmsg_dst) < 0) { > + if (inet_pton(AF_INET6, host, &rt.rtmsg_dst) < 1) { > err = -errno; > goto out; > } > > rt.rtmsg_flags = RTF_UP | RTF_HOST; > > - if (gateway) { > + if (gateway && inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) > 0) > rt.rtmsg_flags |= RTF_GATEWAY; > - inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway); > - } > At this point if inet_pton fails, why are we continuing with the > rt.rtmsg? Apparently there was a gateway with an unusable IPv6 address > so shouldn't we return error instead? In cases where it does happen in reality, even if we return an error, the caller (add_nameserver_route) would call connman_inet_add_ipv6_network_route again with NULL gateway. This code is doing it in one shot but it does make an assumption about what the caller actually means by providing an invalid gateway address. In any case it's better than using bogus gateway address. Regards, -Slava ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] inet: fixed the check of inet_pton return value
Hi Slava, On to, 2015-04-02 at 12:20 +0300, Slava Monich wrote: > On 02/04/15 09:14, Patrik Flykt wrote: > > @@ -634,17 +634,15 @@ int connman_inet_add_ipv6_network_route(int index, > > const char *host, > > > > rt.rtmsg_dst_len = prefix_len; > > > > - if (inet_pton(AF_INET6, host, &rt.rtmsg_dst) < 0) { > > + if (inet_pton(AF_INET6, host, &rt.rtmsg_dst) < 1) { > > err = -errno; > > goto out; > > } > > > > rt.rtmsg_flags = RTF_UP | RTF_HOST; > > > > - if (gateway) { > > + if (gateway && inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) > 0) > > rt.rtmsg_flags |= RTF_GATEWAY; > > - inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway); > > - } > > At this point if inet_pton fails, why are we continuing with the > > rt.rtmsg? Apparently there was a gateway with an unusable IPv6 address > > so shouldn't we return error instead? > > > In cases where it does happen in reality, even if we return an error, > the caller (add_nameserver_route) would call > connman_inet_add_ipv6_network_route again with NULL gateway. This code > is doing it in one shot but it does make an assumption about what the > caller actually means by providing an invalid gateway address. > > In any case it's better than using bogus gateway address. As Patrik mentioned, why not just do if (gateway) { if (inet_pton(AF_INET6, gateway, &rt.rtmsg_gateway) < 1) return -EINVAL; rt.rtmsg_flags |= RTF_GATEWAY; } and let caller decide what to do next if gateway address was bogus. Cheers, Jukka ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman