[dm-devel] [PATCH v2 2/4] multipathd: cli_getprkey(): fix return value

[Benjamin Marzinski]

On Mon, May 17, 2021 at 11:27:25PM +0200, mwilck at suse.com wrote:
> From: Martin Wilck <mwilck at suse.com>
> 
> By setting (*reply)[19] = '\0', we always truncated a possible
> ":aptpl" suffix. Fix it, and use the return value of snprintf()
> as length.
> 
> Signed-off-by: Martin Wilck <mwilck at suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins at redhat.com>
> ---
>  multipathd/cli_handlers.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
> index 9606494..59d44b4 100644
> --- a/multipathd/cli_handlers.c
> +++ b/multipathd/cli_handlers.c
> @@ -1540,7 +1540,7 @@ cli_getprkey(void * v, char ** reply, int * len, void * data)
>  	struct multipath * mpp;
>  	struct vectors * vecs = (struct vectors *)data;
>  	char *mapname = get_keyparam(v, MAP);
> -	char *flagstr = "";
> +	uint64_t key;
>  
>  	mapname = convert_dev(mapname, 0);
>  	condlog(3, "%s: get persistent reservation key (operator)", mapname);
> @@ -1553,17 +1553,16 @@ cli_getprkey(void * v, char ** reply, int * len, void * data)
>  	if (!*reply)
>  		return 1;
>  
> -	if (!get_be64(mpp->reservation_key)) {
> +	key = get_be64(mpp->reservation_key);
> +	if (!key) {
>  		sprintf(*reply, "none\n");
> -		*len = strlen(*reply) + 1;
> +		*len = sizeof("none\n");
>  		return 0;
>  	}
> -	if (mpp->sa_flags & MPATH_F_APTPL_MASK)
> -		flagstr = ":aptpl";
> -	snprintf(*reply, 26, "0x%" PRIx64 "%s\n",
> -		 get_be64(mpp->reservation_key), flagstr);
> -	(*reply)[19] = '\0';
> -	*len = strlen(*reply) + 1;
> +
> +	/* This snprintf() can't overflow - PRIx64 needs max 16 chars */
> +	*len = snprintf(*reply, 26, "0x%" PRIx64 "%s\n", key,
> +			mpp->sa_flags & MPATH_F_APTPL_MASK ? ":aptpl" : "") + 1;
>  	return 0;
>  }
>  
> -- 
> 2.31.1