From 0b5cbcf45f3fb4b03a1f762c5704183787d30696 Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Fri, 12 Jan 2018 08:38:22 -0500
Subject: [PATCH] Ticket 49529 - Fix Coverity warnings: invalid deferences
Description: So many of the warnings were false positives, but
I "fixed" 90% of them anyway for these two reasons:
One, it's possible that a future change could actually
result in a NULL pointer being referenced.
Two, it would be nice to stop these coverity warnings
so we can focus on real warnings. Auto waivers also
don't always work as the surrounding code changes.
https://pagure.io/389-ds-base/issue/49529
Reviewed by: firstyear (Thanks!)
(cherry picked from commit 7e27face5ef021d883a44d70bb3e9732b115016f)
---
ldap/servers/slapd/abandon.c | 10 ++++++++--
ldap/servers/slapd/add.c | 18 +++++++++++++++---
ldap/servers/slapd/bind.c | 20 +++++++++++++++-----
ldap/servers/slapd/compare.c | 17 +++++++++++++----
ldap/servers/slapd/connection.c | 19 +++++++++++++------
ldap/servers/slapd/delete.c | 4 ++--
ldap/servers/slapd/dn.c | 7 +++++++
ldap/servers/slapd/entry.c | 10 +++++++++-
ldap/servers/slapd/extendop.c | 7 +++++++
ldap/servers/slapd/filter.c | 6 +++++-
ldap/servers/slapd/modify.c | 18 ++++++++++++++++--
ldap/servers/slapd/passwd_extop.c | 4 ++++
ldap/servers/slapd/psearch.c | 13 +++++++++----
ldap/servers/slapd/result.c | 14 +++++++++++++-
ldap/servers/slapd/search.c | 5 ++++-
ldap/servers/slapd/task.c | 5 +++++
16 files changed, 145 insertions(+), 32 deletions(-)
diff --git a/ldap/servers/slapd/abandon.c b/ldap/servers/slapd/abandon.c
index 5c30c972d..e2237e5fc 100644
--- a/ldap/servers/slapd/abandon.c
+++ b/ldap/servers/slapd/abandon.c
@@ -42,10 +42,16 @@ do_abandon(Slapi_PBlock *pb)
slapi_pblock_get(pb, SLAPI_OPERATION, &pb_op);
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
- BerElement *ber = pb_op->o_ber;
-
slapi_log_err(SLAPI_LOG_TRACE, "do_abandon", "->\n");
+ if (pb_op == NULL || pb_conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "do_abandon", "NULL param: pb_conn (0x%p) pb_op (0x%p)\n",
+ pb_conn, pb_op);
+ return;
+ }
+
+ BerElement *ber = pb_op->o_ber;
+
/*
* Parse the abandon request. It looks like this:
*
diff --git a/ldap/servers/slapd/add.c b/ldap/servers/slapd/add.c
index 0a4a5d7b2..8f2fdeac8 100644
--- a/ldap/servers/slapd/add.c
+++ b/ldap/servers/slapd/add.c
@@ -66,6 +66,14 @@ do_add(Slapi_PBlock *pb)
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
slapi_pblock_get(pb, SLAPI_OPERATION, &operation);
+
+
+ if (operation == NULL || pb_conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "do_add", "NULL param: pb_conn (0x%p) pb_op (0x%p)\n",
+ pb_conn, operation);
+ send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, "param error", 0, NULL);
+ return;
+ }
ber = operation->o_ber;
/* count the add request */
@@ -450,8 +458,8 @@ op_shared_add(Slapi_PBlock *pb)
if (!internal_op) {
slapi_log_access(LDAP_DEBUG_STATS, "conn=%" PRIu64 " op=%d ADD dn=\"%s\"%s\n",
- pb_conn->c_connid,
- operation->o_opid,
+ pb_conn ? pb_conn->c_connid : -1,
+ operation ? operation->o_opid: -1,
slapi_entry_get_dn_const(e),
proxystr ? proxystr : "");
} else {
@@ -865,7 +873,11 @@ handle_fast_add(Slapi_PBlock *pb, Slapi_Entry *entry)
int ret;
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
-
+ if (pb_conn == NULL){
+ slapi_log_err(SLAPI_LOG_ERR, "handle_fast_add", "NULL param: pb_conn (0x%p)\n", pb_conn);
+ send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, "param error", 0, NULL);
+ return;
+ }
be = pb_conn->c_bi_backend;
if ((be == NULL) || (be->be_wire_import == NULL)) {
diff --git a/ldap/servers/slapd/bind.c b/ldap/servers/slapd/bind.c
index 4a8e4deaf..a34a21a77 100644
--- a/ldap/servers/slapd/bind.c
+++ b/ldap/servers/slapd/bind.c
@@ -54,11 +54,7 @@ do_bind(Slapi_PBlock *pb)
{
Operation *pb_op = NULL;
Connection *pb_conn = NULL;
-
- slapi_pblock_get(pb, SLAPI_OPERATION, &pb_op);
- slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
-
- BerElement *ber = pb_op->o_ber;
+ BerElement *ber;
int err, isroot;
ber_tag_t method = LBER_DEFAULT;
ber_int_t version = -1;
@@ -83,6 +79,16 @@ do_bind(Slapi_PBlock *pb)
slapi_log_err(SLAPI_LOG_TRACE, "do_bind", "=>\n");
+ slapi_pblock_get(pb, SLAPI_OPERATION, &pb_op);
+ slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
+ if (pb_op == NULL || pb_conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "do_bind", "NULL param: pb_conn (0x%p) pb_op (0x%p)\n",
+ pb_conn, pb_op);
+ send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, NULL, 0, NULL);
+ goto free_and_return;
+ }
+ ber = pb_op->o_ber;
+
/*
* Parse the bind request. It looks like this:
*
@@ -856,6 +862,10 @@ log_bind_access(
slapi_pblock_get(pb, SLAPI_OPERATION, &pb_op);
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
+ if (pb_op == NULL || pb_conn == NULL) {
+ return;
+ }
+
if (method == LDAP_AUTH_SASL && saslmech && msg) {
slapi_log_access(LDAP_DEBUG_STATS,
"conn=%" PRIu64 " op=%d BIND dn=\"%s\" "
diff --git a/ldap/servers/slapd/compare.c b/ldap/servers/slapd/compare.c
index 9bc6b693a..2626d91d0 100644
--- a/ldap/servers/slapd/compare.c
+++ b/ldap/servers/slapd/compare.c
@@ -35,10 +35,7 @@ do_compare(Slapi_PBlock *pb)
{
Operation *pb_op = NULL;
Connection *pb_conn = NULL;
- slapi_pblock_get(pb, SLAPI_OPERATION, &pb_op);
- slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
-
- BerElement *ber = pb_op->o_ber;
+ BerElement *ber;
char *rawdn = NULL;
const char *dn = NULL;
struct ava ava = {0};
@@ -50,6 +47,18 @@ do_compare(Slapi_PBlock *pb)
slapi_log_err(SLAPI_LOG_TRACE, "do_compare", "=>\n");
+ slapi_pblock_get(pb, SLAPI_OPERATION, &pb_op);
+ slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
+
+ if (pb_op == NULL || pb_conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "do_compare", "NULL param: pb_conn (0x%p) pb_op (0x%p)\n",
+ pb_conn, pb_op);
+ send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, NULL, 0, NULL);
+ goto free_and_return;
+ }
+
+ ber = pb_op->o_ber;
+
/* count the compare request */
slapi_counter_increment(g_get_global_snmp_vars()->ops_tbl.dsCompareOps);
diff --git a/ldap/servers/slapd/connection.c b/ldap/servers/slapd/connection.c
index 8ef115691..fa24ec040 100644
--- a/ldap/servers/slapd/connection.c
+++ b/ldap/servers/slapd/connection.c
@@ -1518,7 +1518,7 @@ connection_threadmain()
}
if (!thread_turbo_flag && !more_data) {
- Connection *pb_conn = NULL;
+ Connection *pb_conn = NULL;
/* If more data is left from the previous connection_read_operation,
we should finish the op now. Client might be thinking it's
@@ -1530,6 +1530,13 @@ connection_threadmain()
* Connection wait for new work provides the conn and op for us.
*/
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
+ if (pb_conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "connection_threadmain",
+ "pb_conn is NULL\n");
+ slapi_pblock_destroy(pb);
+ g_decr_active_threadcnt();
+ return;
+ }
switch (ret) {
case CONN_NOWORK:
@@ -1702,11 +1709,11 @@ connection_threadmain()
* so need locking from here on */
signal_listner();
/* with nunc-stans, I see an enormous amount of time spent in the poll() in
- * connection_read_operation() when the below code is enabled - not sure why
- * nunc-stans makes such a huge difference - for now, just disable this code
- * when using nunc-stans - it is supposed to be an optimization but turns out
- * to not be the opposite with nunc-stans
- */
+ * connection_read_operation() when the below code is enabled - not sure why
+ * nunc-stans makes such a huge difference - for now, just disable this code
+ * when using nunc-stans - it is supposed to be an optimization but turns out
+ * to not be the opposite with nunc-stans
+ */
} else if (!enable_nunc_stans) { /* more data in conn - just put back on work_q - bypass poll */
bypasspollcnt++;
PR_EnterMonitor(conn->c_mutex);
diff --git a/ldap/servers/slapd/delete.c b/ldap/servers/slapd/delete.c
index ba238b18f..49cdab138 100644
--- a/ldap/servers/slapd/delete.c
+++ b/ldap/servers/slapd/delete.c
@@ -262,8 +262,8 @@ op_shared_delete(Slapi_PBlock *pb)
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
slapi_pblock_get(pb, SLAPI_OPERATION, &pb_op);
slapi_log_access(LDAP_DEBUG_STATS, "conn=%" PRIu64 " op=%d DEL dn=\"%s\"%s\n",
- pb_conn->c_connid,
- pb_op->o_opid,
+ pb_conn ? pb_conn->c_connid : -1,
+ pb_op ? pb_op->o_opid : -1,
slapi_sdn_get_dn(sdn),
proxystr ? proxystr : "");
} else {
diff --git a/ldap/servers/slapd/dn.c b/ldap/servers/slapd/dn.c
index afca37214..abc155533 100644
--- a/ldap/servers/slapd/dn.c
+++ b/ldap/servers/slapd/dn.c
@@ -2477,6 +2477,13 @@ slapi_sdn_copy(const Slapi_DN *from, Slapi_DN *to)
{
SDN_DUMP(from, "slapi_sdn_copy from");
SDN_DUMP(to, "slapi_sdn_copy to");
+
+ if (to == NULL || from == NULL){
+ slapi_log_err(SLAPI_LOG_ERR, "slapi_sdn_copy",
+ "NULL param: from (0x%p) to (0x%p)\n", from, to);
+ return;
+ }
+
slapi_sdn_done(to);
if (from->udn) {
to->flag = slapi_setbit_uchar(to->flag, FLAG_UDN);
diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c
index fbbc8faa0..32828b4e2 100644
--- a/ldap/servers/slapd/entry.c
+++ b/ldap/servers/slapd/entry.c
@@ -1998,6 +1998,10 @@ slapi_entry_dup(const Slapi_Entry *e)
struct attrs_in_extension *aiep;
PR_ASSERT(NULL != e);
+ if (e == NULL){
+ slapi_log_err(SLAPI_LOG_ERR, "slapi_entry_dup", "entry is NULL\n");
+ return NULL;
+ }
ec = slapi_entry_alloc();
@@ -3660,7 +3664,11 @@ delete_values_sv_internal(
Slapi_Attr *a;
int retVal = LDAP_SUCCESS;
-/*
+ if (e == NULL){
+ slapi_log_err(SLAPI_LOG_ERR, "delete_values_sv_internal", "entry is NULL\n");
+ return LDAP_OPERATIONS_ERROR;
+ }
+ /*
* If type is in the protected_attrs_all list, we could ignore the failure,
* as the attribute could only exist in the entry in the memory when the
* add/mod operation is done, while the retried entry from the db does not
diff --git a/ldap/servers/slapd/extendop.c b/ldap/servers/slapd/extendop.c
index 1594a8c9c..815949be6 100644
--- a/ldap/servers/slapd/extendop.c
+++ b/ldap/servers/slapd/extendop.c
@@ -219,6 +219,13 @@ do_extended(Slapi_PBlock *pb)
slapi_pblock_get(pb, SLAPI_OPERATION, &pb_op);
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
+ if (pb_conn == NULL || pb_op == NULL) {
+ send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL, "param error", 0, NULL);
+ slapi_log_err(SLAPI_LOG_ERR, "do_extended",
+ "NULL param error: conn (0x%p) op (0x%p)\n", pb_conn, pb_op);
+ goto free_and_return;
+ }
+
/*
* Parse the extended request. It looks like this:
*
diff --git a/ldap/servers/slapd/filter.c b/ldap/servers/slapd/filter.c
index fe3525f34..ef975e679 100644
--- a/ldap/servers/slapd/filter.c
+++ b/ldap/servers/slapd/filter.c
@@ -292,7 +292,11 @@ get_filter_internal(Connection *conn, BerElement *ber, struct slapi_filter **fil
case LDAP_FILTER_EXTENDED:
slapi_log_err(SLAPI_LOG_FILTER, "get_filter_internal", "EXTENDED\n");
- if (conn->c_ldapversion < 3) {
+ if (conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "get_filter_internal",
+ "NULL param: conn (0x%p)\n", conn);
+ err = LDAP_OPERATIONS_ERROR;
+ } else if (conn->c_ldapversion < 3) {
slapi_log_err(SLAPI_LOG_ERR, "get_filter_internal",
"Extensible filter received from v2 client\n");
err = LDAP_PROTOCOL_ERROR;
diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c
index 0dcac646b..10d263159 100644
--- a/ldap/servers/slapd/modify.c
+++ b/ldap/servers/slapd/modify.c
@@ -122,9 +122,16 @@ do_modify(Slapi_PBlock *pb)
slapi_log_err(SLAPI_LOG_TRACE, "do_modify", "=>\n");
slapi_pblock_get(pb, SLAPI_OPERATION, &operation);
- ber = operation->o_ber;
-
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
+ if (operation == NULL) {
+ send_ldap_result(pb, LDAP_OPERATIONS_ERROR,
+ NULL, "operation is NULL parameter", 0, NULL);
+ slapi_log_err(SLAPI_LOG_ERR, "do_modify",
+ "NULL param: pb_conn (0x%p) operation (0x%p)\n", pb_conn, operation);
+ return;
+ }
+
+ ber = operation->o_ber;
/* count the modify request */
slapi_counter_increment(g_get_global_snmp_vars()->ops_tbl.dsModifyEntryOps);
@@ -1165,6 +1172,13 @@ op_shared_allow_pw_change(Slapi_PBlock *pb, LDAPMod *mod, char **old_pw, Slapi_M
internal_op = operation_is_flag_set(operation, OP_FLAG_INTERNAL);
slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
+ if (pb_conn == NULL || operation == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "op_shared_allow_pw_change",
+ "NULL param error: conn (0x%p) op (0x%p)\n", pb_conn, operation);
+ rc = -1;
+ goto done;
+ }
+
slapi_sdn_init_dn_byref(&sdn, dn);
pwpolicy = new_passwdPolicy(pb, (char *)slapi_sdn_get_ndn(&sdn));
diff --git a/ldap/servers/slapd/passwd_extop.c b/ldap/servers/slapd/passwd_extop.c
index 54a9a6716..40145af2e 100644
--- a/ldap/servers/slapd/passwd_extop.c
+++ b/ldap/servers/slapd/passwd_extop.c
@@ -486,6 +486,10 @@ passwd_modify_extop(Slapi_PBlock *pb)
/* Allow password modify only for SSL/TLS established connections and
* connections using SASL privacy layers */
slapi_pblock_get(pb, SLAPI_CONNECTION, &conn);
+ if (conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "passwd_modify_extop", "conn is NULL");
+ goto free_and_return;
+ }
if (slapi_pblock_get(pb, SLAPI_CONN_SASL_SSF, &sasl_ssf) != 0) {
errMesg = "Could not get SASL SSF from connection\n";
rc = LDAP_OPERATIONS_ERROR;
diff --git a/ldap/servers/slapd/psearch.c b/ldap/servers/slapd/psearch.c
index e0dd2bf89..1bf062954 100644
--- a/ldap/servers/slapd/psearch.c
+++ b/ldap/servers/slapd/psearch.c
@@ -271,6 +271,11 @@ ps_send_results(void *arg)
slapi_pblock_get(ps->ps_pblock, SLAPI_CONNECTION, &pb_conn);
slapi_pblock_get(ps->ps_pblock, SLAPI_OPERATION, &pb_op);
+ if (pb_conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "ps_send_results", "pb_conn is NULL\n");
+ return;
+ }
+
/* need to acquire a reference to this connection so that it will not
be released or cleaned up out from under us */
PR_EnterMonitor(pb_conn->c_mutex);
@@ -280,7 +285,7 @@ ps_send_results(void *arg)
if (conn_acq_flag) {
slapi_log_err(SLAPI_LOG_CONNS, "ps_send_results",
"conn=%" PRIu64 " op=%d Could not acquire the connection - psearch aborted\n",
- pb_conn->c_connid, pb_op->o_opid);
+ pb_conn->c_connid, pb_op ? pb_op->o_opid : -1);
}
PR_Lock(psearch_list->pl_cvarlock);
@@ -290,7 +295,7 @@ ps_send_results(void *arg)
if (pb_op == NULL || slapi_op_abandoned(ps->ps_pblock)) {
slapi_log_err(SLAPI_LOG_CONNS, "ps_send_results",
"conn=%" PRIu64 " op=%d The operation has been abandoned\n",
- pb_conn->c_connid, pb_op->o_opid);
+ pb_conn->c_connid, pb_op ? pb_op->o_opid : -1);
break;
}
if (NULL == ps->ps_eq_head) {
@@ -532,7 +537,7 @@ ps_service_persistent_searches(Slapi_Entry *e, Slapi_Entry *eprev, ber_int_t chg
slapi_log_err(SLAPI_LOG_CONNS, "ps_service_persistent_searches",
"conn=%" PRIu64 " op=%d entry %s with chgtype %d "
"matches the ps changetype %d\n",
- pb_conn->c_connid,
+ pb_conn ? pb_conn->c_connid : -1,
pb_op->o_opid,
edn, chgtype, ps->ps_changetypes);
@@ -609,7 +614,7 @@ ps_service_persistent_searches(Slapi_Entry *e, Slapi_Entry *eprev, ber_int_t chg
/* Turn 'em loose */
ps_wakeup_all();
slapi_log_err(SLAPI_LOG_TRACE, "ps_service_persistent_searches", "Enqueued entry "
- "\"%s\" on %d persistent search lists\n",
+ "\"%s\" on %d persistent search lists\n",
slapi_entry_get_dn_const(e), matched);
} else {
slapi_log_err(SLAPI_LOG_TRACE, "ps_service_persistent_searches",
diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c
index 2302ae96b..ce394d948 100644
--- a/ldap/servers/slapd/result.c
+++ b/ldap/servers/slapd/result.c
@@ -396,7 +396,7 @@ send_ldap_result_ext(
break;
case LDAP_REFERRAL:
- if (conn->c_ldapversion > LDAP_VERSION2) {
+ if (conn && conn->c_ldapversion > LDAP_VERSION2) {
tag = LDAP_TAG_REFERRAL;
break;
}
@@ -645,6 +645,11 @@ process_read_entry_controls(Slapi_PBlock *pb, char *oid)
BerElement *req_ber = NULL;
Operation *op = NULL;
slapi_pblock_get(pb, SLAPI_OPERATION, &op);
+ if (op == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "process_read_entry_controls", "op is NULL\n");
+ rc = -1;
+ goto done;
+ }
if (strcmp(oid, LDAP_CONTROL_PRE_READ_ENTRY) == 0) {
/* first verify this is the correct operation for a pre-read entry control */
@@ -2145,6 +2150,13 @@ encode_read_entry(Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluseratt
slapi_pblock_get(pb, SLAPI_OPERATION, &op);
slapi_pblock_get(pb, SLAPI_CONNECTION, &conn);
+ if (conn == NULL || op == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "encode_read_entry",
+ "NULL param error: conn (0x%p) op (0x%p)\n", conn, op);
+ rc = -1;
+ goto cleanup;
+ }
+
/* Start the ber encoding with the DN */
rc = ber_printf(ber, "t{s{", LDAP_RES_SEARCH_ENTRY, slapi_entry_get_dn_const(e));
if (rc == -1) {
diff --git a/ldap/servers/slapd/search.c b/ldap/servers/slapd/search.c
index 5e3413245..731c6519e 100644
--- a/ldap/servers/slapd/search.c
+++ b/ldap/servers/slapd/search.c
@@ -125,7 +125,10 @@ do_search(Slapi_PBlock *pb)
goto free_and_return;
}
- slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn);
+ if (slapi_pblock_get(pb, SLAPI_CONNECTION, &pb_conn) != 0 || pb_conn == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "do_search", "pb_conn is NULL\n");
+ goto free_and_return;
+ }
/*
* If nsslapd-minssf-exclude-rootdse is on, the minssf check has been
diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c
index 53a0af52d..002083c04 100644
--- a/ldap/servers/slapd/task.c
+++ b/ldap/servers/slapd/task.c
@@ -199,6 +199,11 @@ slapi_task_log_status(Slapi_Task *task, char *format, ...)
{
va_list ap;
+ if (task == NULL) {
+ slapi_log_err(SLAPI_LOG_ERR, "slapi_task_log_status",
+ "Slapi_Task is NULL, can not log status\n");
+ return;
+ }
if (!task->task_status)
task->task_status = (char *)slapi_ch_malloc(10 * LOG_BUFFER);
if (!task->task_status)
--
2.13.6