[PATCH] Fix a few CIDs
Jeremy Allison
jra at samba.org
Wed Apr 11 15:52:42 UTC 2018
On Wed, Apr 11, 2018 at 01:12:02PM +0200, Volker Lendecke via samba-technical wrote:
> Hi!
>
> Review appreciated!
LGTM. RB+ and pushed. Thanks !
> --
> 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
> From 3ac467d4a61175d3ea371fe9a03682d43abf0511 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 10 Apr 2018 20:58:11 +0200
> Subject: [PATCH 1/8] tevent: Fix CID 1414792 Unchecked return value
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> lib/tevent/testsuite.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/tevent/testsuite.c b/lib/tevent/testsuite.c
> index e5084521540..63abbf2dc1a 100644
> --- a/lib/tevent/testsuite.c
> +++ b/lib/tevent/testsuite.c
> @@ -375,6 +375,7 @@ static bool test_event_fd1(struct torture_context *tctx,
> const void *test_data)
> {
> struct test_event_fd1_state state;
> + int ret;
>
> ZERO_STRUCT(state);
> state.tctx = tctx;
> @@ -415,7 +416,9 @@ static bool test_event_fd1(struct torture_context *tctx,
> */
> state.sock[0] = -1;
> state.sock[1] = -1;
> - socketpair(AF_UNIX, SOCK_STREAM, 0, state.sock);
> +
> + ret = socketpair(AF_UNIX, SOCK_STREAM, 0, state.sock);
> + torture_assert(tctx, ret == 0, "socketpair() failed");
>
> state.te = tevent_add_timer(state.ev, state.ev,
> timeval_current_ofs(0,1000),
> --
> 2.11.0
>
>
> From 684a0008d49c1910bdedc847e68c58d647f4aa33 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 10 Apr 2018 21:05:09 +0200
> Subject: [PATCH 2/8] vfs_fruit: Fix CID 1416474 Dereference null return value
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/modules/vfs_fruit.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 4299583b21b..0a8141a9e51 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -4065,6 +4065,11 @@ static ssize_t fruit_pread_rsrc(vfs_handle_struct *handle,
> struct fio *fio = (struct fio *)VFS_FETCH_FSP_EXTENSION(handle, fsp);
> ssize_t nread;
>
> + if (fio == NULL) {
> + errno = EINVAL;
> + return -1;
> + }
> +
> switch (fio->config->rsrc) {
> case FRUIT_RSRC_STREAM:
> nread = fruit_pread_rsrc_stream(handle, fsp, data, n, offset);
> --
> 2.11.0
>
>
> From 8310b26b5ace6b24bca8f6ab32d49c8437956ec7 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 10 Apr 2018 21:13:37 +0200
> Subject: [PATCH 3/8] winbind: Fix CID 1427625 Calling risky function
>
> Probably not really a problem, but we have generate_random(), so why not
> use it?
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/winbindd/winbindd_gpupdate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source3/winbindd/winbindd_gpupdate.c b/source3/winbindd/winbindd_gpupdate.c
> index 48ebb5501aa..c86c007be12 100644
> --- a/source3/winbindd/winbindd_gpupdate.c
> +++ b/source3/winbindd/winbindd_gpupdate.c
> @@ -34,7 +34,7 @@
> #define GPUPDATE_RAND_OFFSET (30*60)
> static uint32_t gpupdate_interval(void)
> {
> - int rand_int_offset = rand() % GPUPDATE_RAND_OFFSET;
> + int rand_int_offset = generate_random() % GPUPDATE_RAND_OFFSET;
> return GPUPDATE_INTERVAL+rand_int_offset;
> }
>
> --
> 2.11.0
>
>
> From ca021d7b5b7be1b2b19484dc30d7e5a376de30f7 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 10 Apr 2018 21:18:15 +0200
> Subject: [PATCH 4/8] dnsrpc: Use TALLOC_FREE instead of an explicit
> if-statement
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source4/rpc_server/dnsserver/dnsdata.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c
> index 8080fa480b2..a7b8e74685b 100644
> --- a/source4/rpc_server/dnsserver/dnsdata.c
> +++ b/source4/rpc_server/dnsserver/dnsdata.c
> @@ -219,9 +219,7 @@ int dns_split_name_components(TALLOC_CTX *tmp_ctx, const char *name, char ***com
> return count;
>
> failed:
> - if (str) {
> - talloc_free(str);
> - }
> + TALLOC_FREE(str);
> return -1;
> }
>
> --
> 2.11.0
>
>
> From 190bdc2bb90eac190c2941fc06dca11f2fa4c18c Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Tue, 10 Apr 2018 21:27:47 +0200
> Subject: [PATCH 5/8] smbd: Fix CID 1414783 Double unlock
>
> The loop is unnecessary, both susv4 as well as the Linux manpage
> explicitly say:
>
> > These functions shall not return an error code of [EINTR].
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> source3/smbd/process.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/source3/smbd/process.c b/source3/smbd/process.c
> index df54a44b884..f992e65fc90 100644
> --- a/source3/smbd/process.c
> +++ b/source3/smbd/process.c
> @@ -162,15 +162,9 @@ static bool smbd_unlock_socket_internal(struct smbXsrv_connection *xconn)
>
> #ifdef HAVE_ROBUST_MUTEXES
> if (xconn->smb1.echo_handler.socket_mutex != NULL) {
> - int ret = EINTR;
> -
> - while (ret == EINTR) {
> - ret = pthread_mutex_unlock(
> - xconn->smb1.echo_handler.socket_mutex);
> - if (ret == 0) {
> - break;
> - }
> - }
> + int ret;
> + ret = pthread_mutex_unlock(
> + xconn->smb1.echo_handler.socket_mutex);
> if (ret != 0) {
> DEBUG(1, ("pthread_mutex_unlock failed: %s\n",
> strerror(ret)));
> --
> 2.11.0
>
>
> From 2018cff68a92204d1cbd3cd88a2fd5d751199693 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 11 Apr 2018 08:21:23 +0200
> Subject: [PATCH 6/8] credentials: Revert "credentials: Fix CID 1414796
> Explicit null dereferenced"
>
> This reverts commit 90c02ec64d0e3c860f8d6906cf849bdd2c7bcc54.
>
> We have code to take care of password==NULL, this CID must be fixed in a
> different way
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> auth/credentials/credentials_secrets.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/auth/credentials/credentials_secrets.c b/auth/credentials/credentials_secrets.c
> index 2ae384fdb18..58a87ac9fdb 100644
> --- a/auth/credentials/credentials_secrets.c
> +++ b/auth/credentials/credentials_secrets.c
> @@ -106,11 +106,6 @@ static NTSTATUS cli_credentials_set_secrets_lct(struct cli_credentials *cred,
> }
>
> password = ldb_msg_find_attr_as_string(msg, "secret", NULL);
> - if (password == NULL) {
> - /* This attribute is mandatory */
> - talloc_free(mem_ctx);
> - return NT_STATUS_NOT_FOUND;
> - }
>
> whenChanged = ldb_msg_find_ldb_val(msg, "whenChanged");
> if (!whenChanged || ldb_val_to_time(whenChanged, &lct) != LDB_SUCCESS) {
> --
> 2.11.0
>
>
> From 383b1f07b642368e5d8412e1f919c048d63d23d8 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 11 Apr 2018 08:26:33 +0200
> Subject: [PATCH 7/8] credentials: Fix line length
>
> ... just because I'll modify that line in the next commit
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> auth/credentials/credentials_secrets.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/auth/credentials/credentials_secrets.c b/auth/credentials/credentials_secrets.c
> index 58a87ac9fdb..963024820f3 100644
> --- a/auth/credentials/credentials_secrets.c
> +++ b/auth/credentials/credentials_secrets.c
> @@ -120,7 +120,9 @@ static NTSTATUS cli_credentials_set_secrets_lct(struct cli_credentials *cred,
> return NT_STATUS_NOT_FOUND;
> }
>
> - if (lct == secrets_tdb_last_change_time && secrets_tdb_password && strcmp(password, secrets_tdb_password) != 0) {
> + if ((lct == secrets_tdb_last_change_time) &&
> + (secrets_tdb_password != NULL) &&
> + (strcmp(password, secrets_tdb_password) != 0)) {
> talloc_free(mem_ctx);
> return NT_STATUS_NOT_FOUND;
> }
> --
> 2.11.0
>
>
> From 3278261335125e4f09db3aa6f93c919ac61d1e15 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 11 Apr 2018 08:27:41 +0200
> Subject: [PATCH 8/8] credentials: Fix CID 1414796 Explicit null dereferenced
>
> Signed-off-by: Volker Lendecke <vl at samba.org>
> ---
> auth/credentials/credentials_secrets.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/auth/credentials/credentials_secrets.c b/auth/credentials/credentials_secrets.c
> index 963024820f3..8d2a3b7a46e 100644
> --- a/auth/credentials/credentials_secrets.c
> +++ b/auth/credentials/credentials_secrets.c
> @@ -122,6 +122,7 @@ static NTSTATUS cli_credentials_set_secrets_lct(struct cli_credentials *cred,
>
> if ((lct == secrets_tdb_last_change_time) &&
> (secrets_tdb_password != NULL) &&
> + (password != NULL) &&
> (strcmp(password, secrets_tdb_password) != 0)) {
> talloc_free(mem_ctx);
> return NT_STATUS_NOT_FOUND;
> --
> 2.11.0
>
More information about the samba-technical
mailing list