Re: [PATCH 4/7] doc/migration: Use strncpy() for socket path and fix argv access

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2765 bytes --]

On Wed, Jul 08, 2026 at 06:32:00PM -0400, Jon Maloy wrote:
> Replace strcpy() with strncpy() when copying the helper socket path
> into sun_path, preventing a potential buffer overflow from a long
> argument.
> 
> In target.c, move the argc check before accessing argv[4], so we
> don't dereference past the end of argv.

This fixes real bugs in the examples, but..

strncpy() does not guarantee the target is \0-terminated.  In this
case we will end up \0-terminated, because we limit the strncpy() to
exclude the last byte of the target buffer, which is initialized to
all 0s.  That's rather more riskly subtle than I'd like.

In the passt code proper we usually use snprintf() rather than
strncpy() to move an arbitrary length string into a fixed sized buffer
like (including sun_path in several cases).  It's strictly speaking
more heavyweight than we need, but the meaning is pretty clear and it
guarantees \0 termination.  I'd suggest using that instead.

> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>  doc/migration/source.c | 2 +-
>  doc/migration/target.c | 8 +++-----
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/migration/source.c b/doc/migration/source.c
> index d44ebf1f..77ff8fda 100644
> --- a/doc/migration/source.c
> +++ b/doc/migration/source.c
> @@ -47,7 +47,7 @@ int main(int argc, char **argv)
>  		return -1;
>  	}
>  
> -	strcpy(a_helper.sun_path, argv[4]);
> +	strncpy(a_helper.sun_path, argv[4], sizeof(a_helper.sun_path) - 1);
>  	getaddrinfo(argv[1], argv[2], &hints, &r);
>  
>  	/* Connect socket to server and send some data */
> diff --git a/doc/migration/target.c b/doc/migration/target.c
> index f7d31083..0eb1f6ac 100644
> --- a/doc/migration/target.c
> +++ b/doc/migration/target.c
> @@ -38,11 +38,6 @@ int main(int argc, char **argv)
>  	struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
>  	struct addrinfo *r;
>  
> -	(void)argc;
> -
> -	strcpy(a_helper.sun_path, argv[4]);
> -	getaddrinfo(argv[1], argv[2], &hints, &r);
> -
>  	if (argc != 7) {
>  		fprintf(stderr,
>  			"%s DST_ADDR DST_PORT SRC_PORT HELPER_PATH SSEQ RSEQ\n",
> @@ -50,6 +45,9 @@ int main(int argc, char **argv)
>  		return -1;
>  	}
>  
> +	strncpy(a_helper.sun_path, argv[4], sizeof(a_helper.sun_path) - 1);
> +	getaddrinfo(argv[1], argv[2], &hints, &r);
> +
>  	/* Prepare socket, bind to source port */
>  	s = socket(r->ai_family, SOCK_STREAM, IPPROTO_TCP);
>  	setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &((int){ 1 }), sizeof(int));
> -- 
> 2.52.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]