[CRIU] [PATCH] arch/x86: Fix calculation of xstate_size

[Dmitry Safonov]

On 5/31/20 10:55 AM, Andrei Vagin wrote:
> Hi Cyrill and Dmitry,
> 
> Dima, could you review this patch?
> Cyrill, could you create a pull-request on github?

The patch looks reasonable to me.
I don't like introducing new BUG_ON's, but that could be addressed on
the top.

> 
> Thanks,
> Andrei
> 
> On Mon, May 18, 2020 at 1:19 AM Cyrill Gorcunov <gorcunov at gmail.com> wrote:
>>
>> The layout of xsave frame in a standart format is predefined by the hardware.
>> Lets make sure we're increasing in frame offsets and use latest offset where
>> appropriate.
>>
>> https://github.com/checkpoint-restore/criu/issues/1042
>>
>> Reported-by: Ashutosh Mehra <mehra.ashutosh at ibm.com>
>> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>

Reviewed-by: Dmitry Safonov <0x7f454c46 at gmail.com>

>> ---
>>  criu/arch/x86/crtools.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/criu/arch/x86/crtools.c b/criu/arch/x86/crtools.c
>> index 9c8beeedddf0..bc80225358ff 100644
>> --- a/criu/arch/x86/crtools.c
>> +++ b/criu/arch/x86/crtools.c
>> @@ -437,6 +437,7 @@ int restore_fpu(struct rt_sigframe *sigframe, CoreEntry *core)
>>                         void *from = xsave->member;                                     \
>>                         size_t size = pb_repeated_size(xsave, member);                  \
>>                         size_t xsize = (size_t)compel_fpu_feature_size(feature);        \
>> +                       size_t xstate_size_next = off + xsize;                          \
>>                         if (xsize != size) {                                            \
>>                                 if (size) {                                             \
>>                                         pr_err("%s reported %zu bytes (expecting %zu)\n",\
>> @@ -448,7 +449,8 @@ int restore_fpu(struct rt_sigframe *sigframe, CoreEntry *core)
>>                                 }                                                       \
>>                         }                                                               \
>>                         xstate_bv |= (1UL << feature);                                  \
>> -                       xstate_size += xsize;                                           \
>> +                       BUG_ON(xstate_size > xstate_size_next);                         \
>> +                       xstate_size = xstate_size_next;                                 \
>>                         memcpy(to, from, size);                                         \
>>                 }                                                                       \
>>         } while (0)
>> @@ -485,6 +487,11 @@ int restore_fpu(struct rt_sigframe *sigframe, CoreEntry *core)
>>                         UserX86XsaveEntry *xsave = core->thread_info->fpregs->xsave;
>>                         uint8_t *extended_state_area = (void *)x;
>>
>> +                       /*
>> +                        * Note the order does matter here and bound
>> +                        * to the increasing offsets of XFEATURE_x
>> +                        * inside memory layout (xstate_size calculation).
>> +                        */
>>                         assign_xsave(XFEATURE_YMM,      xsave, ymmh_space,      extended_state_area);
>>                         assign_xsave(XFEATURE_BNDREGS,  xsave, bndreg_state,    extended_state_area);
>>                         assign_xsave(XFEATURE_BNDCSR,   xsave, bndcsr_state,    extended_state_area);
>> --
>> 2.26.2
>>

Thanks,
          Dima