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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 05 Oct 2021 11:49:05 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Chang S. Bae" <chang.seok.bae@...el.com>, bp@...e.de,
        luto@...nel.org, mingo@...nel.org, x86@...nel.org
Cc:     len.brown@...el.com, lenb@...nel.org, dave.hansen@...el.com,
        thiago.macieira@...el.com, jing2.liu@...el.com,
        ravi.v.shankar@...el.com, linux-kernel@...r.kernel.org,
        chang.seok.bae@...el.com
Subject: Re: [PATCH v11 15/29] x86/arch_prctl: Create
 ARCH_SET_STATE_ENABLE/ARCH_GET_STATE_ENABLE

On Tue, Oct 05 2021 at 02:30, Thomas Gleixner wrote:
> On Fri, Oct 01 2021 at 15:37, Chang S. Bae wrote:
>> +		if (state_perm & ~features_mask)
>> +			return -EINVAL;
>> +
>> +		state_perm &= xfeatures_mask_user_perm();
>> +		if (!state_perm)
>> +			return 0;
>
> I really do not get the semantics of this prctl at all.
>
> GET stores _all_ permitted bits in the user space variable which makes
> sense.
>
> SET is just accepting everything except not supported bits, but as it
> takes a feature bitmask it suggests that this sets the xfeature bits
> which are available for the task or the process.
>
> How does prctl(..., SET, 0) make sense?
>
> It does not make any sense at all. There is no support for downgrading
> the permitted features:
>
>     1) Default features up to AVX512 cannot be disabled
>     
>     2) Once AMX (or any upcoming state) is enabled there is not way back.
>
> So no. This really want's to be
>
>    prctl(SET, xfeature_number)
>
> and not something which is semanticaly ill defined.

So of course this is odd at all ends because AMX requires two feature
bits to be enabled (17 and 18).

Now with the above bitmap based thing prctl(SET, 1 << 17) is valid
because it's supported and of course there is no sanity check at all.

With a feature number based interface it's even worse. Duh, should have
thought about that.

So this gives us two options:

   1) Bitmap with proper sanity checks

      reject (1 << 17) and (1 << 18)
      grant (1 << 17 | 1 << 18)

      but for sanity sake and also for ease of filtering, we want to
      restrict a permission request to one functional block at a time.

      #define X86_XCOMP_AMX	(1 << 17 | 1 << 18)
      #define X86_XCOMP_XYZ1    (1 << 19)

      But that gets a bit odd when there is a component which depends on
      others:

      #define X86_XCOMP_XYZ2    (1 << 19 | 1 << 20)

   2) Facility based numerical interface, i.e.

      #define X86_XCOMP_AMX	1
      #define X86_XCOMP_XYZ1    2
      #define X86_XCOMP_XYZ2    3

      is way simpler to understand IMO.

Thanks,

        tglx

Powered by blists - more mailing lists