Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Wrong configuration segmentionfault #1065

Merged
merged 3 commits into from

2 participants

@skinkie
Collaborator

In issue #356 I have described a way to alter the configuration file which caused a segmentation fault in the webserver. This segmentation fault origins from the fact that if a configuration phase is broken off, the properties of the handler will be freed. If the properties are on the other hand never initialised this will case badness in some (very) minor locations.

My solution vector is to first initialise the base class, so everything is set up to be "potentially" freed, just in case. Then the configuration phase hits in.

Specially review the handler_proxy.c code where I move the initialisation of the proxy hosts, to a different location, and I wonder if that is a good idea ...or not.

(I notice some commits are unrelated here)

skinkie added some commits
@skinkie skinkie We add an extra selector, because of the CSS selectivity which otherw…
…ise wouldn't allow the optional colour.
d6186a6
@skinkie skinkie Initialise the base class in the configuration phase, before parsing …
…the configuration.

This scheme prevents that a _free on the final class, results in chaos
because the base is not initialised yet, while an attempt is made to
free it.
ed75a47
@skinkie skinkie Free the virtual server if the configuration fails
In an extremely unlikely case that a configuration of a virtual server
fails we must free what we have just allocated.

The virtual server cannot be freed prior to a succesfull _new,
we may leak memory there, but in that situation we already exit the
server (too).
93fc150
@kinnison
Collaborator

I've looked as requested, but cannot see anything bad per-se. I'm not terribly pleased with unrelated commits in the same pull request but I understand how it goes :-)

This is not somewhere in the codebase I'm yet familiar though, so there may yet be issues I simply cannot predict.

@skinkie
Collaborator

In a shower moment I realised that there might be another solution for the base class initialisation that works out better.

@skinkie skinkie merged commit dc123e2 into master
@skinkie skinkie deleted the wrong_configuration_segv_356 branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 2, 2014
  1. @skinkie

    We add an extra selector, because of the CSS selectivity which otherw…

    skinkie authored
    …ise wouldn't allow the optional colour.
  2. @skinkie

    Initialise the base class in the configuration phase, before parsing …

    skinkie authored
    …the configuration.
    
    This scheme prevents that a _free on the final class, results in chaos
    because the base is not initialised yet, while an attempt is made to
    free it.
  3. @skinkie

    Free the virtual server if the configuration fails

    skinkie authored
    In an extremely unlikely case that a configuration of a virtual server
    fails we must free what we have just allocated.
    
    The virtual server cannot be freed prior to a succesfull _new,
    we may leak memory there, but in that situation we already exit the
    server (too).
This page is out of date. Refresh to see the latest.
View
3  admin/CTK/static/css/CTK.css
@@ -56,7 +56,8 @@ ul {list-style: none;}
/* Form Styles
*/
-input[type=text], input[type=password], textarea { border: 1px solid #b0bfce; margin: 1px; padding: 4px; color: #000; background: #fff url(/CTK/images/input-bg.png) left top repeat-x; outline: none; -moz-border-radius: 4px; -webkit-border-radius: 4px; }
+input { color: #000; }
+input[type=text], input[type=password], textarea { border: 1px solid #b0bfce; margin: 1px; padding: 4px; background: #fff url(/CTK/images/input-bg.png) left top repeat-x; outline: none; -moz-border-radius: 4px; -webkit-border-radius: 4px; !important; }
input[type=text]:focus, input[type=password]:focus, textarea:focus { border: 2px solid #2d70cb; margin: 0; }
input[type=text], input[type=password], textarea { width: 240px; }
select { border: 1px solid #b0bfce; color: #000; background: #fff; padding: 2px; outline: none; -moz-border-radius: 4px; -webkit-border-radius: 4px;}
View
10 cherokee/handler_fcgi.c
@@ -235,6 +235,11 @@ cherokee_handler_fcgi_configure (cherokee_config_node_t *conf,
props = PROP_FCGI(*_props);
+ /* Init base class
+ */
+ ret = cherokee_handler_cgi_base_configure (conf, srv, _props);
+ if (ret != ret_ok) return ret;
+
/* Parse the configuration tree
*/
cherokee_config_node_foreach (i, conf) {
@@ -246,11 +251,6 @@ cherokee_handler_fcgi_configure (cherokee_config_node_t *conf,
}
}
- /* Init base class
- */
- ret = cherokee_handler_cgi_base_configure (conf, srv, _props);
- if (ret != ret_ok) return ret;
-
/* Final checks
*/
if (props->balancer == NULL) {
View
6 cherokee/handler_proxy.c
@@ -140,6 +140,8 @@ cherokee_handler_proxy_configure (cherokee_config_node_t *conf,
cherokee_avl_init (&n->out_headers_hide);
cherokee_avl_set_case (&n->out_headers_hide, true);
+ cherokee_handler_proxy_hosts_init (&n->hosts);
+
*_props = MODULE_PROPS(n);
}
@@ -234,10 +236,6 @@ cherokee_handler_proxy_configure (cherokee_config_node_t *conf,
}
}
- /* Init properties
- */
- cherokee_handler_proxy_hosts_init (&props->hosts);
-
/* Final checks
*/
if (props->balancer == NULL) {
View
10 cherokee/handler_scgi.c
@@ -78,6 +78,11 @@ cherokee_handler_scgi_configure (cherokee_config_node_t *conf, cherokee_server_t
props = PROP_SCGI(*_props);
+ /* Init base class
+ */
+ ret = cherokee_handler_cgi_base_configure (conf, srv, _props);
+ if (ret != ret_ok) return ret;
+
/* Parse the configuration tree
*/
cherokee_config_node_foreach (i, conf) {
@@ -89,11 +94,6 @@ cherokee_handler_scgi_configure (cherokee_config_node_t *conf, cherokee_server_t
}
}
- /* Init base class
- */
- ret = cherokee_handler_cgi_base_configure (conf, srv, _props);
- if (ret != ret_ok) return ret;
-
/* Final checks
*/
if (props->balancer == NULL) {
View
9 cherokee/handler_uwsgi.c
@@ -83,6 +83,11 @@ cherokee_handler_uwsgi_configure (cherokee_config_node_t *conf, cherokee_server_
props = PROP_UWSGI(*_props);
+ /* Init base class
+ */
+ ret = cherokee_handler_cgi_base_configure (conf, srv, _props);
+ if (ret != ret_ok) return ret;
+
/* Parse the configuration tree
*/
cherokee_config_node_foreach (i, conf) {
@@ -112,10 +117,6 @@ cherokee_handler_uwsgi_configure (cherokee_config_node_t *conf, cherokee_server_
}
}
- /* Init base class
- */
- ret = cherokee_handler_cgi_base_configure (conf, srv, _props);
- if (ret != ret_ok) return ret;
/* Final checks
*/
View
1  cherokee/server.c
@@ -1290,6 +1290,7 @@ add_vserver (cherokee_config_node_t *conf, void *data)
cherokee_virtual_server_free (vsrv);
return ret_ok;
} else if (ret != ret_ok) {
+ cherokee_virtual_server_free (vsrv);
return ret;
}
Something went wrong with that request. Please try again.