Found by Mischa Peters by running some experimental code of mine.
When chaining two proc-based filters together the second proc will not
get its parameter correctly, since lka_filter_process_response won't get
a parameter from the proceed keyword, resulting in a "...|(null)"
The solution seems to be to save the parameter in the filter_session
struct.
While here remove some useless if(NULL) checks and fix two memory leaks
(fs->helo and fs->mail_from).
Mischa has been running with this diff for several days now without
issue.
OK?
martijn@
Index: lka_filter.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.36
diff -u -p -r1.36 lka_filter.c
--- lka_filter.c 2 May 2019 11:39:45 -0000 1.36
+++ lka_filter.c 30 Jun 2019 14:50:58 -0000
@@ -41,7 +41,7 @@ struct filter;
struct filter_session;
static void filter_protocol_internal(struct filter_session *, uint64_t *,
uint64_t, enum filter_phase, const char *);
static void filter_protocol(uint64_t, enum filter_phase, const char *);
-static void filter_protocol_next(uint64_t, uint64_t, enum filter_phase,
const char *);
+static void filter_protocol_next(uint64_t, uint64_t, enum filter_phase);
static void filter_protocol_query(struct filter *, uint64_t, uint64_t,
const char *, const char *);
static void filter_data_internal(struct filter_session *, uint64_t,
uint64_t, const char *);
@@ -68,6 +68,8 @@ struct filter_session {
uint64_t id;
struct io *io;
+ char *lastparam;
+
char *filter_name;
struct sockaddr_storage ss_src;
struct sockaddr_storage ss_dest;
@@ -337,6 +339,9 @@ lka_filter_end(uint64_t reqid)
fs = tree_xpop(&sessions, reqid);
free(fs->rdns);
+ free(fs->helo);
+ free(fs->mail_from);
+ free(fs->lastparam);
free(fs);
log_trace(TRACE_FILTERS, "%016"PRIx64" filters session-end", reqid);
}
@@ -504,7 +509,7 @@ lka_filter_process_response(const char *
return 1;
}
- filter_protocol_next(token, reqid, 0, parameter);
+ filter_protocol_next(token, reqid, 0);
return 1;
}
@@ -658,14 +663,11 @@ filter_protocol(uint64_t reqid, enum fil
switch (phase) {
case FILTER_HELO:
case FILTER_EHLO:
- if (fs->helo)
- free(fs->helo);
+ free(fs->helo);
fs->helo = xstrdup(param);
break;
case FILTER_MAIL_FROM:
- if (fs->mail_from)
- free(fs->mail_from);
-
+ free(fs->mail_from);
fs->mail_from = xstrdup(param + 1);
*strchr(fs->mail_from, '>') = '\0';
param = fs->mail_from;
@@ -683,13 +685,17 @@ filter_protocol(uint64_t reqid, enum fil
default:
break;
}
+
+ free(fs->lastparam);
+ fs->lastparam = xstrdup(param);
+
filter_protocol_internal(fs, &token, reqid, phase, param);
if (nparam)
free(nparam);
}
static void
-filter_protocol_next(uint64_t token, uint64_t reqid, enum filter_phase phase,
const char *param)
+filter_protocol_next(uint64_t token, uint64_t reqid, enum filter_phase phase)
{
struct filter_session *fs;
@@ -697,7 +703,7 @@ filter_protocol_next(uint64_t token, uin
if ((fs = tree_get(&sessions, reqid)) == NULL)
return;
- filter_protocol_internal(fs, &token, reqid, phase, param);
+ filter_protocol_internal(fs, &token, reqid, phase, fs->lastparam);
}
static void