Skip to content

Commit

Permalink
5494 libndmp memory leaks when setting a property
Browse files Browse the repository at this point in the history 
Reviewed by: Dan Fields <dan.fields@nexenta.com>
Approved by: Gordon Ross <gwr@nexenta.com>
  • Loading branch information
@gwr
Jan Kryl authored and gwr committed Jan 8, 2015
1 parent 5ff8cfa commit f093add
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 107 deletions.
8 changes: 4 additions & 4 deletions usr/src/cmd/ndmpadm/ndmpadm_main.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
* Copyright 2014 Nexenta Systems, Inc. All rights reserved.
* Copyright 2015 Nexenta Systems, Inc. All rights reserved.
*/

/*
Expand Down Expand Up @@ -355,7 +355,7 @@ ndmp_set_config_process(char *propname)
ret = ndmp_set_prop(propname, propvalue);
if (ret != -1) {
if (!ndmp_door_status()) {
if (ndmp_service_refresh() == -1)
if (ndmp_service_refresh() != 0)
(void) fprintf(stdout, gettext("Could not "
"refesh property of service ndmpd\n"));
}
Expand Down Expand Up @@ -631,7 +631,7 @@ ndmp_enable_auth(int argc, char **argv, ndmp_command_t *cur_cmd)
continue;
}
if (!ndmp_door_status() &&
(ndmp_service_refresh()) == -1) {
(ndmp_service_refresh()) != 0) {
(void) fprintf(stdout,
gettext("Could not refesh ndmpd service "
"properties\n"));
Expand Down Expand Up @@ -692,7 +692,7 @@ ndmp_disable_auth(int argc, char **argv, ndmp_command_t *cur_cmd)
continue;
}
if (!ndmp_door_status() &&
(ndmp_service_refresh()) == -1) {
(ndmp_service_refresh()) != 0) {
(void) fprintf(stdout, gettext("Could not "
"refesh ndmpd service properties\n"));
}
Expand Down
205 changes: 102 additions & 103 deletions usr/src/lib/libndmp/common/libndmp_prop.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright 2008 Sun Microsystems, Inc. All rights reserved.
* Use is subject to license terms.
* Copyright 2012 Milan Jurik. All rights reserved.
* Copyright 2014 Nexenta Systems, Inc. All rights reserved.
* Copyright 2015 Nexenta Systems, Inc. All rights reserved.
*/

