* [PATCH] connman: fix bad file descriptor initialisation
@ 2016-11-01 8:50 Lukasz Nowak
2016-11-01 11:46 ` Burton, Ross
0 siblings, 1 reply; 6+ messages in thread
From: Lukasz Nowak @ 2016-11-01 8:50 UTC (permalink / raw)
To: openembedded-core
From: Lukasz Nowak <lnowak@tycoint.com>
Import a patch from upstream, which fixes a connman daemon freeze
under certain conditions (multiple active interfaces, no r/w storage).
Signed-off-by: Lukasz Nowak <lnowak@tycoint.com>
---
...ts-Fix-bad-file-descriptor-initialisation.patch | 102 +++++++++++++++++++++
meta/recipes-connectivity/connman/connman_1.33.bb | 1 +
2 files changed, 103 insertions(+)
create mode 100644 meta/recipes-connectivity/connman/connman/0003-stats-Fix-bad-file-descriptor-initialisation.patch
diff --git a/meta/recipes-connectivity/connman/connman/0003-stats-Fix-bad-file-descriptor-initialisation.patch b/meta/recipes-connectivity/connman/connman/0003-stats-Fix-bad-file-descriptor-initialisation.patch
new file mode 100644
index 0000000..c545811
--- /dev/null
+++ b/meta/recipes-connectivity/connman/connman/0003-stats-Fix-bad-file-descriptor-initialisation.patch
@@ -0,0 +1,102 @@
+From c7f4151fb053b0d0691d8f10d7e3690265d28889 Mon Sep 17 00:00:00 2001
+From: Lukasz Nowak <lnowak@tycoint.com>
+Date: Wed, 26 Oct 2016 18:13:02 +0100
+Subject: [PATCH] stats: Fix bad file descriptor initialisation
+
+Stats file code initialises its file descriptor field to 0. But 0 is
+a valid fd value. -1 should be used instead. This causes problems
+when an error happens before a stats file is open (e.g. mkdir
+fails). The clean-up procedure, stats_free() calls close(fd). When fd
+is 0, this first closes stdin, and then any files/sockets which
+received fd=0, re-used by the OS.
+
+Fixed several instances of bad file descriptor field handling, in case
+of errors.
+
+The bug results with connman freezing if there is no read/write storage
+directory available, and there are multiple active interfaces
+(fd=0 gets re-used for sockets in that case).
+
+The patch was imported from the Connman git repository
+(git://git.kernel.org/pub/scm/network/connman) as of commit id
+c7f4151fb053b0d0691d8f10d7e3690265d28889.
+
+Upstream-Status: Accepted
+Signed-off-by: Lukasz Nowak <lnowak@tycoint.com>
+---
+ src/stats.c | 15 +++++++++++++++
+ src/util.c | 4 ++--
+ 2 files changed, 17 insertions(+), 2 deletions(-)
+
+diff --git a/src/stats.c b/src/stats.c
+index 26343b1..c3ca738 100644
+--- a/src/stats.c
++++ b/src/stats.c
+@@ -378,6 +378,7 @@ static int stats_file_setup(struct stats_file *file)
+ strerror(errno), file->name);
+
+ TFR(close(file->fd));
++ file->fd = -1;
+ g_free(file->name);
+ file->name = NULL;
+
+@@ -393,6 +394,7 @@ static int stats_file_setup(struct stats_file *file)
+ err = stats_file_remap(file, size);
+ if (err < 0) {
+ TFR(close(file->fd));
++ file->fd = -1;
+ g_free(file->name);
+ file->name = NULL;
+
+@@ -649,6 +651,13 @@ static int stats_file_history_update(struct stats_file *data_file)
+ bzero(history_file, sizeof(struct stats_file));
+ bzero(temp_file, sizeof(struct stats_file));
+
++ /*
++ * 0 is a valid file descriptor - fd needs to be initialized
++ * to -1 to handle errors correctly
++ */
++ history_file->fd = -1;
++ temp_file->fd = -1;
++
+ err = stats_open(history_file, data_file->history_name);
+ if (err < 0)
+ return err;
+@@ -682,6 +691,12 @@ int __connman_stats_service_register(struct connman_service *service)
+ if (!file)
+ return -ENOMEM;
+
++ /*
++ * 0 is a valid file descriptor - fd needs to be initialized
++ * to -1 to handle errors correctly
++ */
++ file->fd = -1;
++
+ g_hash_table_insert(stats_hash, service, file);
+ } else {
+ return -EALREADY;
+diff --git a/src/util.c b/src/util.c
+index e6532c8..732d451 100644
+--- a/src/util.c
++++ b/src/util.c
+@@ -63,7 +63,7 @@ int __connman_util_init(void)
+ {
+ int r = 0;
+
+- if (f > 0)
++ if (f >= 0)
+ return 0;
+
+ f = open(URANDOM, O_RDONLY);
+@@ -86,7 +86,7 @@ int __connman_util_init(void)
+
+ void __connman_util_cleanup(void)
+ {
+- if (f > 0)
++ if (f >= 0)
+ close(f);
+
+ f = -1;
+--
+2.7.4
+
diff --git a/meta/recipes-connectivity/connman/connman_1.33.bb b/meta/recipes-connectivity/connman/connman_1.33.bb
index 6ea1a08..56a0a0f 100644
--- a/meta/recipes-connectivity/connman/connman_1.33.bb
+++ b/meta/recipes-connectivity/connman/connman_1.33.bb
@@ -5,6 +5,7 @@ SRC_URI = "${KERNELORG_MIRROR}/linux/network/${BPN}/${BP}.tar.xz \
file://connman \
file://no-version-scripts.patch \
file://includes.patch \
+ file://0003-stats-Fix-bad-file-descriptor-initialisation.patch \
"
SRC_URI_append_libc-musl = " file://0002-resolve-musl-does-not-implement-res_ninit.patch"
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] connman: fix bad file descriptor initialisation
2016-11-01 8:50 [PATCH] connman: fix bad file descriptor initialisation Lukasz Nowak
@ 2016-11-01 11:46 ` Burton, Ross
2016-11-01 12:53 ` Lukasz Nowak
0 siblings, 1 reply; 6+ messages in thread
From: Burton, Ross @ 2016-11-01 11:46 UTC (permalink / raw)
To: Lukasz Nowak; +Cc: OE-core
[-- Attachment #1: Type: text/plain, Size: 390 bytes --]
On 1 November 2016 at 08:50, Lukasz Nowak <lnowak.tyco@gmail.com> wrote:
> From: Lukasz Nowak <lnowak@tycoint.com>
>
> Import a patch from upstream, which fixes a connman daemon freeze
> under certain conditions (multiple active interfaces, no r/w storage).
>
> Signed-off-by: Lukasz Nowak <lnowak@tycoint.com>
> ---
>
Is this a resent of your earlier patch, or a v2?
Ross
[-- Attachment #2: Type: text/html, Size: 896 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] connman: fix bad file descriptor initialisation
2016-11-01 11:46 ` Burton, Ross
@ 2016-11-01 12:53 ` Lukasz Nowak
2016-11-02 2:45 ` Paul Eggleton
2016-11-02 8:10 ` Anders Darander
0 siblings, 2 replies; 6+ messages in thread
From: Lukasz Nowak @ 2016-11-01 12:53 UTC (permalink / raw)
To: Burton, Ross; +Cc: OE-core
Hi Ross,
It is a re-send. Apologies, but I have not received either of my two e-mails back from the mailing list, so I assumed the first one got stuck somewhere. I can see both of them in the web archive, though. Is that normal?
I've just checked and I have the "Receive your own posts to the list?" option set to Yes.
Lukasz
On 01.11.2016 12:46, Burton, Ross wrote:
> On 1 November 2016 at 08:50, Lukasz Nowak <lnowak.tyco@gmail.com <mailto:lnowak.tyco@gmail.com>> wrote:
>
> From: Lukasz Nowak <lnowak@tycoint.com <mailto:lnowak@tycoint.com>>
>
> Import a patch from upstream, which fixes a connman daemon freeze
> under certain conditions (multiple active interfaces, no r/w storage).
>
> Signed-off-by: Lukasz Nowak <lnowak@tycoint.com <mailto:lnowak@tycoint.com>>
> ---
>
>
> Is this a resent of your earlier patch, or a v2?
>
> Ross
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] connman: fix bad file descriptor initialisation
2016-11-01 12:53 ` Lukasz Nowak
@ 2016-11-02 2:45 ` Paul Eggleton
2016-11-02 8:10 ` Anders Darander
1 sibling, 0 replies; 6+ messages in thread
From: Paul Eggleton @ 2016-11-02 2:45 UTC (permalink / raw)
To: Lukasz Nowak; +Cc: openembedded-core
On Tue, 01 Nov 2016 13:53:44 Lukasz Nowak wrote:
> It is a re-send. Apologies, but I have not received either of my two e-mails
> back from the mailing list, so I assumed the first one got stuck somewhere.
> I can see both of them in the web archive, though. Is that normal?
>
> I've just checked and I have the "Receive your own posts to the list?"
> option set to Yes.
You should be receiving your own posts then. They aren't going into your spam
folder by any chance?
Cheers,
Paul
--
Paul Eggleton
Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] connman: fix bad file descriptor initialisation
2016-11-01 12:53 ` Lukasz Nowak
2016-11-02 2:45 ` Paul Eggleton
@ 2016-11-02 8:10 ` Anders Darander
1 sibling, 0 replies; 6+ messages in thread
From: Anders Darander @ 2016-11-02 8:10 UTC (permalink / raw)
To: openembedded-core
* Lukasz Nowak <lnowak.tyco@gmail.com> [161101 13:57]:
> It is a re-send. Apologies, but I have not received either of my two
> e-mails back from the mailing list, so I assumed the first one got
> stuck somewhere. I can see both of them in the web archive, though. Is
> that normal?
Are you receiving any email you send to an list? GMail is pretty good at
removing the e-mails from a list that you sent yourself... At least,
I've never received any of them myself.
There's quite a few old threads about this on GMails support forum.
/Anders
--
Anders Darander, Senior System Architect
ChargeStorm AB / eStorm AB
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] connman: fix bad file descriptor initialisation
@ 2016-10-30 17:10 Lukasz Nowak
0 siblings, 0 replies; 6+ messages in thread
From: Lukasz Nowak @ 2016-10-30 17:10 UTC (permalink / raw)
To: openembedded-core
From: Lukasz Nowak <lnowak@tycoint.com>
Import a patch from upstream, which fixes a connman daemon freeze
under certain conditions (multiple active interfaces, no r/w storage).
Signed-off-by: Lukasz Nowak <lnowak@tycoint.com>
---
...ts-Fix-bad-file-descriptor-initialisation.patch | 102 +++++++++++++++++++++
meta/recipes-connectivity/connman/connman_1.33.bb | 1 +
2 files changed, 103 insertions(+)
create mode 100644 meta/recipes-connectivity/connman/connman/0003-stats-Fix-bad-file-descriptor-initialisation.patch
diff --git a/meta/recipes-connectivity/connman/connman/0003-stats-Fix-bad-file-descriptor-initialisation.patch b/meta/recipes-connectivity/connman/connman/0003-stats-Fix-bad-file-descriptor-initialisation.patch
new file mode 100644
index 0000000..c545811
--- /dev/null
+++ b/meta/recipes-connectivity/connman/connman/0003-stats-Fix-bad-file-descriptor-initialisation.patch
@@ -0,0 +1,102 @@
+From c7f4151fb053b0d0691d8f10d7e3690265d28889 Mon Sep 17 00:00:00 2001
+From: Lukasz Nowak <lnowak@tycoint.com>
+Date: Wed, 26 Oct 2016 18:13:02 +0100
+Subject: [PATCH] stats: Fix bad file descriptor initialisation
+
+Stats file code initialises its file descriptor field to 0. But 0 is
+a valid fd value. -1 should be used instead. This causes problems
+when an error happens before a stats file is open (e.g. mkdir
+fails). The clean-up procedure, stats_free() calls close(fd). When fd
+is 0, this first closes stdin, and then any files/sockets which
+received fd=0, re-used by the OS.
+
+Fixed several instances of bad file descriptor field handling, in case
+of errors.
+
+The bug results with connman freezing if there is no read/write storage
+directory available, and there are multiple active interfaces
+(fd=0 gets re-used for sockets in that case).
+
+The patch was imported from the Connman git repository
+(git://git.kernel.org/pub/scm/network/connman) as of commit id
+c7f4151fb053b0d0691d8f10d7e3690265d28889.
+
+Upstream-Status: Accepted
+Signed-off-by: Lukasz Nowak <lnowak@tycoint.com>
+---
+ src/stats.c | 15 +++++++++++++++
+ src/util.c | 4 ++--
+ 2 files changed, 17 insertions(+), 2 deletions(-)
+
+diff --git a/src/stats.c b/src/stats.c
+index 26343b1..c3ca738 100644
+--- a/src/stats.c
++++ b/src/stats.c
+@@ -378,6 +378,7 @@ static int stats_file_setup(struct stats_file *file)
+ strerror(errno), file->name);
+
+ TFR(close(file->fd));
++ file->fd = -1;
+ g_free(file->name);
+ file->name = NULL;
+
+@@ -393,6 +394,7 @@ static int stats_file_setup(struct stats_file *file)
+ err = stats_file_remap(file, size);
+ if (err < 0) {
+ TFR(close(file->fd));
++ file->fd = -1;
+ g_free(file->name);
+ file->name = NULL;
+
+@@ -649,6 +651,13 @@ static int stats_file_history_update(struct stats_file *data_file)
+ bzero(history_file, sizeof(struct stats_file));
+ bzero(temp_file, sizeof(struct stats_file));
+
++ /*
++ * 0 is a valid file descriptor - fd needs to be initialized
++ * to -1 to handle errors correctly
++ */
++ history_file->fd = -1;
++ temp_file->fd = -1;
++
+ err = stats_open(history_file, data_file->history_name);
+ if (err < 0)
+ return err;
+@@ -682,6 +691,12 @@ int __connman_stats_service_register(struct connman_service *service)
+ if (!file)
+ return -ENOMEM;
+
++ /*
++ * 0 is a valid file descriptor - fd needs to be initialized
++ * to -1 to handle errors correctly
++ */
++ file->fd = -1;
++
+ g_hash_table_insert(stats_hash, service, file);
+ } else {
+ return -EALREADY;
+diff --git a/src/util.c b/src/util.c
+index e6532c8..732d451 100644
+--- a/src/util.c
++++ b/src/util.c
+@@ -63,7 +63,7 @@ int __connman_util_init(void)
+ {
+ int r = 0;
+
+- if (f > 0)
++ if (f >= 0)
+ return 0;
+
+ f = open(URANDOM, O_RDONLY);
+@@ -86,7 +86,7 @@ int __connman_util_init(void)
+
+ void __connman_util_cleanup(void)
+ {
+- if (f > 0)
++ if (f >= 0)
+ close(f);
+
+ f = -1;
+--
+2.7.4
+
diff --git a/meta/recipes-connectivity/connman/connman_1.33.bb b/meta/recipes-connectivity/connman/connman_1.33.bb
index 6ea1a08..56a0a0f 100644
--- a/meta/recipes-connectivity/connman/connman_1.33.bb
+++ b/meta/recipes-connectivity/connman/connman_1.33.bb
@@ -5,6 +5,7 @@ SRC_URI = "${KERNELORG_MIRROR}/linux/network/${BPN}/${BP}.tar.xz \
file://connman \
file://no-version-scripts.patch \
file://includes.patch \
+ file://0003-stats-Fix-bad-file-descriptor-initialisation.patch \
"
SRC_URI_append_libc-musl = " file://0002-resolve-musl-does-not-implement-res_ninit.patch"
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-02 8:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-01 8:50 [PATCH] connman: fix bad file descriptor initialisation Lukasz Nowak
2016-11-01 11:46 ` Burton, Ross
2016-11-01 12:53 ` Lukasz Nowak
2016-11-02 2:45 ` Paul Eggleton
2016-11-02 8:10 ` Anders Darander
-- strict thread matches above, loose matches on Subject: below --
2016-10-30 17:10 Lukasz Nowak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox