Re: [PATCH] target/ppc/kvm: Fix const violation when trimming CPU alias suffix

[Vaibhav Jain <vaibhav@linux.ibm.com>]

Hi Amit,

Thanks for reporting this an proposing a fix. My review comments below:


Amit Machhiwal <amachhiw@linux.ibm.com> writes:

> GCC 16 tightens diagnostics around const correctness and now correctly
> rejects attempts to modify strings referenced through const-qualified
> pointers. In kvm_ppc_register_host_cpu_type(), ppc_cpu_aliases[i].model
> is defined as const char *, but the code was using strstr() on it and
> then modifying the returned pointer in-place to strip
> POWERPC_CPU_TYPE_SUFFIX.
>
> This results in a write through a pointer derived from const data,
> triggering a build failure with GCC 16:
>
>   error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
>         suffix = strstr(ppc_cpu_aliases[i].model, POWERPC_CPU_TYPE_SUFFIX);
>                ^
>
This looks more like an aliasing issue for the pointer returned by
g_strdup and it being used between const and non-const contexts.


> Fix this by duplicating the model string into a mutable buffer using
> g_strdup(), storing it in the alias table, and then performing the
> suffix truncation on the mutable copy.
>
> This preserves the existing behavior while avoiding modification of
> const data and ensures compatibility with newer compilers.

I think the better approach would be to avoid modifying the value
returned by g_strdup before its assigned to 'ppc_cpu_alias[i].model'
struct member.

>
> No functional change intended.
>
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> ---
>  target/ppc/kvm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 25c28ad089c6..e71e5c0117da 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2654,10 +2654,12 @@ static int kvm_ppc_register_host_cpu_type(void)
>      dc = DEVICE_CLASS(ppc_cpu_get_family_class(pvr_pcc));
>      for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
>          if (g_ascii_strcasecmp(ppc_cpu_aliases[i].alias, dc->desc) == 0) {
> +            char *model;
>              char *suffix;
>  
> -            ppc_cpu_aliases[i].model = g_strdup(object_class_get_name(oc));
> -            suffix = strstr(ppc_cpu_aliases[i].model, POWERPC_CPU_TYPE_SUFFIX);
> +            model = g_strdup(object_class_get_name(oc));
while we are at it lets also handle ENOMEM error and trickle it back to arch_kvm_init()

> +            ppc_cpu_aliases[i].model = model;
> +            suffix = strstr(model, POWERPC_CPU_TYPE_SUFFIX);
>              if (suffix) {
>                  *suffix = 0;
>              }
>
> base-commit: ac0cc20ad2fe0b8df2e5d9458e90a095ac711ab1
> -- 
> 2.50.1 (Apple Git-155)
>
>

I have done the above proposed changes in a separate patch which i will
post in some time.

-- 
Cheers
~ Vaibhav