linux-kernel - Re: [PATCH] x86/fpu: Set the corret permission value for perm.__state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20211109003656.GA642@yangzhon-Virtual>
Date:   Tue, 9 Nov 2021 08:36:56 +0800
From:   Yang Zhong <yang.zhong@...el.com>
To:     "Bae, Chang Seok" <chang.seok.bae@...el.com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, tglx@...utronix.de,
        bp@...e.de, dave.hansen@...el.com, jing2.liu@...el.com,
        yang.zhong@...el.com
Subject: Re: [PATCH] x86/fpu: Set the corret permission value for
 perm.__state_perm

On Tue, Nov 09, 2021 at 12:13:58AM +0800, Bae, Chang Seok wrote:
> On Nov 8, 2021, at 14:28, Zhong, Yang <yang.zhong@...el.com> wrote:
> >
> > The perm.__state_perm is default xsave set features, when we request
> > AMX permission from application, the requested value(bit18) should be
> > replaced with mask value(requested | permitted), which can keep default
> > features there.
> >
> > Without this change, the below prctl syscall:
> > (1). ARCH_GET_XCOMP_PERM, the bitmask=0x202e7
> > (2). set ARCH_REQ_XCOMP_PERM with XFEATURE_XTILEDATA
> > (3). ARCH_GET_XCOMP_PERM, the bitmask=0x40000, the correct value
> >     should be 0x602e7 here.
> 
> Oh, bad.
> 
> > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> > index d28829403ed0..fc1ab0116f4e 100644
> > --- a/arch/x86/kernel/fpu/xstate.c
> > +++ b/arch/x86/kernel/fpu/xstate.c
> > @@ -1626,7 +1626,7 @@ static int __xstate_request_perm(u64 permitted, u64 requested)
> >               return ret;
> >
> >       /* Pairs with the READ_ONCE() in xstate_get_group_perm() */
> > -     WRITE_ONCE(fpu->perm.__state_perm, requested);
> > +     WRITE_ONCE(fpu->perm.__state_perm, mask);
> 
> I think this has to update the group leader’s like:
>     WRITE_ONCE(current->group_leader->thread.fpu.perm.__state_perm, mask);
>

  I think this is no need to change with group leader here, the fpu pointer has defined as below: 
  
  struct fpu *fpu = &current->group_leader->thread.fpu;

  Please check Thomas's changes for KVM, the WRITE_ONCE(fpu->perm.__state_perm, mask) is more reasonable
  here.

  https://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git/commit/?h=x86/fpu-kvm&id=62016e4cc41716b33da890ad43145b646f9db51d

  I also verified this changes from host and guest side, all work well.

  Regards,

  Yang
   


> Thanks,
> Chang
> 
> 
>