Re: [PATCH] Avoid overlapping addresses to stpcpy calls in nscd (BZ #16760)

[Mike Frysinger <vapier at gentoo dot org>]

On Thu 27 Mar 2014 09:34:06 Siddhesh Poyarekar wrote:
> Calls to stpcpy from nscd netgroups code will have overlapping source
> and destination when all three values in the returned triplet are
> non-NULL and in the expected (host,user,domain) order.  This is seen
> in valgrind as:
> 
> ==3181== Source and destination overlap in stpcpy(0x19973b48, 0x19973b48)
> ==3181==    at 0x4C2F30A: stpcpy (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==3181==    by
> 0x12567A: addgetnetgrentX (string3.h:111)
> ==3181==    by 0x12722D: addgetnetgrent (netgroupcache.c:665)
> ==3181==    by 0x11114C: nscd_run_worker (connections.c:1338)
> ==3181==    by 0x4E3C102: start_thread (pthread_create.c:309)
> ==3181==    by 0x59B81AC: clone (clone.S:111)
> ==3181==
> 
> Fix this by using memmove instead of stpcpy.  Tested x86_64 using
> various combinations of triplets (including NULL and non-NULL ones) to
> verify that this works correctly and there are no regressions.

i feel like we've wanted an equivalent of stpcpy/memccpy for memmove.  good 
time to add it ? :)

> +			    size_t hostlen = strlen (nhost ?: "") + 1;
> +			    size_t userlen = strlen (nuser ?: "") + 1;
> +			    size_t domainlen = strlen (ndomain ?: "") + 1;

we do the ?: thing a lot in this code.  time to assign a local var for it ?

>  			    char *wp = buffer + buffilled;
> -			    wp = stpcpy (wp, nhost) + 1;
> -			    wp = stpcpy (wp, nuser) + 1;
> -			    wp = stpcpy (wp, ndomain) + 1;
> +			    wp = memmove (wp, nhost ?: "", hostlen);
> +			    wp += hostlen;
> +			    wp = memmove (wp, nuser ?: "", userlen);
> +			    wp += userlen;
> +			    wp = memmove (wp, ndomain ?: "", domainlen);
> +			    wp += domainlen;

looks OK
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.