/*
Expand Down Expand Up @@ -74,11 +74,11 @@ typedef struct ndmp_scfhandle {
scf_propertygroup_t *scf_pg;
} ndmp_scfhandle_t;

static int ndmp_config_saveenv(ndmp_scfhandle_t *);
static int ndmp_config_saveenv(ndmp_scfhandle_t *, boolean_t);
static ndmp_scfhandle_t *ndmp_smf_scf_init(const char *);
static void ndmp_smf_scf_fini(ndmp_scfhandle_t *);
static int ndmp_smf_start_transaction(ndmp_scfhandle_t *);
static int ndmp_smf_end_transaction(ndmp_scfhandle_t *);
static int ndmp_smf_end_transaction(ndmp_scfhandle_t *, boolean_t);
static int ndmp_smf_set_property(ndmp_scfhandle_t *, const char *,
const char *);
static int ndmp_smf_get_property(ndmp_scfhandle_t *, const char *, char *,
Expand All @@ -94,11 +94,11 @@ static int ndmp_smf_get_pg_name(ndmp_scfhandle_t *, const char *, char **);
int
ndmp_service_refresh(void)
{
if ((smf_get_state(NDMP_INST)) != NULL)
return (smf_refresh_instance(NDMP_INST));
int rc = smf_refresh_instance(NDMP_INST);

ndmp_errno = ENDMP_SMF_INTERNAL;
return (-1);
if (rc != 0)
ndmp_errno = ENDMP_SMF_INTERNAL;
return (rc);
}

/*
Expand All @@ -109,26 +109,26 @@ ndmp_service_refresh(void)
int
ndmp_get_prop(const char *prop, char **value)
{
ndmp_scfhandle_t *handle = NULL;
char *lval = (char *)malloc(NDMP_PROP_LEN);
ndmp_scfhandle_t *handle;
char *lval;
char *pgname;

if (!lval) {
ndmp_errno = ENDMP_MEM_ALLOC;
return (-1);
}
*value = NULL;
if ((handle = ndmp_smf_scf_init(NDMP_GROUP_FMRI_PREFIX)) == NULL) {
free(lval);
return (-1);
}
if (ndmp_smf_get_pg_name(handle, prop, &pgname)) {
free(lval);
ndmp_smf_scf_fini(handle);
ndmp_errno = ENDMP_SMF_PROP_GRP;
return (-1);
}
if (ndmp_smf_create_service_pgroup(handle, pgname)) {
ndmp_smf_scf_fini(handle);
free(lval);
return (-1);
}
if ((lval = malloc(NDMP_PROP_LEN)) == NULL) {
ndmp_smf_scf_fini(handle);
ndmp_errno = ENDMP_MEM_ALLOC;
return (-1);
}
if (ndmp_smf_get_property(handle, prop, lval, NDMP_PROP_LEN) != 0) {
Expand All @@ -145,36 +145,38 @@ ndmp_get_prop(const char *prop, char **value)
int
ndmp_set_prop(const char *env, const char *env_val)
{
ndmp_scfhandle_t *handle = NULL;
ndmp_scfhandle_t *handle;
char *pgname;
int rc;

if ((handle = ndmp_smf_scf_init(NDMP_GROUP_FMRI_PREFIX)) == NULL)
return (-1);

if (ndmp_smf_get_pg_name(handle, env, &pgname)) {
ndmp_smf_scf_fini(handle);
ndmp_errno = ENDMP_SMF_PROP_GRP;
return (-1);
}

if (ndmp_smf_create_service_pgroup(handle, pgname))
if (ndmp_smf_create_service_pgroup(handle, pgname)) {
ndmp_smf_scf_fini(handle);
return (-1);
}

if (ndmp_smf_start_transaction(handle))
if (ndmp_smf_start_transaction(handle)) {
ndmp_smf_scf_fini(handle);
return (-1);

if (env_val) {
if (ndmp_smf_set_property(handle, env, env_val)) {
return (-1);
}
} else {
if (ndmp_smf_delete_property(handle, env))
return (-1);
}

if (ndmp_config_saveenv(handle) != 0)
return (-1);
if (env_val)
rc = ndmp_smf_set_property(handle, env, env_val);
else
rc = ndmp_smf_delete_property(handle, env);

return (0);
if (ndmp_config_saveenv(handle, (rc == 0)) == 0)
return (rc);
else
return (-1);
}

static int
Expand Down Expand Up @@ -218,11 +220,11 @@ ndmp_smf_get_pg_name(ndmp_scfhandle_t *h, const char *pname, char **pgname)
* Basically commit the transaction.
*/
static int
ndmp_config_saveenv(ndmp_scfhandle_t *handle)
ndmp_config_saveenv(ndmp_scfhandle_t *handle, boolean_t commit)
{
int ret = 0;

ret = ndmp_smf_end_transaction(handle);
ret = ndmp_smf_end_transaction(handle, commit);

ndmp_smf_scf_fini(handle);
return (ret);
Expand All @@ -236,15 +238,16 @@ ndmp_config_saveenv(ndmp_scfhandle_t *handle)
static void
ndmp_smf_scf_fini(ndmp_scfhandle_t *handle)
{
if (handle != NULL) {
scf_scope_destroy(handle->scf_scope);
scf_service_destroy(handle->scf_service);
scf_pg_destroy(handle->scf_pg);
handle->scf_state = NDMP_SCH_STATE_UNINIT;
(void) scf_handle_unbind(handle->scf_handle);
scf_handle_destroy(handle->scf_handle);
free(handle);
}
if (handle == NULL)
return;

scf_scope_destroy(handle->scf_scope);
scf_service_destroy(handle->scf_service);
scf_pg_destroy(handle->scf_pg);
handle->scf_state = NDMP_SCH_STATE_UNINIT;
(void) scf_handle_unbind(handle->scf_handle);
scf_handle_destroy(handle->scf_handle);
free(handle);
}

/*
Expand Down Expand Up @@ -381,18 +384,22 @@ ndmp_smf_start_transaction(ndmp_scfhandle_t *handle)
* necessary cleanup.
*/
static int
ndmp_smf_end_transaction(ndmp_scfhandle_t *handle)
ndmp_smf_end_transaction(ndmp_scfhandle_t *handle, boolean_t commit)
{
if (scf_transaction_commit(handle->scf_trans) < 0) {
ndmp_errno = ENDMP_SMF_INTERNAL;
return (-1);
int rc = 0;

if (commit) {
if (scf_transaction_commit(handle->scf_trans) < 0) {
ndmp_errno = ENDMP_SMF_INTERNAL;
rc = -1;
}
}

scf_transaction_destroy_children(handle->scf_trans);
scf_transaction_destroy(handle->scf_trans);
handle->scf_trans = NULL;

return (0);
return (rc);
}

/*
Expand Down Expand Up @@ -431,13 +438,13 @@ ndmp_smf_delete_property(ndmp_scfhandle_t *handle, const char *propname)
* Sets property in current pg
*/
static int
ndmp_smf_set_property(ndmp_scfhandle_t *handle,
const char *propname, const char *valstr)
ndmp_smf_set_property(ndmp_scfhandle_t *handle, const char *propname,
const char *valstr)
{
int ret = 0;
scf_value_t *value = NULL;
scf_transaction_entry_t *entry = NULL;
scf_property_t *prop;
scf_property_t *prop = NULL;
scf_type_t type;
int64_t valint;
uint8_t valbool;
Expand All @@ -446,68 +453,60 @@ ndmp_smf_set_property(ndmp_scfhandle_t *handle,
* Properties must be set in transactions and don't take effect until
* the transaction has been ended/committed.
*/
if (((value = scf_value_create(handle->scf_handle)) != NULL) &&
(entry = scf_entry_create(handle->scf_handle)) != NULL) {
if (((prop =
scf_property_create(handle->scf_handle)) != NULL) &&
((scf_pg_get_property(handle->scf_pg, propname,
prop)) == 0)) {
if (scf_property_get_value(prop, value) == 0) {
type = scf_value_type(value);
if ((scf_transaction_property_change(
handle->scf_trans, entry, propname,
type) == 0) ||
(scf_transaction_property_new(
handle->scf_trans, entry, propname,
type) == 0)) {
switch (type) {
case SCF_TYPE_ASTRING:
if ((scf_value_set_astring(
value,
valstr)) != SCF_SUCCESS)
ret = -1;
break;
case SCF_TYPE_INTEGER:
valint = strtoll(valstr, 0, 0);
scf_value_set_integer(value,
valint);
break;
case SCF_TYPE_BOOLEAN:
if (strncmp(valstr, "yes", 3))
valbool = 0;
else
valbool = 1;
scf_value_set_boolean(value,
valbool);
break;
default:
ret = -1;
}
if (scf_entry_add_value(entry,
value) != 0) {
ret = -1;
scf_value_destroy(value);
}
/* The value is in the transaction */
value = NULL;
}
/* The entry is in the transaction */
entry = NULL;
} else {
ret = -1;
}
} else {
if (((value = scf_value_create(handle->scf_handle)) == NULL) ||
((entry = scf_entry_create(handle->scf_handle)) == NULL) ||
((prop = scf_property_create(handle->scf_handle)) == NULL) ||
(scf_pg_get_property(handle->scf_pg, propname, prop) != 0) ||
(scf_property_get_value(prop, value) != 0)) {
ret = -1;
goto out;
}

type = scf_value_type(value);
if ((scf_transaction_property_change(handle->scf_trans, entry, propname,
type) != 0) &&
(scf_transaction_property_new(handle->scf_trans, entry, propname,
type) != 0)) {
ret = -1;
goto out;
}

switch (type) {
case SCF_TYPE_ASTRING:
if ((scf_value_set_astring(value, valstr)) != SCF_SUCCESS)
ret = -1;
}
break;
case SCF_TYPE_INTEGER:
valint = strtoll(valstr, 0, 0);
scf_value_set_integer(value, valint);
break;
case SCF_TYPE_BOOLEAN:
if (strncmp(valstr, "yes", 3))
valbool = 0;
else
valbool = 1;
scf_value_set_boolean(value, valbool);
break;
default:
ret = -1;
}
if (scf_entry_add_value(entry, value) == 0) {
/* The value is in the transaction */
value = NULL;
} else {
ret = -1;
}
/* The entry is in the transaction */
entry = NULL;

out:
if (ret == -1) {
if ((scf_error() == SCF_ERROR_PERMISSION_DENIED))
ndmp_errno = ENDMP_SMF_PERM;
else
ndmp_errno = ENDMP_SMF_INTERNAL;
}
scf_property_destroy(prop);
scf_value_destroy(value);
scf_entry_destroy(entry);
return (ret);
Expand All @@ -522,8 +521,8 @@ ndmp_smf_get_property(ndmp_scfhandle_t *handle, const char *propname,
char *valstr, size_t sz)
{
int ret = 0;
scf_value_t *value;
scf_property_t *prop;
scf_value_t *value = NULL;
scf_property_t *prop = NULL;
scf_type_t type;
int64_t valint;
uint8_t valbool;
Expand Down

0 comments on commit f093add

Please sign in to comment.