mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-01-12 00:39:32 +00:00
BUG/MAJOR: tcp/http: fix current_rule assignment when restarting over a ruleset
Commit bc4c1ac
("MEDIUM: http/tcp: permit to resume http and tcp custom
actions") introduced the ability to interrupt and restart processing in
the middle of a TCP/HTTP ruleset. But it doesn't do it in a consistent
way : it checks current_rule_list, immediately dereferences current_rule,
which is only set in certain cases and never cleared. So that broke the
tcp-request content rules when the processing was interrupted due to
missing data, because current_rule was not yet set (segfault) or could
have been inherited from another ruleset if it was used in a backend
(random behaviour).
The proper way to do it is to always set current_rule before dereferencing
it. But we don't want to set it for all rules because we don't want any
action to provide a checkpointing mechanism. So current_rule is set to NULL
before entering the loop, and only used if not NULL and if current_rule_list
matches the current list. This way they both serve as a guard for the other
one. This fix also makes the current rule point to the rule instead of its
list element, as it's much easier to manipulate.
No backport is needed, this is 1.6-specific.
This commit is contained in:
parent
e759749b50
commit
152b81e7b2
@ -156,7 +156,7 @@ struct stream {
|
||||
|
||||
/* These two pointers are used to resume the execution of the rule lists. */
|
||||
struct list *current_rule_list; /* this is used to store the current executed rule list. */
|
||||
struct list *current_rule; /* this is used to store the current rule to be resumed. */
|
||||
void *current_rule; /* this is used to store the current rule to be resumed. */
|
||||
struct hlua hlua; /* lua runtime context */
|
||||
};
|
||||
|
||||
|
@ -3359,11 +3359,14 @@ http_req_get_intercept_rule(struct proxy *px, struct list *rules, struct stream
|
||||
* and never in the ACL or converters. In this case, we initialise the
|
||||
* current rule, and go to the action execution point.
|
||||
*/
|
||||
if (s->current_rule_list == rules) {
|
||||
rule = LIST_ELEM(s->current_rule, typeof(rule), list);
|
||||
goto resume_execution;
|
||||
if (s->current_rule) {
|
||||
rule = s->current_rule;
|
||||
s->current_rule = NULL;
|
||||
if (s->current_rule_list == rules)
|
||||
goto resume_execution;
|
||||
}
|
||||
s->current_rule_list = rules;
|
||||
|
||||
list_for_each_entry(rule, rules, list) {
|
||||
if (rule->action >= HTTP_REQ_ACT_MAX)
|
||||
continue;
|
||||
@ -3383,7 +3386,6 @@ http_req_get_intercept_rule(struct proxy *px, struct list *rules, struct stream
|
||||
}
|
||||
|
||||
resume_execution:
|
||||
|
||||
switch (rule->action) {
|
||||
case HTTP_REQ_ACT_ALLOW:
|
||||
return HTTP_RULE_RES_STOP;
|
||||
@ -3566,7 +3568,7 @@ resume_execution:
|
||||
|
||||
case HTTP_REQ_ACT_CUSTOM_CONT:
|
||||
if (!rule->action_ptr(rule, px, s)) {
|
||||
s->current_rule = &rule->list;
|
||||
s->current_rule = rule;
|
||||
return HTTP_RULE_RES_YIELD;
|
||||
}
|
||||
break;
|
||||
@ -3637,11 +3639,14 @@ http_res_get_intercept_rule(struct proxy *px, struct list *rules, struct stream
|
||||
* and never in the ACL or converters. In this case, we initialise the
|
||||
* current rule, and go to the action execution point.
|
||||
*/
|
||||
if (s->current_rule_list == rules) {
|
||||
rule = LIST_ELEM(s->current_rule, typeof(rule), list);
|
||||
goto resume_execution;
|
||||
if (s->current_rule) {
|
||||
rule = s->current_rule;
|
||||
s->current_rule = NULL;
|
||||
if (s->current_rule_list == rules)
|
||||
goto resume_execution;
|
||||
}
|
||||
s->current_rule_list = rules;
|
||||
|
||||
list_for_each_entry(rule, rules, list) {
|
||||
if (rule->action >= HTTP_RES_ACT_MAX)
|
||||
continue;
|
||||
@ -3661,7 +3666,6 @@ http_res_get_intercept_rule(struct proxy *px, struct list *rules, struct stream
|
||||
}
|
||||
|
||||
resume_execution:
|
||||
|
||||
switch (rule->action) {
|
||||
case HTTP_RES_ACT_ALLOW:
|
||||
return HTTP_RULE_RES_STOP; /* "allow" rules are OK */
|
||||
@ -3814,7 +3818,7 @@ resume_execution:
|
||||
|
||||
case HTTP_RES_ACT_CUSTOM_CONT:
|
||||
if (!rule->action_ptr(rule, px, s)) {
|
||||
s->current_rule = &rule->list;
|
||||
s->current_rule = rule;
|
||||
return HTTP_RULE_RES_YIELD;
|
||||
}
|
||||
break;
|
||||
|
@ -1125,11 +1125,14 @@ int tcp_inspect_request(struct stream *s, struct channel *req, int an_bit)
|
||||
* and never in the ACL or converters. In this case, we initialise the
|
||||
* current rule, and go to the action execution point.
|
||||
*/
|
||||
if (s->current_rule_list == &s->be->tcp_req.inspect_rules) {
|
||||
rule = LIST_ELEM(s->current_rule, typeof(rule), list);
|
||||
goto resume_execution;
|
||||
if (s->current_rule) {
|
||||
rule = s->current_rule;
|
||||
s->current_rule = NULL;
|
||||
if (s->current_rule_list == &s->be->tcp_req.inspect_rules)
|
||||
goto resume_execution;
|
||||
}
|
||||
s->current_rule_list = &s->be->tcp_req.inspect_rules;
|
||||
|
||||
list_for_each_entry(rule, &s->be->tcp_req.inspect_rules, list) {
|
||||
enum acl_test_res ret = ACL_TEST_PASS;
|
||||
|
||||
@ -1144,9 +1147,7 @@ int tcp_inspect_request(struct stream *s, struct channel *req, int an_bit)
|
||||
}
|
||||
|
||||
if (ret) {
|
||||
|
||||
resume_execution:
|
||||
|
||||
/* we have a matching rule. */
|
||||
if (rule->action == TCP_ACT_REJECT) {
|
||||
channel_abort(req);
|
||||
@ -1216,7 +1217,7 @@ resume_execution:
|
||||
else {
|
||||
/* Custom keywords. */
|
||||
if (rule->action_ptr(rule, s->be, s) == 0) {
|
||||
s->current_rule = &rule->list;
|
||||
s->current_rule = rule;
|
||||
goto missing_data;
|
||||
}
|
||||
|
||||
@ -1283,11 +1284,14 @@ int tcp_inspect_response(struct stream *s, struct channel *rep, int an_bit)
|
||||
* and never in the ACL or converters. In this case, we initialise the
|
||||
* current rule, and go to the action execution point.
|
||||
*/
|
||||
if (s->current_rule_list == &s->be->tcp_rep.inspect_rules) {
|
||||
rule = LIST_ELEM(s->current_rule, typeof(rule), list);
|
||||
goto resume_execution;
|
||||
if (s->current_rule) {
|
||||
rule = s->current_rule;
|
||||
s->current_rule = NULL;
|
||||
if (s->current_rule_list == &s->be->tcp_rep.inspect_rules)
|
||||
goto resume_execution;
|
||||
}
|
||||
s->current_rule_list = &s->be->tcp_rep.inspect_rules;
|
||||
|
||||
list_for_each_entry(rule, &s->be->tcp_rep.inspect_rules, list) {
|
||||
enum acl_test_res ret = ACL_TEST_PASS;
|
||||
|
||||
@ -1306,9 +1310,7 @@ int tcp_inspect_response(struct stream *s, struct channel *rep, int an_bit)
|
||||
}
|
||||
|
||||
if (ret) {
|
||||
|
||||
resume_execution:
|
||||
|
||||
/* we have a matching rule. */
|
||||
if (rule->action == TCP_ACT_REJECT) {
|
||||
channel_abort(rep);
|
||||
@ -1336,7 +1338,7 @@ resume_execution:
|
||||
/* Custom keywords. */
|
||||
if (!rule->action_ptr(rule, s->be, s)) {
|
||||
channel_dont_close(rep);
|
||||
s->current_rule = &rule->list;
|
||||
s->current_rule = rule;
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user