[PATCH] Fix some Coverity findings
Volker Lendecke
Volker.Lendecke at SerNet.DE
Wed Mar 28 18:56:32 UTC 2018
Hi!
Review appreciated!
Thanks, Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From e9145c2648c13736887a4fb7ae5ede34bb35af13 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 27 Mar 2018 17:00:46 -0500
Subject: [PATCH 1/8] libads: Fix CID 1433606 Dereference before null check
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/libads/kerberos_keytab.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/libads/kerberos_keytab.c b/source3/libads/kerberos_keytab.c
index 85f195abcae..8eb7b2a7c6b 100644
--- a/source3/libads/kerberos_keytab.c
+++ b/source3/libads/kerberos_keytab.c
@@ -93,7 +93,7 @@ static bool fill_default_spns(TALLOC_CTX *ctx, const char *machine_name,
if (*spns == NULL) {
*spns = talloc_zero_array(ctx, const char*, 3);
- if (spns == NULL) {
+ if (*spns == NULL) {
return false;
}
}
--
2.11.0
From 424825c09ad8d4aa0309920daec8410642b3d3dd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 27 Mar 2018 17:03:15 -0500
Subject: [PATCH 2/8] lsa_server: Fix CID 1433608 Dereference after null check
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/rpc_server/lsa/lsa_lookup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/rpc_server/lsa/lsa_lookup.c b/source4/rpc_server/lsa/lsa_lookup.c
index 3baff1ec11f..becbcfc8b64 100644
--- a/source4/rpc_server/lsa/lsa_lookup.c
+++ b/source4/rpc_server/lsa/lsa_lookup.c
@@ -88,7 +88,7 @@ static NTSTATUS dcesrv_lsa_lookup_name(struct lsa_policy_state *state,
bool match = false;
int ret;
- if (principal == NULL && principal[0] == '\0') {
+ if ((principal == NULL) || (principal[0] == '\0')) {
return NT_STATUS_NONE_MAPPED;
}
--
2.11.0
From 7d2f96d8c2e01452968411b17d6cfe627e7122fe Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 28 Mar 2018 07:00:34 -0500
Subject: [PATCH 3/8] libads: Fix 1433611 Resource leak
Not really a memleak due to the passed-in talloc ctx, but this way it's cleaner
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/libads/ldap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
index 78b813c67d0..70166f2f594 100644
--- a/source3/libads/ldap.c
+++ b/source3/libads/ldap.c
@@ -3471,6 +3471,7 @@ out:
if (name != NULL) {
ok = (strlen(name) > 0);
}
+ TALLOC_FREE(name);
return ok;
}
--
2.11.0
From eba5e6b79a208cf2ffb48f9addc2b5ac1160c15a Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 28 Mar 2018 07:10:59 -0500
Subject: [PATCH 4/8] dsdb: Fix CID 1433614 Dereference after null check
This whole routine assumes module!=NULL, both in the successful as
well as in error cases. So checking for module!=NULL is confusing both
the reader as well as Coverity.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/dsdb/samdb/ldb_modules/partition.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/partition.c b/source4/dsdb/samdb/ldb_modules/partition.c
index 422ed369ff5..37e714d6e1b 100644
--- a/source4/dsdb/samdb/ldb_modules/partition.c
+++ b/source4/dsdb/samdb/ldb_modules/partition.c
@@ -1016,7 +1016,8 @@ int partition_del_trans(struct ldb_module *module)
struct partition_private_data);
for (i=0; data && data->partitions && data->partitions[i]; i++) {
- if ((module && ldb_module_flags(ldb_module_get_ctx(module)) & LDB_FLG_ENABLE_TRACING)) {
+ if (ldb_module_flags(ldb_module_get_ctx(module)) &
+ LDB_FLG_ENABLE_TRACING) {
ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_TRACE, "partition_del_trans() -> %s",
ldb_dn_get_linearized(data->partitions[i]->ctrl->dn));
}
@@ -1035,7 +1036,8 @@ int partition_del_trans(struct ldb_module *module)
}
data->in_transaction--;
- if ((module && ldb_module_flags(ldb_module_get_ctx(module)) & LDB_FLG_ENABLE_TRACING)) {
+ if (ldb_module_flags(ldb_module_get_ctx(module)) &
+ LDB_FLG_ENABLE_TRACING) {
ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_TRACE, "partition_del_trans() -> (metadata partition)");
}
ret = ldb_next_del_trans(module);
--
2.11.0
From 0702e5798bfee6b6133a7eadb0d4f7e41e12dd02 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 28 Mar 2018 07:17:59 -0500
Subject: [PATCH 5/8] vfs_fruit: Fix CID 1433613 Operands don't affect result
Two changes: First, we can't check multiplication overflow after the
operation. This has to be done before with a division. Second, there
is no OFF_T_MAX, and both operands are size_t, so check for
SIZE_MAX. The result is assigned to off_t, but I'm not sure where the
automatic coercion from size_t to off_t would happen.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/modules/vfs_fruit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 1a05d0bae34..4299583b21b 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -6515,12 +6515,12 @@ static bool fruit_tmsize_do_dirent(vfs_handle_struct *handle,
return true;
}
- tm_size = bandsize * nbands;
- if (tm_size > UINT64_MAX) {
+ if (bandsize > SIZE_MAX/nbands) {
DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n",
bandsize, nbands);
return false;
}
+ tm_size = bandsize * nbands;
if (state->total_size + tm_size < state->total_size) {
DBG_ERR("tmsize overflow: bandsize [%zu] nbands [%zu]\n",
--
2.11.0
From b16a84221266401d1b601339e24305fe791edfe6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 28 Mar 2018 07:22:02 -0500
Subject: [PATCH 6/8] smbstatus: Fix CID 1128560 Dereference null return value
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/utils/status.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/source3/utils/status.c b/source3/utils/status.c
index beae85c4d3e..6370f7002e8 100644
--- a/source3/utils/status.c
+++ b/source3/utils/status.c
@@ -528,6 +528,7 @@ int main(int argc, const char *argv[])
};
TALLOC_CTX *frame = talloc_stackframe();
int ret = 0;
+ struct tevent_context *ev;
struct messaging_context *msg_ctx = NULL;
char *db_path;
bool ok;
@@ -618,7 +619,14 @@ int main(int argc, const char *argv[])
* This implicitly initializes the global ctdbd connection,
* usable by the db_open() calls further down.
*/
- msg_ctx = messaging_init(NULL, samba_tevent_context_init(NULL));
+ ev = samba_tevent_context_init(NULL);
+ if (ev == NULL) {
+ fprintf(stderr, "samba_tevent_context_init failed\n");
+ ret = -1;
+ goto done;
+ }
+
+ msg_ctx = messaging_init(NULL, ev);
if (msg_ctx == NULL) {
fprintf(stderr, "messaging_init failed\n");
ret = -1;
--
2.11.0
From 5c5cd491e7ea5ea8522ce3578b84160ea24491b9 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 28 Mar 2018 07:24:01 -0500
Subject: [PATCH 7/8] net: Fix CID 1128559 Dereference null return value
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/utils/net.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/source3/utils/net.c b/source3/utils/net.c
index bde261670b5..44daa6088ca 100644
--- a/source3/utils/net.c
+++ b/source3/utils/net.c
@@ -915,6 +915,7 @@ static struct functable net_func[] = {
const char **argv_const = discard_const_p(const char *, argv);
poptContext pc;
TALLOC_CTX *frame = talloc_stackframe();
+ struct tevent_context *ev;
struct net_context *c = talloc_zero(frame, struct net_context);
NTSTATUS status;
@@ -1036,9 +1037,12 @@ static struct functable net_func[] = {
exit(1);
}
- status = messaging_init_client(c,
- samba_tevent_context_init(c),
- &c->msg_ctx);
+ ev = samba_tevent_context_init(c);
+ if (ev == NULL) {
+ d_fprintf(stderr, "samba_tevent_context_init failed\n");
+ exit(1);
+ }
+ status = messaging_init_client(c, ev, &c->msg_ctx);
if (geteuid() != 0 &&
NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
/*
--
2.11.0
From 1069b3f74bd84533ed7f7b9510305d22cba86a6b Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 28 Mar 2018 07:27:08 -0500
Subject: [PATCH 8/8] libads: Fix CID 1349423 Resource leak
get_sorted_dc_list should already take care, but this way it's safer
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/libads/ldap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/libads/ldap.c b/source3/libads/ldap.c
index 70166f2f594..f9463043cc8 100644
--- a/source3/libads/ldap.c
+++ b/source3/libads/ldap.c
@@ -412,7 +412,7 @@ static NTSTATUS resolve_and_ping_dns(ADS_STRUCT *ads, const char *sitename,
const char *realm)
{
int count;
- struct ip_service *ip_list;
+ struct ip_service *ip_list = NULL;
NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
DEBUG(6, ("resolve_and_ping_dns: (cldap) looking for realm '%s'\n",
@@ -421,6 +421,7 @@ static NTSTATUS resolve_and_ping_dns(ADS_STRUCT *ads, const char *sitename,
status = get_sorted_dc_list(realm, sitename, &ip_list, &count,
true);
if (!NT_STATUS_IS_OK(status)) {
+ SAFE_FREE(ip_list);
return status;
}
--
2.11.0
More information about the samba-technical
mailing list