Re: [PATCH] mountd: fix segfault in add_name with newer gcc compilers

[Steve Dickson <SteveD@redhat.com>]

On 04/30/2014 03:39 PM, Jeff Layton wrote:
> I hit a segfault in add_name with a mountd built with gcc-4.9.0. Some
> NULL pointer checks got reordered such that a pointer was dereferenced
> before checking to see whether it was NULL. The problem was due to
> nfs-utils relying on undefined behavior, which tricked gcc into assuming
> that the pointer would never be NULL.
> 
> At first I assumed that this was a compiler bug, but Jakub Jelinek and
> Jeff Law pointed out:
> 
> "If old is NULL, then:
> 
> 	strncpy(new, old, cp-old);
> 
> is undefined behavior (even when cp == old == NULL in that case),
> therefore gcc assumes that old is never NULL, as otherwise it would be
> invalid.
> 
> Just guard
> 	strncpy(new, old, cp-old);
> 	new[cp-old] = 0;
> with if (old) { ... }."
> 
> This patch does that. If old is NULL though, then we still need to
> ensure that new is NULL terminated, lest the subsequent strcats walk off
> the end of it.
> 
> Cc: Jeff Law <law@redhat.com>
> Cc: Jakub Jelinek <jakub@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@poochiereds.net>
Committed...

steved.

> ---
>  support/export/client.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/support/export/client.c b/support/export/client.c
> index dbf47b966522..f85e11c8b535 100644
> --- a/support/export/client.c
> +++ b/support/export/client.c
> @@ -482,8 +482,12 @@ add_name(char *old, const char *add)
>  		else
>  			cp = cp + strlen(cp);
>  	}
> -	strncpy(new, old, cp-old);
> -	new[cp-old] = 0;
> +	if (old) {
> +		strncpy(new, old, cp-old);
> +		new[cp-old] = 0;
> +	} else {
> +		new[0] = 0;
> +	}
>  	if (cp != old && !*cp)
>  		strcat(new, ",");
>  	strcat(new, add);
> -- 1.9.0
>