The "cpuid_empty" label is also (in principle; maybe only for rubbish
input) reachable in the "cpuid_only" case. Hence the label needs to live
ahead of the check of the variable.

Fixes: 5b80cecb747b ("libxl: introduce MSR data in libxl_cpuid_policy")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -710,10 +710,11 @@ parse_cpuid:
                     libxl__strdup(NOGC, libxl__json_object_get_string(r));
         }
     }
+
+cpuid_empty:
     if (cpuid_only)
         return 0;
 
-cpuid_empty:
     co = libxl__json_map_get("msr", o, JSON_ARRAY);
     if (!libxl__json_object_is_array(co))
         return ERROR_FAIL;

On Tue, Aug 15, 2023 at 02:35:55PM +0200, Jan Beulich wrote:
> The "cpuid_empty" label is also (in principle; maybe only for rubbish
> input) reachable in the "cpuid_only" case. Hence the label needs to live
> ahead of the check of the variable.
> 
> Fixes: 5b80cecb747b ("libxl: introduce MSR data in libxl_cpuid_policy")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

On 15/08/2023 3:26 pm, Anthony PERARD wrote:
> On Tue, Aug 15, 2023 at 02:35:55PM +0200, Jan Beulich wrote:
>> The "cpuid_empty" label is also (in principle; maybe only for rubbish
>> input) reachable in the "cpuid_only" case. Hence the label needs to live
>> ahead of the check of the variable.
>>
>> Fixes: 5b80cecb747b ("libxl: introduce MSR data in libxl_cpuid_policy")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Hmm - this was included in the security backports.  I guess it ought to
go back to 4.15 (now that 4.14 is properly dead).

~Andrew

On 15.08.2023 16:28, Andrew Cooper wrote:
> On 15/08/2023 3:26 pm, Anthony PERARD wrote:
>> On Tue, Aug 15, 2023 at 02:35:55PM +0200, Jan Beulich wrote:
>>> The "cpuid_empty" label is also (in principle; maybe only for rubbish
>>> input) reachable in the "cpuid_only" case. Hence the label needs to live
>>> ahead of the check of the variable.
>>>
>>> Fixes: 5b80cecb747b ("libxl: introduce MSR data in libxl_cpuid_policy")
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Hmm - this was included in the security backports.  I guess it ought to
> go back to 4.15 (now that 4.14 is properly dead).

Depends on whether this is just a theoretical issue, I would say. Can
legacy format input legitimately be empty? If not, I wouldn't bother
putting this onto the security-only branches. (Guess how I found it.
For the 4.13 backport much of this needed quite a bit of adjustment.
Pulling in more stuff from 4.14 isn't really an option, not the least
since I will need to [at least try to] go further back.)

Jan

On Tue, Aug 15, 2023 at 04:44:57PM +0200, Jan Beulich wrote:
> On 15.08.2023 16:28, Andrew Cooper wrote:
> > On 15/08/2023 3:26 pm, Anthony PERARD wrote:
> >> On Tue, Aug 15, 2023 at 02:35:55PM +0200, Jan Beulich wrote:
> >>> The "cpuid_empty" label is also (in principle; maybe only for rubbish
> >>> input) reachable in the "cpuid_only" case. Hence the label needs to live
> >>> ahead of the check of the variable.
> >>>
> >>> Fixes: 5b80cecb747b ("libxl: introduce MSR data in libxl_cpuid_policy")
> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
> > 
> > Hmm - this was included in the security backports.  I guess it ought to
> > go back to 4.15 (now that 4.14 is properly dead).
> 
> Depends on whether this is just a theoretical issue, I would say. Can
> legacy format input legitimately be empty? If not, I wouldn't bother
> putting this onto the security-only branches.

I don't think libxl would generate an empty array.
libxl calls libxl__cpuid_policy_is_empty() before calling
libxl_cpuid_policy_list_gen_json(), so no empty array.

Then, without this fix, I think it would just mean that libxl would
return an error on something that was ok to do before. So backporting
the patch would be a nice to have.

-- 
Anthony PERARD