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] [thread-next>] [day] [month] [year] [list]
Date: Wed, 08 May 2024 14:52:18 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Aruna Ramakrishna <aruna.ramakrishna@...cle.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "x86@...nel.org" <x86@...nel.org>, "dave.hansen@...ux.intel.com"
 <dave.hansen@...ux.intel.com>, Ingo Molnar <mingo@...nel.org>, Keith Lucas
 <keith.lucas@...cle.com>
Subject: Re: [PATCH v3 3/4] x86/pkeys: Update PKRU to enable all pkeys
 before XSAVE

On Tue, May 07 2024 at 17:34, Aruna Ramakrishna wrote:
>> On May 7, 2024, at 9:47 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
>>
>> Also this lacks any justification why this enables all pkeys and how
>> that is the right thing to do instead of using init_pkru_value which
>> is what is set by fpu__clear_user_states() before going back to user
>> space. For signal handling this can be the only valid PKEY state unless
>> I'm missing something here.
>
> If the alt sig stack is protected by a different pkey (other than pkey 0), then
> this flow would need to enable that, along with the pkey for the thread’s 
> stack. Since the code has no way of knowing what pkey the altstack needs,
> it enables all for this brief window.

Again. The flow here is:

handle_signal()
  enable_access_to_altstack()

  ....

  fpu__clear_user_states()
    restore_fpregs_from_init_fpstate(XFEATURE_MASK_USER_RESTORE)
    os_xrstor(&init_fpstate, features_mask)
    pkru_write_default()
      write_pkru(init_pkru_value);  <- Loads the default PKRU value
           
return_to_user_space()

User space resumes with the default PKRU value and the first thing user
space does when entering the signal handler is to push stuff on the
signal stack.

If the signal stack is protected by a key which is not contained in
init_pkru_value then the application segfaults in a non recoverable way,
no?

So arguably it is sufficient to ensure that PKRU has the keys in
init_pkru_value enabled:

    sigpkru = read_pkru() & init_pkru_value;

If user space protects the task stack or the sigalt stack with a key
which is not in init_pkru_value then it does not matter at all whether
it dies in handle_signal() or later when returning to user space, no?

I'm not fundamentaly opposed to enable all keys, but I don't buy this
without a proper explanation why this has been chosen over enabling only
the absolute minimum access rights.

Thanks,

        tglx

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