[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