Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-08-06 Thread Patrik Flykt 

Hi,

Please do not send patches as attachments as the formatting gets mangled
when I reply to your patch.

On Mon, 2014-08-04 at 11:16 +, Saurav Babu wrote:
 diff --git a/src/dhcp.c b/src/dhcp.c index 132b787..a3dd3c4 100644 ---
 a/src/dhcp.c +++ b/src/dhcp.c @@ -587,6 +587,7 @@ int
 __connman_dhcp_start(struct connman_ipconfig *ipconfig, {
  const char *last_addr = NULL; struct connman_dhcp *dhcp;
  + int err; DBG();
  @@ -615,9 +616,12 @@
  int __connman_dhcp_start(struct connman_ipconfig *ipconfig,
 connman_network_ref(network);
  }
  - g_hash_table_insert(ipconfig_table, ipconfig, dhcp);
  + err = dhcp_initialize(dhcp);
  - dhcp_initialize(dhcp);
  + if(err != 0) + return err;
  + + g_hash_table_insert(ipconfig_table, ipconfig, dhcp);
  }
  dhcp-callback = callback;

First of all, check err for  0 as 0 and bigger signal some variants of
success. There is a memory leak here, the dhcp structure is allocated
but not freed on error. Also the network is referenced, but not
unreferenced on error.

Cheers,

Patrik


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] dhcp: Fixed Crash on Switching Network

