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

Hi Vaibhav,

Thanks for taking a look ...

On 11/05/26 6:40 pm, Vaibhav Jain wrote:
> 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);
>                 ^
> Fix this by copying only the needed suffix from the object class-name to
> 'ppc_cpu_aliases.model' using g_strdup and g_strndup, which maintains the
> constness of the allocated buffer assigned to 'ppc_cpu_aliases.model'
> member of the 'ppc_cpu_aliases' struct array.
> 
> The patch also adds error handling for possible memory allocation failure
> while calling g_str{n}dup functions so that the error is properly
> propogated back from kvm_arch_init().
> 
> Signed-off-by: Amit Machhiwal <[email protected]>
> Signed-off-by: Vaibhav Jain (IBM) <[email protected]>
> 
> ---
> Change-log:
> 
> v1->v2:
> https://lore.kernel.org/all/[email protected]
> * Avoid pointer aliasing
> * Handle memory allocation failure error
> ---
>   target/ppc/kvm.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 25c28ad089..42cd4f4fff 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -170,9 +170,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>   
>       cap_rpt_invalidate = kvm_vm_check_extension(s, KVM_CAP_PPC_RPT_INVALIDATE);
>       cap_ail_mode_3 = kvm_vm_check_extension(s, KVM_CAP_PPC_AIL_MODE_3);
> -    kvm_ppc_register_host_cpu_type();
>   
> -    return 0;
> +    return kvm_ppc_register_host_cpu_type();
>   }
>   
>   int kvm_arch_irqchip_create(KVMState *s)
> @@ -2654,14 +2653,17 @@ 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 *suffix;
> +            const gchar *suffix, *cname = object_class_get_name(oc);
> +
> +            suffix = g_strstr_len(cname, -1, POWERPC_CPU_TYPE_SUFFIX);
> +            ppc_cpu_aliases[i].model = unlikely(suffix) ?
> +                g_strndup(cname, suffix - cname) : g_strdup(cname);
>   
> -            ppc_cpu_aliases[i].model = g_strdup(object_class_get_name(oc));
> -            suffix = strstr(ppc_cpu_aliases[i].model, POWERPC_CPU_TYPE_SUFFIX);
> -            if (suffix) {
> -                *suffix = 0;
> +            if (!ppc_cpu_aliases[i].model) {
> +                return -ENOMEM;

IIUC, g_strndup will abort the program in case of OOM eventually making
it a dead code. We may consider using g_try_strndup at all if needed.
Also, not sure if we really need unlikely() and a typecast may be needed 
for (suffix - cname).

However, can we keep OOM related changes in a separate patch and keep 
only GCC16 build breakage fix in this patch retaining the original patch 
authorship with delta changes attributed alongside respective SoB?

Thanks
Harsh

> +            } else {
> +                return 0;
>               }
> -            break;
>           }
>       }
>