[
https://issues.apache.org/jira/browse/AXIS2C-783?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12546095
]
Bill Mitchell commented on AXIS2C-783:
--------------------------------------
In doing some additional testing I uncovered additional error paths that
generate a seg fault. These were revealed indirectly as a result of the
introduction of the call to axis2_dep_engine_free () above to avoid memory
leaks. To create these, I accidentally pointed the home directory to a
structure with valid xml files that enabled module addressing, but without the
module addressing DLL being present.
This scenario uncovered three cleanup problems on error:
(1) In 3 places in dep_engine.c, it sets the error code
AXIS2_ERROR_MODULE_VALIDATION_FAILED
if (AXIS2_FAILURE == status)
{
axis2_repos_listener_free(dep_engine->repos_listener, env);
axis2_conf_free(dep_engine->conf, env);
AXIS2_ERROR_SET(env->error, AXIS2_ERROR_MODULE_VALIDATION_FAILED,
AXIS2_FAILURE);
return NULL;
}
In all 3 of these, after calling axis2_conf_free() the pointer to the free
configuration should be cleared:
dep_engine->conf = NULL;
I observed 3 additional places axis2_dep_engine_load() in dep_engine.c where
axis2_conf_free() is invoked without clearing the pointer to the freed
structure.
(2) In axis2_dep_engine_do_deploy(), the path of the AXIS2_MODULE remembers the
arch_reader in dep_engine->arch_reader. There are three places were the
arch_reader is then freed without clearing the dep_engine->arch_reader pointer.
These later result is a second free of the same structure. (See below for
code changes.)
(3) Also in axis2_dep_engine_do_deploy(), the current file is remembered in
dep_engine->curr_file. This pointer is not cleared upon error and should be.
Otherwise, as the file is also recorded in the ws_to_deploy list, the file will
be freed twice in axis2_dep_engine_free(), once as the curr_file and again when
the entire ws_to_deploy_list is freed.
The following code of the trailing body of axis2_dep_engine_do_deploy()
illustrates the changes for (2) and (3):
type = axis2_arch_file_data_get_type(dep_engine->curr_file, env);
switch (type)
{
case AXIS2_SVC:
arch_reader = axis2_arch_reader_create(env);
svc_grp = axis2_svc_grp_create_with_conf(env,
dep_engine->conf);
file_name = axis2_arch_file_data_get_name(dep_engine->
curr_file, env);
status = axis2_arch_reader_process_svc_grp(arch_reader, env,
file_name,
dep_engine,
svc_grp);
if (AXIS2_SUCCESS != status)
{
axis2_arch_reader_free(arch_reader, env);
dep_engine->curr_file = NULL;
AXIS2_ERROR_SET(env->error, AXIS2_ERROR_INVALID_SVC,
AXIS2_FAILURE);
return status;
}
status = axis2_dep_engine_add_new_svc(dep_engine, env,
svc_grp);
if (AXIS2_SUCCESS != status)
{
axis2_arch_reader_free(arch_reader, env);
dep_engine->curr_file = NULL;
AXIS2_ERROR_SET(env->error, AXIS2_ERROR_INVALID_SVC,
AXIS2_FAILURE);
return status;
}
dep_engine->curr_file = NULL;
break;
case AXIS2_MODULE:
arch_reader = axis2_arch_reader_create(env);
if (dep_engine->arch_reader)
{
axis2_arch_reader_free(dep_engine->arch_reader, env);
}
dep_engine->arch_reader = axis2_arch_reader_create(env);
meta_data = axis2_module_desc_create(env);
file_name = axis2_arch_file_data_get_name(dep_engine->
curr_file, env);
status =
axis2_arch_reader_read_module_arch(env, file_name,
dep_engine,
meta_data);
if (AXIS2_SUCCESS != status)
{
axis2_arch_reader_free(arch_reader, env);
dep_engine->arch_reader = NULL;
dep_engine->curr_file = NULL;
AXIS2_ERROR_SET(env->error, AXIS2_ERROR_INVALID_MODULE,
AXIS2_FAILURE);
return AXIS2_FAILURE;
}
status = axis2_dep_engine_add_new_module(dep_engine, env,
meta_data);
if (AXIS2_SUCCESS != status)
{
axis2_arch_reader_free(arch_reader, env);
dep_engine->arch_reader = NULL;
dep_engine->curr_file = NULL;
AXIS2_ERROR_SET(env->error, AXIS2_ERROR_INVALID_MODULE,
AXIS2_FAILURE);
return AXIS2_FAILURE;
}
dep_engine->curr_file = NULL;
break;
}
axis2_arch_reader_free(arch_reader, env);
dep_engine->arch_reader = NULL;
dep_engine->curr_file = NULL;
}
}
return AXIS2_SUCCESS;
After introducing these fixes, when the axis2_mod_addr dll was not found in the
addressing directory, the client receives an Axis error 27, Module validation
failed, and no seg fault.
> Client seg faults if AXIS2C_HOME variable not set
> -------------------------------------------------
>
> Key: AXIS2C-783
> URL: https://issues.apache.org/jira/browse/AXIS2C-783
> Project: Axis2-C
> Issue Type: Bug
> Components: code generation, core/clientapi, core/deployment
> Affects Versions: 1.1.0
> Environment: Window XP, Visual Studio 2005
> Reporter: Bill Mitchell
> Assignee: Dinesh Premalal
> Fix For: 1.2.0
>
>
> If the AXIS2C_HOME variable is not set, or if the client application code
> sets the ClientHome argument to a directory that does not containt axis2.xml
> and related files, the client application fails with a segmentation fault.
> Investigating this uncovered several issues, best described by highlighting
> the relevant actions in the code in chronological order:
> (1) axis2_dep_engine_check_client_home() in depengine.c detects the problem
> and stores an error code, AXIS2_ERROR_CONFIG_NOT_FOUND (18). But processing
> at this point is allowed to continue.
> (2) axis2_conf_builder_create_with_file_and_dep_engine_and_conf(), in
> particular axis2_desc_builder_create_with_file_and_dep_engine(), then changes
> the status_code in the environment error structure back to success. This is
> a side-effect of invoking the AXIS2_PARM_CHECK macro.
> (3) In axis2_desc_builder_build_om(), the call to
> axiom_xml_reader_create_for_file() returns an error, but the error number has
> now been replaced
> with AXIS2_ERROR_CREATING_XML_STREAM_READER(141).
> Axis2_desc_builder_build_om() then replaces this error number with the same
> value, just to
> make sure it is well and truly changed.
> (4) In stub.c, axis2_stub_create_with_endpoint_ref_and_client_home() replaces
> the error 141 with a truly misleading AXIS2_ERROR_NO_MEMORY(2).
> (5) In the generated stub code to create the service,
> axis2_stub_servicename_create() ignores the fact that the returned stub
> pointer is zero and goes ahead and calls
> axis2_stub_servicename_populate_services() anyway, where the code dies
> dereferencing the zero pointer.
> Recommendations:
> (5) In the generated stub code for axis2_stub_servicename_create(), the
> fragment
> stub = axis2_stub_create_with_endpoint_ref_and_client_home ( env,
> endpoint_ref, client_home );
> axis2_stub_servicename_populate_services( stub, env );
> return stub;
> should read:
> stub = axis2_stub_create_with_endpoint_ref_and_client_home ( env,
> endpoint_ref, client_home );
> if (stub)
> {
> axis2_stub_servicename_populate_services( stub, env );
> }
> return stub;
> (4) In stub.c, the procedure
> axis2_stub_create_with_endpoint_ref_and_client_home() really needs to let the
> underlying error number be returned. In the fragment:
> /* create service_client*/
> stub->svc_client = axis2_svc_client_create(env , client_home);
> if (!stub->svc_client)
> {
> axis2_stub_free(stub, env);
> AXIS2_ERROR_SET(env->error, AXIS2_ERROR_NO_MEMORY, AXIS2_FAILURE);
> return NULL;
> }
> the AXIS2_ERROR_SET invocation should be removed.
> (1) Where the client has passed a clienthome parameter, but it does not point
> to a valid Axis2C home directory structure, I prefer the first error code
> AXIS2_ERROR_CONFIG_NOT_FOUND to the second
> AXIS2_ERROR_CREATING_XML_STREAM_READER. So I suggest that
> axis2_dep_engine_load_client() in depengine.c be changed to treat this as an
> immediate error instead of letting processing continue. The fragment:
> if (client_home && 0 != axutil_strcmp("", client_home))
> {
> status = axis2_dep_engine_check_client_home(dep_engine, env,
> client_home);
> if (AXIS2_SUCCESS == status)
> {
> is_repos_exist = AXIS2_TRUE;
> }
> }
> else
> could be changed to:
> if (client_home && 0 != axutil_strcmp("", client_home))
> {
> status = axis2_dep_engine_check_client_home(dep_engine, env,
> client_home);
> if (AXIS2_SUCCESS != status)
> {
> return NULL;
> }
> is_repos_exist = AXIS2_TRUE;
> }
> else
> Of course, there may be a good reason to want to continue processing that I
> have not uncovered. In this case, you could instead do something to resolve
> item(2), such that the original error code could be returned instead of
> overlaying it as happens at point (3).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
