Re: git: aae67a2c2b66 - main - mfiutil: Fix unsafe assumptions of snprintf(3) return value

On 6/12/25 21:21, Warner Losh wrote:
> The branch main has been updated by imp:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=aae67a2c2b663a6bce8fbc087ff8490336b8618f
> 
> commit aae67a2c2b663a6bce8fbc087ff8490336b8618f
> Author:     WHR <whr@rivoreo.one>
> AuthorDate: 2024-09-03 10:19:04 +0000
> Commit:     Warner Losh <imp@FreeBSD.org>
> CommitDate: 2025-06-13 01:21:44 +0000
> 
>      mfiutil: Fix unsafe assumptions of snprintf(3) return value
>      
>      PR: 281160
>      Reviewed by: imp
>      Pull Request: https://github.com/freebsd/freebsd-src/pull/1405
>      Closes: https://github.com/freebsd/freebsd-src/pull/1405
> ---
>   usr.sbin/mfiutil/mfi_bbu.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/usr.sbin/mfiutil/mfi_bbu.c b/usr.sbin/mfiutil/mfi_bbu.c
> index 9075c4d0ddd0..e97227d47c70 100644
> --- a/usr.sbin/mfiutil/mfi_bbu.c
> +++ b/usr.sbin/mfiutil/mfi_bbu.c
> @@ -50,10 +50,23 @@ mfi_autolearn_period(uint32_t period, char *buf, size_t sz)
>   
>   	tmp = buf;
>   	if (d != 0) {
> -		tmp += snprintf(buf, sz, "%u day%s", d, d == 1 ? "" : "s");
> +		int fmt_len;
> +		fmt_len = snprintf(buf, sz, "%u day%s", d, d == 1 ? "" : "s");
> +		if (fmt_len < 0) {
> +			*buf = 0;
> +			return;
> +		}
> +		if ((size_t)fmt_len >= sz) {
> +			return;
> +		}
> +		tmp += fmt_len;
>   		sz -= tmp - buf;
>   		if (h != 0) {
> -			tmp += snprintf(tmp, sz, ", ");
> +			fmt_len = snprintf(tmp, sz, ", ");
> +			if (fmt_len < 0 || (size_t)fmt_len >= sz) {
> +				return;
> +			}
> +			tmp += fmt_len;
>   			sz -= 2;
>   		}
>   	}

It seems like using a string builder like fmemopen() or sbuf() would be
better here than fragile dances with snprintf().

-- 
John Baldwin