2014-08-06 Thread Saurav Babu 
Sometimes while switching network dhcp_initialize() fails because
interface is not up and hence dhcp-dhcp_client remains NULL. Here we
don't check return type of dhcp_initialize() and go on to call function
g_dhcp_client_start() and crash occurs.
Below trace is obtained when connman crashes:
connmand[19034]: Aborting (signal 11) [/usr/local/sbin/connmand]
connmand[19034]:  backtrace 
connmand[19034]: #0  0xb7630f38 in /lib/i386-linux-gnu/libpthread.so.0
connmand[19034]: #1  0x8055a22 in debug() at client.c:0
connmand[19034]: #2  0x8058837 in g_dhcp_client_start() at polkit.c:0
connmand[19034]: #3  0x80a4772 in __connman_dhcp_start() at polkit.c:0
connmand[19034]: #4  0x8082a80 in set_connected.part.8() at network.c:0
connmand[19034]: #5  0x8082f7f in connman_network_set_connected() at
??:0
connmand[19034]: #6  0x805f921 in eth_network_connect() at ethernet.c:0
connmand[19034]: #7  0x8082dc3 in __connman_network_connect() at
polkit.c:0
connmand[19034]: #8  0x808e7e4 in __connman_service_connect() at
polkit.c:0
connmand[19034]: #9  0x808eef0 in auto_connect_service() at service.c:0
connmand[19034]: #10 0x808efde in run_auto_connect() at service.c:0
connmand[19034]: #11 0xb76cea3f in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #12 0xb76cdd46 in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #13 0xb76ce0e5 in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #14 0xb76ce52b in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #15 0x80544cd in main() at polkit.c:0
connmand[19034]: #16 0xb739b4d3 in /lib/i386-linux-gnu/libc.so.6
connmand[19034]: +++
---
 src/dhcp.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index d714f99..3e6ca3b 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -590,6 +590,7 @@ int __connman_dhcp_start(struct connman_ipconfig *ipconfig,
 {
const char *last_addr = NULL;
struct connman_dhcp *dhcp;
+   int err;
 
DBG();
 
@@ -618,9 +619,15 @@ int __connman_dhcp_start(struct connman_ipconfig *ipconfig,
connman_network_ref(network);
}
 
-   g_hash_table_insert(ipconfig_table, ipconfig, dhcp);
+   err = dhcp_initialize(dhcp);
 
-   dhcp_initialize(dhcp);
+   if(err  0) {
+   connman_network_unref(network);
+   g_free(dhcp);
+   return err;
+   }
+
+   g_hash_table_insert(ipconfig_table, ipconfig, dhcp);
}
 
dhcp-callback = callback;
-- 
1.9.1


Incorporated Patrik's Comments

Thanks,
Saurav
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-08-06 Thread Patrik Flykt 
On Mon, 2014-08-04 at 11:16 +, Saurav Babu wrote:
 Sometimes while switching network dhcp_initialize() fails because
 interface is not up and hence dhcp-dhcp_client remains NULL. Here we
 don't check return type of dhcp_initialize() and go on to call
 function g_dhcp_client_start() and crash occurs.

Could you actually post the logs when this happens? The log from just
before the start of the switch until the crash happens would be very
much more informative. The fix proposed does work and it will be
applied, but the root cause remains unknown with only the backtrace.

Just checking, but is this a regular desktop/laptop and a standard
ethernet card?

Cheers,

Patrik


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-08-06 Thread Patrik Flykt 
On Wed, 2014-08-06 at 14:29 +0530, Saurav Babu wrote:
 Sometimes while switching network dhcp_initialize() fails because
 interface is not up and hence dhcp-dhcp_client remains NULL. Here we
 don't check return type of dhcp_initialize() and go on to call function
 g_dhcp_client_start() and crash occurs.

Applied, thanks!

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

RE: [PATCH] dhcp: Fixed Crash on Switching Network

2014-08-04 Thread Chengyi Zhao 
Hi

 Date: Wed, 30 Jul 2014 15:51:54 +0300
 From: tomasz.burszt...@linux.intel.com
 To: saurav.b...@samsung.com; connman@connman.net
 Subject: Re: [PATCH] dhcp: Fixed Crash on Switching Network
 
 Hi,
 
  Sometimes while switching network dhcp_initialize() fails because
  interface is not up and hence dhcp-dhcp_client remains NULL. Here we
  don't check return type of dhcp_initialize() and go on to call function
  g_dhcp_client_start() and crash occurs.
 
 Good catch, but this makes me think there is another bug: if the device 
 is down,
 connman should not even think of starting dhcp on it, so there might be 
 another issue
 to get fixed here. What version of connman are you using btw?

This issue can be found in ConnMan 1.24,
and you can reproduce it when repeatedly operated the following command:

./test-connman offlinemode on
./test-connman offlinemode off

Thanks,
Chengyi
  
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-08-04 Thread Saurav Babu 


0001-dhcp-Fixed-Crash-on-Switching-Network.patch
Description: Binary data
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-08-04 Thread Tomasz Bursztyka 

Hi,

Thanks, patch looks ok to me. But that does not fix the root cause yet 
since basically,

connman should never start dhcp client if the device is not up.
Though I think your patch should come in anyway, it's indeed necessary 
to know if the

initialization went fine before proceeding further.

We are investigating this on our side since we've got the proper steps 
to reproduce it.


Tomasz


Sometimes while switching network dhcp_initialize() fails because
interface is not up and hence dhcp-dhcp_client remains NULL. Here we
don't check return type of dhcp_initialize() and go on to call function
g_dhcp_client_start() and crash occurs.
Below trace is obtained when connman crashes:
connmand[19034]: Aborting (signal 11) [/usr/local/sbin/connmand]
connmand[19034]:  backtrace 
connmand[19034]: #0  0xb7630f38 in /lib/i386-linux-gnu/libpthread.so.0
connmand[19034]: #1  0x8055a22 in debug() at client.c:0
connmand[19034]: #2  0x8058837 in g_dhcp_client_start() at polkit.c:0
connmand[19034]: #3  0x80a4772 in __connman_dhcp_start() at polkit.c:0
connmand[19034]: #4  0x8082a80 in set_connected.part.8() at network.c:0
connmand[19034]: #5  0x8082f7f in connman_network_set_connected() at
??:0
connmand[19034]: #6  0x805f921 in eth_network_connect() at ethernet.c:0
connmand[19034]: #7  0x8082dc3 in __connman_network_connect() at
polkit.c:0
connmand[19034]: #8  0x808e7e4 in __connman_service_connect() at
polkit.c:0
connmand[19034]: #9  0x808eef0 in auto_connect_service() at service.c:0
connmand[19034]: #10 0x808efde in run_auto_connect() at service.c:0
connmand[19034]: #11 0xb76cea3f in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #12 0xb76cdd46 in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #13 0xb76ce0e5 in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #14 0xb76ce52b in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #15 0x80544cd in main() at polkit.c:0
connmand[19034]: #16 0xb739b4d3 in /lib/i386-linux-gnu/libc.so.6
connmand[19034]: +++
---
 src/dhcp.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index 132b787..a3dd3c4 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -587,6 +587,7 @@ int __connman_dhcp_start(struct connman_ipconfig 
*ipconfig,

 {
  const char *last_addr = NULL;
  struct connman_dhcp *dhcp;
+ int err;

  DBG();

@@ -615,9 +616,12 @@ int __connman_dhcp_start(struct connman_ipconfig 
*ipconfig,

connman_network_ref(network);
   }

-  g_hash_table_insert(ipconfig_table, ipconfig, dhcp);
+  err = dhcp_initialize(dhcp);

-  dhcp_initialize(dhcp);
+  if(err != 0)
+   return err;
+
+  g_hash_table_insert(ipconfig_table, ipconfig, dhcp);
  }

  dhcp-callback = callback;



___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-07-30 Thread Saurav Babu 
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-07-30 Thread Tomasz Bursztyka 

Hi,

We still don't see your patch unfortunately.

Can you try with git send-email?
It's an opens-source mailing list here, so you don't need to send any 
header specifying there is trade secret etc...


Thanks,

Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-07-30 Thread Saurav Babu 


0001-dhcp-Fixed-Crash-on-Switching-Network.patch
Description: Binary data
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-07-30 Thread Tomasz Bursztyka 

Hi,


Sometimes while switching network dhcp_initialize() fails because
interface is not up and hence dhcp-dhcp_client remains NULL. Here we
don't check return type of dhcp_initialize() and go on to call function
g_dhcp_client_start() and crash occurs.


Good catch, but this makes me think there is another bug: if the device 
is down,
connman should not even think of starting dhcp on it, so there might be 
another issue

to get fixed here. What version of connman are you using btw?


Below trace is obtained when connman crashes:
connmand[19034]: Aborting (signal 11) [/usr/local/sbin/connmand]
connmand[19034]:  backtrace 
connmand[19034]: #0  0xb7630f38 in /lib/i386-linux-gnu/libpthread.so.0
connmand[19034]: #1  0x8055a22 in debug() at client.c:0
connmand[19034]: #2  0x8058837 in g_dhcp_client_start() at polkit.c:0
connmand[19034]: #3  0x80a4772 in __connman_dhcp_start() at polkit.c:0
connmand[19034]: #4  0x8082a80 in set_connected.part.8() at network.c:0
connmand[19034]: #5  0x8082f7f in connman_network_set_connected() at
??:0
connmand[19034]: #6  0x805f921 in eth_network_connect() at ethernet.c:0
connmand[19034]: #7  0x8082dc3 in __connman_network_connect() at
polkit.c:0
connmand[19034]: #8  0x808e7e4 in __connman_service_connect() at
polkit.c:0
connmand[19034]: #9  0x808eef0 in auto_connect_service() at service.c:0
connmand[19034]: #10 0x808efde in run_auto_connect() at service.c:0
connmand[19034]: #11 0xb76cea3f in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #12 0xb76cdd46 in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #13 0xb76ce0e5 in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #14 0xb76ce52b in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #15 0x80544cd in main() at polkit.c:0
connmand[19034]: #16 0xb739b4d3 in /lib/i386-linux-gnu/libc.so.6
connmand[19034]: +++
---
  src/dhcp.c |5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index 132b787..a2780ca 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -587,6 +587,7 @@ int __connman_dhcp_start(struct connman_ipconfig *ipconfig,
  {
const char *last_addr = NULL;
struct connman_dhcp *dhcp;
+   int err;
  
  	DBG();
  
@@ -617,7 +618,9 @@ int __connman_dhcp_start(struct connman_ipconfig *ipconfig,
  
  		g_hash_table_insert(ipconfig_table, ipconfig, dhcp);
  
-		dhcp_initialize(dhcp);

+   err = dhcp_initialize(dhcp);
+   if(err != 0)
+   return err;
}


There is an issue here: dhcp structure is inserted before hand in the 
hash table but can

be uninitialized in case of the error you spotted.

So I would instead change the order here: run first dhcp_initialize
- if an error is returned, free the dhcp structure, and return the error 
code.

- on success: insert it in the hash table.

Thanks,

Tomasz

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-07-30 Thread Saurav Babu 
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-07-30 Thread Saurav Babu 
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH] dhcp: Fixed Crash on Switching Network

2014-07-28 Thread Saurav Babu 
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Re: [PATCH] dhcp: Fixed Crash on Switching Network

2014-07-28 Thread Jukka Rissanen 
Hi,

I see only a .gif in your attachment, perhaps the actual patch is
missing? Also please send the patches as inline, you can for example use
git send-email to send them.


Cheers,
Jukka


___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman