Re: git: 61482760a0ca - main - bhyve: Accept a variable-length string name for qemu_fwcfg_add_file.

On 22 Mar 2023, at 19:35, John Baldwin <jhb@FreeBSD.org> wrote:
> 
> The branch main has been updated by jhb:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=61482760a0ca198a9310d450133e9ac792b67955
> 
> commit 61482760a0ca198a9310d450133e9ac792b67955
> Author:     John Baldwin <jhb@FreeBSD.org>
> AuthorDate: 2023-03-22 19:34:34 +0000
> Commit:     John Baldwin <jhb@FreeBSD.org>
> CommitDate: 2023-03-22 19:34:34 +0000
> 
>    bhyve: Accept a variable-length string name for qemu_fwcfg_add_file.
> 
>    It is illegal (UB?) to pass a shorter array to a function argument
>    that takes a fixed-length array.  Do a runtime check for names that
>    are too long via strlen() instead.

So, without static in there (that very weird corner of the C grammar*),
the size is meaningless. GCC just treats this as a convention that you
meant the size to do something for the purposes of diagnostics, but not
semantics, so this is in fact a known, and by design, false-positive.

Jess

*: void foo(int array[static N])

>    Reviewed by:    markj
>    Reported by:    GCC -Wstringop-overread
>    Differential Revision:  https://reviews.freebsd.org/D39211
> ---
> usr.sbin/bhyve/qemu_fwcfg.c | 6 ++++--
> usr.sbin/bhyve/qemu_fwcfg.h | 2 +-
> 2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/usr.sbin/bhyve/qemu_fwcfg.c b/usr.sbin/bhyve/qemu_fwcfg.c
> index 1b0b5e3e9931..2d91213dc7c5 100644
> --- a/usr.sbin/bhyve/qemu_fwcfg.c
> +++ b/usr.sbin/bhyve/qemu_fwcfg.c
> @@ -261,9 +261,11 @@ qemu_fwcfg_register_port(const char *const name, const int port, const int size,
> }
> 
> int
> -qemu_fwcfg_add_file(const uint8_t name[QEMU_FWCFG_MAX_NAME],
> -    const uint32_t size, void *const data)
> +qemu_fwcfg_add_file(const char *name, const uint32_t size, void *const data)
> {
> +	if (strlen(name) >= QEMU_FWCFG_MAX_NAME)
> +		return (EINVAL);
> +
> 	/*
> 	 * QEMU specifies count as big endian.
> 	 * Convert it to host endian to work with it.
> diff --git a/usr.sbin/bhyve/qemu_fwcfg.h b/usr.sbin/bhyve/qemu_fwcfg.h
> index f59087250816..f3846d64085a 100644
> --- a/usr.sbin/bhyve/qemu_fwcfg.h
> +++ b/usr.sbin/bhyve/qemu_fwcfg.h
> @@ -18,6 +18,6 @@ struct qemu_fwcfg_item {
> 	uint8_t *data;
> };
> 
> -int qemu_fwcfg_add_file(const uint8_t name[QEMU_FWCFG_MAX_NAME],
> +int qemu_fwcfg_add_file(const char *name,
>     const uint32_t size, void *const data);
> int qemu_fwcfg_init(struct vmctx *const ctx);