File: 0023-groupingby_fix_invalid_memory_access.patch

package info (click to toggle)
syslog-ng 3.19.1-5
  • links: PTS, VCS
  • area: main
  • in suites: buster
  • size: 13,176 kB
  • sloc: ansic: 114,472; makefile: 4,697; sh: 4,391; python: 4,282; java: 4,047; xml: 2,435; yacc: 1,108; lex: 426; perl: 193; awk: 184
file content (136 lines) | stat: -rw-r--r-- 4,493 bytes parent folder | download
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
From 20926fb6ecd4ebfa8a36737cdbc7e8ae639fa085 Mon Sep 17 00:00:00 2001
From: Antal Nemes <antal.nemes@balabit.com>
Date: Tue, 2 Apr 2019 19:56:34 +0200
Subject: [PATCH] groupingby: fix invalid memory access

There was an invalid memory access in groupingby. The TimerWheel
object stores all timers, however the individual timers are also
stored inside the contexts.

The original code stores contexts in persist state, however the
timer_wheel is freed during reload. When the new config starts, and a
context is fetched, groupingby will access the already freed timer.

This patch stores timerwheel in persist state too.

Signed-off-by: Antal Nemes <antal.nemes@balabit.com>
---
 modules/dbparser/groupingby.c | 61 ++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 11 deletions(-)

diff --git a/modules/dbparser/groupingby.c b/modules/dbparser/groupingby.c
index a96b9a68c1..5a925c4347 100644
--- a/modules/dbparser/groupingby.c
+++ b/modules/dbparser/groupingby.c
@@ -46,8 +46,22 @@ typedef struct _GroupingBy
   FilterExprNode *having_condition_expr;
 } GroupingBy;
 
+typedef struct
+{
+  CorrellationState *correllation;
+  TimerWheel *timer_wheel;
+} GroupingByPersistData;
+
 static NVHandle context_id_handle = 0;
 
+static void
+_free_persist_data(GroupingByPersistData *self)
+{
+  correllation_state_free(self->correllation);
+  timer_wheel_free(self->timer_wheel);
+  g_free(self);
+}
+
 void
 grouping_by_set_key_template(LogParser *s, LogTemplate *key_template)
 {
@@ -381,6 +395,25 @@ grouping_by_process(LogParser *s, LogMessage **pmsg, const LogPathOptions *path_
   return TRUE;
 }
 
+static void
+_load_correllation_state(GroupingBy *self, GlobalConfig *cfg)
+{
+  GroupingByPersistData *persist_data = cfg_persist_config_fetch(cfg, grouping_by_format_persist_name(self));
+  if (persist_data)
+    {
+      self->correllation = persist_data->correllation;
+      self->timer_wheel = persist_data->timer_wheel;
+      timer_wheel_set_associated_data(self->timer_wheel, log_pipe_ref((LogPipe *)self), (GDestroyNotify)log_pipe_unref);
+    }
+  else
+    {
+      self->correllation = correllation_state_new();
+      self->timer_wheel = timer_wheel_new();
+      timer_wheel_set_associated_data(self->timer_wheel, log_pipe_ref((LogPipe *)self), (GDestroyNotify)log_pipe_unref);
+    }
+  g_free(persist_data);
+}
+
 static gboolean
 grouping_by_init(LogPipe *s)
 {
@@ -406,11 +439,8 @@ grouping_by_init(LogPipe *s)
       return FALSE;
     }
 
-  self->correllation = cfg_persist_config_fetch(cfg, grouping_by_format_persist_name(self));
-  if (!self->correllation)
-    {
-      self->correllation = correllation_state_new();
-    }
+  _load_correllation_state(self, cfg);
+
   iv_validate_now();
   IV_TIMER_INIT(&self->tick);
   self->tick.cookie = self;
@@ -430,6 +460,19 @@ grouping_by_init(LogPipe *s)
   return stateful_parser_init_method(s);
 }
 
+static void
+_store_data_in_persist(GroupingBy *self, GlobalConfig *cfg)
+{
+  GroupingByPersistData *persist_data = g_new0(GroupingByPersistData, 1);
+  persist_data->correllation = self->correllation;
+  persist_data->timer_wheel = self->timer_wheel;
+
+  cfg_persist_config_add(cfg, grouping_by_format_persist_name(self), persist_data,
+                         (GDestroyNotify) _free_persist_data, FALSE);
+  self->correllation = NULL;
+  self->timer_wheel = NULL;
+}
+
 static gboolean
 grouping_by_deinit(LogPipe *s)
 {
@@ -441,9 +484,8 @@ grouping_by_deinit(LogPipe *s)
       iv_timer_unregister(&self->tick);
     }
 
-  cfg_persist_config_add(cfg, grouping_by_format_persist_name(self), self->correllation,
-                         (GDestroyNotify) correllation_state_free, FALSE);
-  self->correllation = NULL;
+  _store_data_in_persist(self, cfg);
+
   return stateful_parser_deinit_method(s);
 }
 
@@ -469,7 +511,6 @@ grouping_by_free(LogPipe *s)
   log_template_unref(self->key_template);
   if (self->synthetic_message)
     synthetic_message_free(self->synthetic_message);
-  timer_wheel_free(self->timer_wheel);
   stateful_parser_free_method(s);
 
   filter_expr_unref(self->trigger_condition_expr);
@@ -490,8 +531,6 @@ grouping_by_new(GlobalConfig *cfg)
   self->super.super.process = grouping_by_process;
   g_static_mutex_init(&self->lock);
   self->scope = RCS_GLOBAL;
-  self->timer_wheel = timer_wheel_new();
-  timer_wheel_set_associated_data(self->timer_wheel, self, NULL);
   cached_g_current_time(&self->last_tick);
   self->timeout = -1;
   return &self->super.super;