Send connman mailing list submissions to
        connman@lists.01.org

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
        connman-requ...@lists.01.org

You can reach the person managing the list at
        connman-ow...@lists.01.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. [PATCH 1/4] nat: Fix handle leak (Peter Meerwald-Stadler)
   2. [PATCH 3/4] iptables: Fix undefined code (right shift 32)
      (Peter Meerwald-Stadler)
   3. [PATCH 4/4] wifi: Drop unused error code (Peter Meerwald-Stadler)
   4. [PATCH 2/4] service: Fix wrong use of wrong enum type in
      __connman_service_reset_ipconfig() (Peter Meerwald-Stadler)
   5. Re: [PATCH] ntp: add detailed error msg on adjtimex failure
      (Daniel Wagner)
   6. Re: [PATCH 1/4] nat: Fix handle leak (Daniel Wagner)
   7. Re: rtnl sockets breaks with ENOBUFS after endless
      NEWROUTE/DELROUTE (Daniel Wagner)
   8. RE: rtnl sockets breaks with ENOBUFS after endless
      NEWROUTE/DELROUTE (Cunningham, Joel)


----------------------------------------------------------------------

Message: 1
Date: Sat, 16 Jun 2018 14:03:11 +0200
From: Peter Meerwald-Stadler <pme...@pmeerw.net>
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Subject: [PATCH 1/4] nat: Fix handle leak
Message-ID: <20180616120314.10002-1-pme...@pmeerw.net>

Need to close file handle on error

CID 1393448
---
 src/nat.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/nat.c b/src/nat.c
index fb557101..2aac60df 100644
--- a/src/nat.c
+++ b/src/nat.c
@@ -55,8 +55,10 @@ static int enable_ip_forward(bool enable)
                if (read(f, &value, sizeof(value)) < 0)
                        value = 0;
 
