[PATCH] Fix 'net time' segfault

[Jeremy Allison]

On Fri, Jan 16, 2015 at 06:11:38PM +0100, Michael Adam wrote:
> On 2015-01-16 at 10:59 +0100, Michael Adam wrote:
> > I am looking at the patch, but I'm not convinced yet that the
> > logic is correct. More later...
> 
> My concerns were justified.
> After pair-reviewing/programming for a while,
> we came up with this improved patchset that also
> fixes the underlying problem in cli_connect_nb().
> 
> I'll push later today or tomorrow if noone objects.

LGTM - pushed !


> From 9bcccc35c04172be41e06f42eb8b3918754de8f9 Mon Sep 17 00:00:00 2001
> From: Michael Adam <obnox at samba.org>
> Date: Fri, 16 Jan 2015 16:18:45 +0100
> Subject: [PATCH 1/2] cli_connect_nb_send: don't segfault on host == NULL.
> 
> The functions called futher down can cope with host == NULL.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=11058
> 
> This is part one of the bugfix:
> This ensures that it is enough to pass one of host or address to the function.
> 
> Pair-Programmed-With: Andreas Schneider <asn at samba.org>
> 
> Signed-off-by: Michael Adam <obnox at samba.org>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
>  source3/libsmb/cliconnect.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
> index 7a9e648..9cbf11f 100644
> --- a/source3/libsmb/cliconnect.c
> +++ b/source3/libsmb/cliconnect.c
> @@ -3004,21 +3004,29 @@ static struct tevent_req *cli_connect_nb_send(
>  {
>  	struct tevent_req *req, *subreq;
>  	struct cli_connect_nb_state *state;
> -	char *p;
>  
>  	req = tevent_req_create(mem_ctx, &state, struct cli_connect_nb_state);
>  	if (req == NULL) {
>  		return NULL;
>  	}
> -	state->desthost = host;
>  	state->signing_state = signing_state;
>  	state->flags = flags;
>  
> -	p = strchr(host, '#');
> -	if (p != NULL) {
> -		name_type = strtol(p+1, NULL, 16);
> -		host = talloc_strndup(state, host, p - host);
> -		if (tevent_req_nomem(host, req)) {
> +	if (host != NULL) {
> +		char *p = strchr(host, '#');
> +
> +		if (p != NULL) {
> +			name_type = strtol(p+1, NULL, 16);
> +			host = talloc_strndup(state, host, p - host);
> +			if (tevent_req_nomem(host, req)) {
> +				return tevent_req_post(req, ev);
> +			}
> +		}
> +
> +		state->desthost = host;
> +	} else {
> +		state->desthost = print_canonical_sockaddr(state, dest_ss);
> +		if (tevent_req_nomem(state->desthost, req)) {
>  			return tevent_req_post(req, ev);
>  		}
>  	}
> -- 
> 2.1.0
> 
> 
> From f87a39b99457ec6b3fcdf1cf781aba35ee019529 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Tue, 13 Jan 2015 17:04:26 +0100
> Subject: [PATCH 2/2] utils: Fix 'net time' segfault.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=11058
> 
> This is part two of the bugfix. Make sure we pass the IP we found to
> cli_servertime(). Hence we always pass at least one of name or IP.
> 
> Pair-Programmed-With: Michael Adam <obnox at samba.org>
> 
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Signed-off-by: Michael Adam <obnox at samba.org>
> ---
>  source3/utils/net_time.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/source3/utils/net_time.c b/source3/utils/net_time.c
> index 56ce8f7..595d903 100644
> --- a/source3/utils/net_time.c
> +++ b/source3/utils/net_time.c
> @@ -226,16 +226,23 @@ int net_time(struct net_context *c, int argc, const char **argv)
>  		return 0;
>  	}
>  
> -	if (!c->opt_host && !c->opt_have_ip &&
> -	    !find_master_ip(c->opt_target_workgroup, &c->opt_dest_ip)) {
> -		d_fprintf(stderr, _("Could not locate a time server.  Try "
> -				    "specifying a target host.\n"));
> -		net_time_usage(c, argc,argv);
> -		return -1;
> +	if (c->opt_host == NULL && !c->opt_have_ip) {
> +		bool ok;
> +
> +		ok = find_master_ip(c->opt_target_workgroup, &c->opt_dest_ip);
> +		if (!ok) {
> +			d_fprintf(stderr,
> +				  _("Could not locate a time server.  "
> +				    "Try specifying a target host.\n"));
> +			net_time_usage(c, argc, argv);
> +			return -1;
> +		}
> +		c->opt_have_ip = true;
>  	}
>  
>  	/* default - print the time */
> -	t = cli_servertime(c->opt_host, c->opt_have_ip? &c->opt_dest_ip : NULL,
> +	t = cli_servertime(c->opt_host,
> +			   c->opt_have_ip? &c->opt_dest_ip : NULL,
>  			   NULL);
>  	if (t == 0) return -1;
>  
> -- 
> 2.1.0
>