Brad Bishop | 37a0e4d | 2017-12-04 01:01:44 -0500 | [diff] [blame^] | 1 | From c7f4151fb053b0d0691d8f10d7e3690265d28889 Mon Sep 17 00:00:00 2001 |
| 2 | From: Lukasz Nowak <lnowak@tycoint.com> |
| 3 | Date: Wed, 26 Oct 2016 18:13:02 +0100 |
| 4 | Subject: [PATCH] stats: Fix bad file descriptor initialisation |
| 5 | |
| 6 | Stats file code initialises its file descriptor field to 0. But 0 is |
| 7 | a valid fd value. -1 should be used instead. This causes problems |
| 8 | when an error happens before a stats file is open (e.g. mkdir |
| 9 | fails). The clean-up procedure, stats_free() calls close(fd). When fd |
| 10 | is 0, this first closes stdin, and then any files/sockets which |
| 11 | received fd=0, re-used by the OS. |
| 12 | |
| 13 | Fixed several instances of bad file descriptor field handling, in case |
| 14 | of errors. |
| 15 | |
| 16 | The bug results with connman freezing if there is no read/write storage |
| 17 | directory available, and there are multiple active interfaces |
| 18 | (fd=0 gets re-used for sockets in that case). |
| 19 | |
| 20 | The patch was imported from the Connman git repository |
| 21 | (git://git.kernel.org/pub/scm/network/connman) as of commit id |
| 22 | c7f4151fb053b0d0691d8f10d7e3690265d28889. |
| 23 | |
| 24 | Upstream-Status: Accepted |
| 25 | Signed-off-by: Lukasz Nowak <lnowak@tycoint.com> |
| 26 | --- |
| 27 | src/stats.c | 15 +++++++++++++++ |
| 28 | src/util.c | 4 ++-- |
| 29 | 2 files changed, 17 insertions(+), 2 deletions(-) |
| 30 | |
| 31 | diff --git a/src/stats.c b/src/stats.c |
| 32 | index 26343b1..c3ca738 100644 |
| 33 | --- a/src/stats.c |
| 34 | +++ b/src/stats.c |
| 35 | @@ -378,6 +378,7 @@ static int stats_file_setup(struct stats_file *file) |
| 36 | strerror(errno), file->name); |
| 37 | |
| 38 | TFR(close(file->fd)); |
| 39 | + file->fd = -1; |
| 40 | g_free(file->name); |
| 41 | file->name = NULL; |
| 42 | |
| 43 | @@ -393,6 +394,7 @@ static int stats_file_setup(struct stats_file *file) |
| 44 | err = stats_file_remap(file, size); |
| 45 | if (err < 0) { |
| 46 | TFR(close(file->fd)); |
| 47 | + file->fd = -1; |
| 48 | g_free(file->name); |
| 49 | file->name = NULL; |
| 50 | |
| 51 | @@ -649,6 +651,13 @@ static int stats_file_history_update(struct stats_file *data_file) |
| 52 | bzero(history_file, sizeof(struct stats_file)); |
| 53 | bzero(temp_file, sizeof(struct stats_file)); |
| 54 | |
| 55 | + /* |
| 56 | + * 0 is a valid file descriptor - fd needs to be initialized |
| 57 | + * to -1 to handle errors correctly |
| 58 | + */ |
| 59 | + history_file->fd = -1; |
| 60 | + temp_file->fd = -1; |
| 61 | + |
| 62 | err = stats_open(history_file, data_file->history_name); |
| 63 | if (err < 0) |
| 64 | return err; |
| 65 | @@ -682,6 +691,12 @@ int __connman_stats_service_register(struct connman_service *service) |
| 66 | if (!file) |
| 67 | return -ENOMEM; |
| 68 | |
| 69 | + /* |
| 70 | + * 0 is a valid file descriptor - fd needs to be initialized |
| 71 | + * to -1 to handle errors correctly |
| 72 | + */ |
| 73 | + file->fd = -1; |
| 74 | + |
| 75 | g_hash_table_insert(stats_hash, service, file); |
| 76 | } else { |
| 77 | return -EALREADY; |
| 78 | diff --git a/src/util.c b/src/util.c |
| 79 | index e6532c8..732d451 100644 |
| 80 | --- a/src/util.c |
| 81 | +++ b/src/util.c |
| 82 | @@ -63,7 +63,7 @@ int __connman_util_init(void) |
| 83 | { |
| 84 | int r = 0; |
| 85 | |
| 86 | - if (f > 0) |
| 87 | + if (f >= 0) |
| 88 | return 0; |
| 89 | |
| 90 | f = open(URANDOM, O_RDONLY); |
| 91 | @@ -86,7 +86,7 @@ int __connman_util_init(void) |
| 92 | |
| 93 | void __connman_util_cleanup(void) |
| 94 | { |
| 95 | - if (f > 0) |
| 96 | + if (f >= 0) |
| 97 | close(f); |
| 98 | |
| 99 | f = -1; |
| 100 | -- |
| 101 | 2.7.4 |
| 102 | |