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
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
|
From bd553499909d2d57fd05696dc7604901cef3a36a Mon Sep 17 00:00:00 2001
From: Soumya Koduri <skoduri@redhat.com>
Date: Fri, 7 Jun 2019 17:20:15 +0530
Subject: [PATCH 200/221] gfapi: fix incorrect initialization of upcall syncop
arguments
While sending upcall notifications via synctasks, the argument used to
carry relevant data for these tasks is not initialized properly. This patch
is to fix the same.
This is backport of below upstream fix -
mainline: https://review.gluster.org/22839
release-6: https://review.gluster.org/22871
Change-Id: I9fa8f841e71d3c37d3819fbd430382928c07176c
fixes: bz#1717784
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/173508
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Kaleb Keithley <kkeithle@redhat.com>
---
api/src/glfs-fops.c | 109 ++++++++++++++++++++++++++++++++++------------------
1 file changed, 72 insertions(+), 37 deletions(-)
diff --git a/api/src/glfs-fops.c b/api/src/glfs-fops.c
index 01ba60b..396f18c 100644
--- a/api/src/glfs-fops.c
+++ b/api/src/glfs-fops.c
@@ -34,7 +34,7 @@
struct upcall_syncop_args {
struct glfs *fs;
- struct gf_upcall *upcall_data;
+ struct glfs_upcall *up_arg;
};
#define READDIRBUF_SIZE (sizeof(struct dirent) + GF_NAME_MAX + 1)
@@ -5714,12 +5714,28 @@ out:
}
static int
+upcall_syncop_args_free(struct upcall_syncop_args *args)
+{
+ if (args && args->up_arg)
+ GLFS_FREE(args->up_arg);
+ GF_FREE(args);
+ return 0;
+}
+
+static int
glfs_upcall_syncop_cbk(int ret, call_frame_t *frame, void *opaque)
{
struct upcall_syncop_args *args = opaque;
- GF_FREE(args->upcall_data);
- GF_FREE(args);
+ /* Here we not using upcall_syncop_args_free as application
+ * will be cleaning up the args->up_arg using glfs_free
+ * post processing upcall.
+ */
+ if (ret) {
+ upcall_syncop_args_free(args);
+ } else
+ GF_FREE(args);
+
return 0;
}
@@ -5727,13 +5743,29 @@ static int
glfs_cbk_upcall_syncop(void *opaque)
{
struct upcall_syncop_args *args = opaque;
- int ret = -1;
struct glfs_upcall *up_arg = NULL;
struct glfs *fs;
- struct gf_upcall *upcall_data;
fs = args->fs;
- upcall_data = args->upcall_data;
+ up_arg = args->up_arg;
+
+ if (fs->up_cbk && up_arg) {
+ (fs->up_cbk)(up_arg, fs->up_data);
+ return 0;
+ }
+
+ return -1;
+}
+
+static struct upcall_syncop_args *
+upcall_syncop_args_init(struct glfs *fs, struct gf_upcall *upcall_data)
+{
+ struct upcall_syncop_args *args = NULL;
+ int ret = -1;
+ struct glfs_upcall *up_arg = NULL;
+
+ if (!fs || !upcall_data)
+ goto out;
up_arg = GLFS_CALLOC(1, sizeof(struct gf_upcall), glfs_release_upcall,
glfs_mt_upcall_entry_t);
@@ -5754,33 +5786,51 @@ glfs_cbk_upcall_syncop(void *opaque)
errno = EINVAL;
}
- if (!ret && (up_arg->reason != GLFS_UPCALL_EVENT_NULL)) {
- /* It could so happen that the file which got
- * upcall notification may have got deleted by
- * the same client. In such cases up_arg->reason
- * is set to GLFS_UPCALL_EVENT_NULL. No need to
- * send upcall then */
- (fs->up_cbk)(up_arg, fs->up_data);
- } else if (up_arg->reason == GLFS_UPCALL_EVENT_NULL) {
+ /* It could so happen that the file which got
+ * upcall notification may have got deleted by
+ * the same client. In such cases up_arg->reason
+ * is set to GLFS_UPCALL_EVENT_NULL. No need to
+ * send upcall then
+ */
+ if (up_arg->reason == GLFS_UPCALL_EVENT_NULL) {
gf_msg(THIS->name, GF_LOG_DEBUG, errno, API_MSG_INVALID_ENTRY,
"Upcall_EVENT_NULL received. Skipping it.");
goto out;
- } else {
+ } else if (ret) {
gf_msg(THIS->name, GF_LOG_ERROR, errno, API_MSG_INVALID_ENTRY,
"Upcall entry validation failed.");
goto out;
}
+ args = GF_CALLOC(1, sizeof(struct upcall_syncop_args),
+ glfs_mt_upcall_entry_t);
+ if (!args) {
+ gf_msg(THIS->name, GF_LOG_ERROR, ENOMEM, API_MSG_ALLOC_FAILED,
+ "Upcall syncop args allocation failed.");
+ goto out;
+ }
+
+ /* Note: we are not taking any ref on fs here.
+ * Ideally applications have to unregister for upcall events
+ * or stop polling for upcall events before performing
+ * glfs_fini. And as for outstanding synctasks created, we wait
+ * for all syncenv threads to finish tasks before cleaning up the
+ * fs->ctx. Hence it seems safe to process these callback
+ * notification without taking any lock/ref.
+ */
+ args->fs = fs;
+ args->up_arg = up_arg;
+
/* application takes care of calling glfs_free on up_arg post
* their processing */
- ret = 0;
+ return args;
out:
- if (ret && up_arg) {
+ if (up_arg) {
GLFS_FREE(up_arg);
}
- return 0;
+ return NULL;
}
static void
@@ -5797,24 +5847,10 @@ glfs_cbk_upcall_data(struct glfs *fs, struct gf_upcall *upcall_data)
goto out;
}
- args = GF_CALLOC(1, sizeof(struct upcall_syncop_args),
- glfs_mt_upcall_entry_t);
- if (!args) {
- gf_msg(THIS->name, GF_LOG_ERROR, ENOMEM, API_MSG_ALLOC_FAILED,
- "Upcall syncop args allocation failed.");
- goto out;
- }
+ args = upcall_syncop_args_init(fs, upcall_data);
- /* Note: we are not taking any ref on fs here.
- * Ideally applications have to unregister for upcall events
- * or stop polling for upcall events before performing
- * glfs_fini. And as for outstanding synctasks created, we wait
- * for all syncenv threads to finish tasks before cleaning up the
- * fs->ctx. Hence it seems safe to process these callback
- * notification without taking any lock/ref.
- */
- args->fs = fs;
- args->upcall_data = gf_memdup(upcall_data, sizeof(*upcall_data));
+ if (!args)
+ goto out;
ret = synctask_new(THIS->ctx->env, glfs_cbk_upcall_syncop,
glfs_upcall_syncop_cbk, NULL, args);
@@ -5823,8 +5859,7 @@ glfs_cbk_upcall_data(struct glfs *fs, struct gf_upcall *upcall_data)
gf_msg(THIS->name, GF_LOG_ERROR, errno, API_MSG_UPCALL_SYNCOP_FAILED,
"Synctak for Upcall event_type(%d) and gfid(%s) failed",
upcall_data->event_type, (char *)(upcall_data->gfid));
- GF_FREE(args->upcall_data);
- GF_FREE(args);
+ upcall_syncop_args_free(args);
}
out:
--
1.8.3.1
|
