2
1

0002-logsource-add-explicit-un-initialized-state-to-Windo.patch 4.2 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118
  1. From e350607f27b78094fb72422faf5394384ae6193b Mon Sep 17 00:00:00 2001
  2. From: Laszlo Budai <laszlo.budai@outlook.com>
  3. Date: Thu, 29 Aug 2019 17:09:39 +0200
  4. Subject: [PATCH] logsource: add explicit (un)initialized state to
  5. WindowSizeCounter
  6. Fixes: #2893
  7. On 32 bit systems (or non-64 bit systems), syslog-ng could abort during
  8. shutdown.
  9. What was the reason of the abort?
  10. a) in `log_source_set_options` where we set the initial window size
  11. conditionally, the condition was false thus the `full_window_size`
  12. remained 0
  13. b) when `log_source_free` is called during shutdown,
  14. * `_release_dynamic_window` called unconditionally and
  15. * a dynamic_part is calculated as full_window_size(=0) - init_window_size(=default 100),
  16. so dynamic_part = -100
  17. * window_size is decremented by dynamic_part(-100) and the
  18. `window_size_counter_sub` asserts on old_value >= value, and this
  19. assert failed, so syslog-ng aborted
  20. So the questions are
  21. 1) why we did not set initial window size?
  22. 2) why old_value was not greater than value?
  23. Answers:
  24. 1) the value we get from `window_size_counter_get` is the masked
  25. value... on 64 bit systems this value is a 63 bits of `1` and it is compared to
  26. a 32 bits of `1` but the 63 bits are truncated to 32 thanks to an explicit cast
  27. And what if we are on a 32 bits system?
  28. Well... the sizeof(gsize) is 4 , sizeof(gint) is also 4 on these
  29. systems. This means that the `window_size_counter_get` returns 31 bits of
  30. `-1`, and it is compared to 32 bits of `1` : they are obviously not
  31. equals -> we won't set full_window_size
  32. 2) old_value is a -1, which is masked, so the actual old value is 2^31-1, while new value is a
  33. -100, which is (2^32-100), so on a 32 bits system 31 bit negative value is
  34. compared to a to 32 bits negative value...
  35. Proposed solution:
  36. * add a initialized state to LogSource: this is checked/(set to TRUE) only in
  37. `log_source_set_options`, and set to FALSE only in `log_source_init_instance`
  38. Signed-off-by: Laszlo Budai <laszlo.budai@outlook.com>
  39. [Retrieved from:
  40. https://github.com/syslog-ng/syslog-ng/commit/e350607f27b78094fb72422faf5394384ae6193b]
  41. Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
  42. ---
  43. lib/logsource.c | 23 +++++++++++++++++------
  44. lib/logsource.h | 1 +
  45. 2 files changed, 18 insertions(+), 6 deletions(-)
  46. diff --git a/lib/logsource.c b/lib/logsource.c
  47. index 3f38b66e8..67e1c1570 100644
  48. --- a/lib/logsource.c
  49. +++ b/lib/logsource.c
  50. @@ -633,7 +633,20 @@ log_source_queue(LogPipe *s, LogMessage *msg, const LogPathOptions *path_options
  51. evt_tag_printf("msg", "%p", msg));
  52. msg_set_context(NULL);
  53. +}
  54. +
  55. +static void
  56. +_initialize_window(LogSource *self, gint init_window_size)
  57. +{
  58. + self->window_initialized = TRUE;
  59. + window_size_counter_set(&self->window_size, init_window_size);
  60. + self->full_window_size = init_window_size;
  61. +}
  62. +static gboolean
  63. +_is_window_initialized(LogSource *self)
  64. +{
  65. + return self->window_initialized;
  66. }
  67. void
  68. @@ -645,11 +658,9 @@ log_source_set_options(LogSource *self, LogSourceOptions *options,
  69. * configuration and we received a SIGHUP. This means that opened
  70. * connections will not have their window_size changed. */
  71. - if ((gint)window_size_counter_get(&self->window_size, NULL) == -1)
  72. - {
  73. - window_size_counter_set(&self->window_size, options->init_window_size);
  74. - self->full_window_size = options->init_window_size;
  75. - }
  76. + if (!_is_window_initialized(self))
  77. + _initialize_window(self, options->init_window_size);
  78. +
  79. self->options = options;
  80. if (self->stats_id)
  81. g_free(self->stats_id);
  82. @@ -679,7 +690,7 @@ log_source_init_instance(LogSource *self, GlobalConfig *cfg)
  83. self->super.free_fn = log_source_free;
  84. self->super.init = log_source_init;
  85. self->super.deinit = log_source_deinit;
  86. - window_size_counter_set(&self->window_size, (gsize)-1);
  87. + self->window_initialized = FALSE;
  88. self->ack_tracker = NULL;
  89. }
  90. diff --git a/lib/logsource.h b/lib/logsource.h
  91. index 370842efc..75d492604 100644
  92. --- a/lib/logsource.h
  93. +++ b/lib/logsource.h
  94. @@ -71,6 +71,7 @@ struct _LogSource
  95. gchar *stats_instance;
  96. WindowSizeCounter window_size;
  97. DynamicWindow dynamic_window;
  98. + gboolean window_initialized;
  99. /* full_window_size = static + dynamic */
  100. gsize full_window_size;
  101. atomic_gssize window_size_to_be_reclaimed;
  102. --
  103. 2.17.1