Send connman mailing list submissions to
[email protected]
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
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. [PATCH] session: Do not call cleanup_session twice in
session_policy_config_cb (Harish Jenny K N)
2. [PATCH] session: call g_dbus_unregister_interface in
free_session (Harish Jenny K N)
3. Re: [PATCH 0/1] Don't use gateway as timeserver (Slava Monich)
4. Re: [PATCH 0/1] Don't use gateway as timeserver (Chris Novakovic)
----------------------------------------------------------------------
Message: 1
Date: Sat, 19 May 2018 16:58:37 +0530
From: Harish Jenny K N <[email protected]>
To: <[email protected]>
Subject: [PATCH] session: Do not call cleanup_session twice in
session_policy_config_cb
Message-ID:
<[email protected]>
Content-Type: text/plain
cleanup_session was getting called twice ( once from removal of
entry from global hash table ( i.e session_hash and another direct
call from error case in session_policy_config_cb.) which was
resulting in memory corruption due to use after free.
This patch removes calling cleanup_session twice in session_policy_config_cb.
---
src/session.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/session.c b/src/session.c
index 834b3a6..5a14cef 100644
--- a/src/session.c
+++ b/src/session.c
@@ -1407,8 +1407,9 @@ static int session_policy_config_cb(struct
connman_session *session,
session_methods, NULL, NULL,
session, NULL)) {
connman_error("Failed to register %s", session->session_path);
+ g_hash_table_remove(session_hash, session->session_path);
err = -EINVAL;
- goto err;
+ goto err_notify;
}
reply = g_dbus_create_reply(creation_data->pending,
@@ -1433,12 +1434,13 @@ static int session_policy_config_cb(struct
connman_session *session,
return 0;
err:
- g_hash_table_remove(session_hash, session->session_path);
+ cleanup_session(session);
+
+err_notify:
reply = __connman_error_failed(creation_data->pending, -err);
g_dbus_send_message(connection, reply);
creation_data->pending = NULL;
- cleanup_session(session);
cleanup_creation_data(creation_data);
return err;
--
1.9.1
------------------------------
Message: 2
Date: Sat, 19 May 2018 17:16:12 +0530
From: Harish Jenny K N <[email protected]>
To: <[email protected]>
Subject: [PATCH] session: call g_dbus_unregister_interface in
free_session
Message-ID:
<[email protected]>
Content-Type: text/plain
There is a scenario where g_dbus_register_interface will fail
in session_policy_config_cb due to failure to call
g_dbus_unregister_interface in cleanup_session.
Hence add a call to unregister dbus interface in cleanup_session.
---
src/session.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/session.c b/src/session.c
index 5a14cef..59a01c0 100644
--- a/src/session.c
+++ b/src/session.c
@@ -515,6 +515,9 @@ static void free_session(struct connman_session *session)
if (session->notify_watch > 0)
g_dbus_remove_watch(connection, session->notify_watch);
+ g_dbus_unregister_interface(connection, session->session_path,
+ CONNMAN_SESSION_INTERFACE);
+
destroy_policy_config(session);
g_slist_free(session->info->config.allowed_bearers);
g_free(session->info->config.allowed_interface);
--
1.9.1
------------------------------
Message: 3
Date: Sat, 19 May 2018 15:45:42 +0300
From: Slava Monich <[email protected]>
To: [email protected]
Subject: Re: [PATCH 0/1] Don't use gateway as timeserver
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
On 04/05/18 19:13, Jonah Petri wrote:
>
>> On May 4, 2018, at 12:06 PM, Chris Novakovic <[email protected]> wrote:
>>
>> Hi Marcel,
>>
>> On 04/05/2018 08:32, Marcel Holtmann wrote:
>>>> On my local network, I've noticed a client using ConnMan sending NTP
>>>> packets to an IP address specified in DHCP option 3, even when that IP
>>>> address doesn't appear in DHCP option 42. Upon closer inspection, this
>>>> is because src/timeserver.c adds the gateway for a particular service to
>>>> its list of NTP servers. This behaviour is incorrect: the only
>>>> assumption that can be made about a gateway is that it functions as a
>>>> router. If an NTP server is also present on the gateway, it is the job
>>>> of the DHCP server to inform clients of this via DHCP option 42, which
>>>> ConnMan already (correctly) uses when building its list of NTP servers.
>>>>
>>>> This patch stops ConnMan from automatically adding the IP addresses of
>>>> gateways to its list of NTP servers.
>>> but we are doing this on purpose. There is a good reason for this.
>> Could you elaborate on the reason, please? There's nothing in the
>> definition of a gateway that compels it to behave as a time server, and
>> I can't think of enough cases where they do to justify having this as
>> the default behaviour: I've yet to come across an ISP-supplied home
>> router that runs its own time server, and enterprises will typically run
>> time servers elsewhere in their network rather than dual-purposing the
>> gateway. That leaves SOHO routers/home routers running third-party
>> firmware, both of which are powerful enough to be configured to
>> correctly set DHCP option 42 if they also happen to run an NTP server.
>>
>>> If you don?t want it that way, then at least this needs to be hidden behind
>>> a main.conf option.
>>
>> Given the above, if a main.conf option is introduced for this, the
>> default behaviour ought to be not to send NTP traffic to the gateway, as
>> there's no prima facie reason for assuming it's capable of responding.
>>
>>> Or you just set the NTP server for your network.
>> This doesn't address the problem, I'm afraid: src/timeserver.c adds the
>> gateway to the list of time servers regardless of whether DHCP option 42
>> is set, so if there is no NTP server in the network, ConnMan will still
>> send NTP traffic to the gateway.
>>
> I want to chime in and say that I have had to disable connman NTP completely
> due to this behavior. Empirically there are lots of gateways with
> badly-configured NTP servers out there, sending out dead wrong time sync.
> We've had certificate validation failures in the field due to gateway-based
> timeservers serving up years-wrong dates.
>
> I would definitely prefer to see the treatment of the gateway as a NTP server
> disabled by default! Keep it as an option if necessary, but doing it by
> default is causing problems.
It had been attempted before (probably more than once, since this
behaviour is so obviously wrong), but to no avail:
https://lists.01.org/pipermail/connman/2015-March/019636.html
It's hopeless.
Cheers,
-Slava
------------------------------
Message: 4
Date: Sat, 19 May 2018 15:21:46 +0100
From: Chris Novakovic <[email protected]>
To: [email protected], Daniel Wagner <[email protected]>
Subject: Re: [PATCH 0/1] Don't use gateway as timeserver
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
I haven't heard back about this in over two weeks.
Daniel, I propose that you follow your original instinct and merge the
patch: the current behaviour is clearly and indefensibly broken.
On 04/05/2018 17:06, Chris Novakovic wrote:
> Hi Marcel,
>
> On 04/05/2018 08:32, Marcel Holtmann wrote:
>>> On my local network, I've noticed a client using ConnMan sending NTP
>>> packets to an IP address specified in DHCP option 3, even when that IP
>>> address doesn't appear in DHCP option 42. Upon closer inspection, this
>>> is because src/timeserver.c adds the gateway for a particular service to
>>> its list of NTP servers. This behaviour is incorrect: the only
>>> assumption that can be made about a gateway is that it functions as a
>>> router. If an NTP server is also present on the gateway, it is the job
>>> of the DHCP server to inform clients of this via DHCP option 42, which
>>> ConnMan already (correctly) uses when building its list of NTP servers.
>>>
>>> This patch stops ConnMan from automatically adding the IP addresses of
>>> gateways to its list of NTP servers.
>>
>> but we are doing this on purpose. There is a good reason for this.
>
> Could you elaborate on the reason, please? There's nothing in the
> definition of a gateway that compels it to behave as a time server, and
> I can't think of enough cases where they do to justify having this as
> the default behaviour: I've yet to come across an ISP-supplied home
> router that runs its own time server, and enterprises will typically run
> time servers elsewhere in their network rather than dual-purposing the
> gateway. That leaves SOHO routers/home routers running third-party
> firmware, both of which are powerful enough to be configured to
> correctly set DHCP option 42 if they also happen to run an NTP server.
>
>> If you don?t want it that way, then at least this needs to be hidden behind
>> a main.conf option.
>
>
> Given the above, if a main.conf option is introduced for this, the
> default behaviour ought to be not to send NTP traffic to the gateway, as
> there's no prima facie reason for assuming it's capable of responding.
>
>> Or you just set the NTP server for your network.
>
> This doesn't address the problem, I'm afraid: src/timeserver.c adds the
> gateway to the list of time servers regardless of whether DHCP option 42
> is set, so if there is no NTP server in the network, ConnMan will still
> send NTP traffic to the gateway.
>
> Cheers,
> Chris
>
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 31, Issue 13
***************************************