-               if (lseek(f, 0, SEEK_SET) < 0)
+               if (lseek(f, 0, SEEK_SET) < 0) {
+                       close(f);
                        return -errno;
+               }
        }
 
        if (enable) {
-- 
2.17.1



------------------------------

Message: 2
Date: Sat, 16 Jun 2018 14:03:13 +0200
From: Peter Meerwald-Stadler <pme...@pmeerw.net>
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Subject: [PATCH 3/4] iptables: Fix undefined code (right shift 32)
Message-ID: <20180616120314.10002-3-pme...@pmeerw.net>

GCC emits
warning: right shift count >= width of type
for 0xffffffffU >> 32

CID 1393447
---
 src/iptables.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/iptables.c b/src/iptables.c
index f3670e77..c4fd62e3 100644
--- a/src/iptables.c
+++ b/src/iptables.c
@@ -1729,9 +1729,11 @@ static int parse_ip_and_mask(const char *str, struct 
in_addr *ip,
                if (prefixlength > 32) {
                        err = -1;
                        goto out;
+               } else if (prefixlength == 32) {
+                       tmp = 0xffffffff;
+               } else {
+                       tmp = ~(0xffffffff >> prefixlength);
                }
-
-               tmp = ~(0xffffffff >> prefixlength);
        } else {
                tmp = 0xffffffff;
        }
-- 
2.17.1



------------------------------

Message: 3
Date: Sat, 16 Jun 2018 14:03:14 +0200
From: Peter Meerwald-Stadler <pme...@pmeerw.net>
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Subject: [PATCH 4/4] wifi: Drop unused error code
Message-ID: <20180616120314.10002-4-pme...@pmeerw.net>

the assigned error value is never used or overwritten before it
can be used

CID 1352483, CID 1352482
---
 plugins/wifi.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index 5b050469..ef8f435c 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -603,10 +603,8 @@ static int peer_register_service(const unsigned char 
*specification,
                params = fill_in_peer_service_params(specification,
                                                specification_length, query,
                                                query_length, version);
-               if (!params) {
-                       ret = -ENOMEM;
+               if (!params)
                        continue;
-               }
 
                if (!found) {
                        ret_f = g_supplicant_interface_p2p_add_service(iface,
@@ -691,10 +689,8 @@ static int peer_unregister_service(const unsigned char 
*specification,
                params = fill_in_peer_service_params(specification,
                                                specification_length, query,
                                                query_length, version);
-               if (!params) {
-                       ret = -ENOMEM;
+               if (!params)
                        continue;
-               }
 
                ret = g_supplicant_interface_p2p_del_service(iface, params);
                if (ret != 0 && ret != -EINPROGRESS)
-- 
2.17.1



------------------------------

Message: 4
Date: Sat, 16 Jun 2018 14:03:12 +0200
From: Peter Meerwald-Stadler <pme...@pmeerw.net>
To: Daniel Wagner <w...@monom.org>
Cc: connman@lists.01.org
Subject: [PATCH 2/4] service: Fix wrong use of wrong enum type in
        __connman_service_reset_ipconfig()
Message-ID: <20180616120314.10002-2-pme...@pmeerw.net>

address_updated() takes enum connman_ipconfig_type, not
enum connman_ipconfig_method

CID 1393449
---
 src/service.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/service.c b/src/service.c
index 4f819a66..1a7576f6 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3414,7 +3414,7 @@ int __connman_service_reset_ipconfig(struct 
connman_service *service,
                        *new_state = service->state_ipv6;
 
                settings_changed(service, new_ipconfig);
-               address_updated(service, new_method);
+               address_updated(service, type);
 
                
__connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
        }
-- 
2.17.1



------------------------------

Message: 5
Date: Sun, 17 Jun 2018 19:53:08 +0200
From: Daniel Wagner <w...@monom.org>
To: Eliott Dumeix <e.dum...@overkiz.com>
Cc: connman@lists.01.org
Subject: Re: [PATCH] ntp: add detailed error msg on adjtimex failure
Message-ID: <20180617175307.kmgddapd47kfu...@helium.monom.org>
Content-Type: text/plain; charset=us-ascii

Patch applied.

Thanks,
Daniel


------------------------------

Message: 6
Date: Sun, 17 Jun 2018 21:37:52 +0200
From: Daniel Wagner <w...@monom.org>
To: Peter Meerwald-Stadler <pme...@pmeerw.net>
Cc: connman@lists.01.org
Subject: Re: [PATCH 1/4] nat: Fix handle leak
Message-ID: <20180617193752.yrljpl3skbzuv...@helium.monom.org>
Content-Type: text/plain; charset=us-ascii

Hi Peter,

all 4 patches applied.

Thanks,
Daniel


------------------------------

Message: 7
Date: Sun, 17 Jun 2018 21:53:05 +0200
From: Daniel Wagner <w...@monom.org>
To: "Cunningham, Joel" <joel.cunning...@garmin.com>
Cc: "connman@lists.01.org" <connman@lists.01.org>
Subject: Re: rtnl sockets breaks with ENOBUFS after endless
        NEWROUTE/DELROUTE
Message-ID: <20180617195304.iik5dgsj5r3k5...@helium.monom.org>
Content-Type: text/plain; charset=us-ascii

Hi Joel,

On Fri, Jun 15, 2018 at 09:56:00PM +0000, Cunningham, Joel wrote:
> > I haven't look at the session code for a while but I suggest you start at
> > session.c:ipconfig_changed() and try to filter the
> >
> >  wlan0 {add} route 192.168.254.254 gw 0.0.0.0 scope 253 <LINK>
> >
> > for the policy routing table. I don't know if the RNTL message tells us for
> > which table it is.
> 
> struct rtmsg has the rtm_table member and RTA_TABLE will be present
> if rtm_table is RT_TABLE_UNSPEC. I was able to pass that through to
> __connman_ipconfig_newroute and __connman_ipconfig_delroute.

Okay, that sound reasonable. Guess we need to add something like this
to fix the problem.

> What I found out was for additional Adds/Deletes that come back,
> i.e. wlan0 {add} route 192.168.254.254 gw 0.0.0.0 scope 253 <LINK>,
> is for table 254 (Main). I don't have a lot of experience with Linux
> routing policies, but does it sound right that we'd add a default
> route for our custom table (256) and then the kernel would make
>  updates to the Main table?

At least this is not what I would expect. Maybe we should post a
question on the netdev mailing list. But best thing would be to post
the question alongside with the steps to reproduce it without ConnMan, e.g

$ ip route add ...

> I was able to do some simple filtering in
> __connman_ipconfig_newroute/__connman_ipconfig_delroute so that it
> ignores the route change if table < 256.  If that sounds correct, I
> can put together a patch.  Further thinking, I'm wondering if we
> should only be matching the route changes if the table ID matches the
> session's table ID.

It would suggest to go for the full solution. We might be able to
simplify some stuff if we match the changes correclty.

> This would require either maintaining a route
> table list in ipconfig.c or sending the table ID further down so it
> can be dropped before applying to the session in
> session.c:ipconfig_changed

Hmm, withouth looking at the code, I would probably do the table id
matching only for the policy tables in the session code. But if you
think it simpler to add it ipconfig.c just send a patch. It's hard
to tell without seeing the patch :)

Thanks,
Daniel


------------------------------

Message: 8
Date: Mon, 18 Jun 2018 19:03:50 +0000
From: "Cunningham, Joel" <joel.cunning...@garmin.com>
To: Daniel Wagner <w...@monom.org>
Cc: "connman@lists.01.org" <connman@lists.01.org>
Subject: RE: rtnl sockets breaks with ENOBUFS after endless
        NEWROUTE/DELROUTE
Message-ID: <5069da0739514cb195d289b685e39...@garmin.com>
Content-Type: text/plain; charset="us-ascii"

> -----Original Message-----
> From: Daniel Wagner [mailto:w...@monom.org]
> Sent: Sunday, June 17, 2018 2:53 PM
> To: Cunningham, Joel <joel.cunning...@garmin.com>
> Cc: connman@lists.01.org
> Subject: Re: rtnl sockets breaks with ENOBUFS after endless
> NEWROUTE/DELROUTE
>
> Hi Joel,
>
> On Fri, Jun 15, 2018 at 09:56:00PM +0000, Cunningham, Joel wrote:
> > > I haven't look at the session code for a while but I suggest you
> > > start at
> > > session.c:ipconfig_changed() and try to filter the
> > >
> > >  wlan0 {add} route 192.168.254.254 gw 0.0.0.0 scope 253 <LINK>
> > >
> > > for the policy routing table. I don't know if the RNTL message tells
> > > us for which table it is.
> >
> > struct rtmsg has the rtm_table member and RTA_TABLE will be present if
> > rtm_table is RT_TABLE_UNSPEC. I was able to pass that through to
> > __connman_ipconfig_newroute and __connman_ipconfig_delroute.
>
> Okay, that sound reasonable. Guess we need to add something like this to fix
> the problem.
>
> > What I found out was for additional Adds/Deletes that come back, i.e.
> > wlan0 {add} route 192.168.254.254 gw 0.0.0.0 scope 253 <LINK>, is for
> > table 254 (Main). I don't have a lot of experience with Linux routing
> > policies, but does it sound right that we'd add a default route for
> > our custom table (256) and then the kernel would make  updates to the
> > Main table?
>
> At least this is not what I would expect. Maybe we should post a question on
> the netdev mailing list. But best thing would be to post the question
> alongside with the steps to reproduce it without ConnMan, e.g
>
> $ ip route add ...

In attempting to reproduce this with 'ip route', I discovered that Connman's 
custom IP policy table was empty (ip route list table 256) and further that in 
my embedded system's kernel, CONFIG_IP_MULTIPLE_TABLES (IP policy routing) was 
set to 'n'.  I'm guessing the kernel in this mode, applied the routes to the 
default table rather than a custom one.

With CONFIG_IP_MULTIPLE_TABLES, we do still get a RTM_NEWROUTE (for the default 
table) when adding the default gateway to our custom table, which causes an 
del/add cycle, but no RTM_DELROUTE is reported for the default table when we 
remove the default route from our custom table and no additional RTM_NEWROUTE 
when adding back to our custom table, thus preventing a loop.

One additional thing that is not clear to me is why our NETLINK_ROUTE socket 
isn't receiving RTM_NEWROUTE/RTM_DELROUTE messages for the custom table.  Maybe 
custom tables aren't reported to RTMGRP_IPV4_ROUTE?

>
> > I was able to do some simple filtering in
> > __connman_ipconfig_newroute/__connman_ipconfig_delroute so that it
> > ignores the route change if table < 256.  If that sounds correct, I
> > can put together a patch.  Further thinking, I'm wondering if we
> > should only be matching the route changes if the table ID matches the
> > session's table ID.
>
> It would suggest to go for the full solution. We might be able to simplify 
> some
> stuff if we match the changes correclty.
>
> > This would require either maintaining a route table list in ipconfig.c
> > or sending the table ID further down so it can be dropped before
> > applying to the session in session.c:ipconfig_changed
>
> Hmm, withouth looking at the code, I would probably do the table id
> matching only for the policy tables in the session code. But if you think it
> simpler to add it ipconfig.c just send a patch. It's hard to tell without 
> seeing
> the patch :)

Even though my occurrence of this was resolved by enabling IP policy routing, 
to me it seems that Connman should be ignoring route changes for tables it's 
not using, so there's still a bug.

Joel

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of 
the intended recipient(s) and contain information that may be Garmin 
confidential and/or Garmin legally privileged. If you have received this email 
in error, please notify the sender by reply email and delete the message. Any 
disclosure, copying, distribution or use of this communication (including 
attachments) by someone other than the intended recipient is prohibited. Thank 
you.


------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
connman@lists.01.org
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 32, Issue 9
**************************************

Reply via email to