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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9e59305039f2c8077ee087313d1df5ff06028cfe.camel@intel.com>
Date:   Sat, 14 May 2022 23:06:58 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "aryabinin@...tuozzo.com" <aryabinin@...tuozzo.com>,
        "dvyukov@...gle.com" <dvyukov@...gle.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "glider@...gle.com" <glider@...gle.com>
Subject: Re: [RFCv2 03/10] x86: Introduce userspace API to handle per-thread
 features

On Sat, 2022-05-14 at 10:37 +0200, Thomas Gleixner wrote:
> On Fri, May 13 2022 at 23:50, Edgecombe, Rick P wrote:
> > On Sat, 2022-05-14 at 02:09 +0300, Kirill A. Shutemov wrote:
> > > On Fri, May 13, 2022 at 05:34:12PM +0000, Edgecombe, Rick P
> > > wrote:
> > > > On Fri, 2022-05-13 at 16:09 +0200, Alexander Potapenko wrote:
> > > > > > +
> > > > > > +       /* Handle ARCH_THREAD_FEATURE_ENABLE */
> > > > > > +
> > > > > > +       task->thread.features |= features;
> > > > > > +out:
> > > > > > +       return task->thread.features;
> > > > > 
> > > > > Isn't arch_prctl() supposed to return 0 on success?
> > > > 
> > > > Hmm, good point. Maybe we'll need a struct to pass info in and
> > > > out.
> > > 
> > > But values >0 are unused. I don't see why can't we use them.
> > 
> > Hmm, I don't know what it would break since it is a new "code"
> > argument. But the man page says:
> > "On success, arch_prctl() returns 0; on error, -1 is returned, and
> > errno is set to indicate the error."
> > 
> > So just change the man pages?
> > "On success, arch_prctl() returns positive values; on error, -1 is
> > returned, and errno is set to indicate the error."
> 
> Why?
> 
>         prctl(GET, &out)
> 
> is the pattern used all over the place.

It seems better to me, but we also need to pass something in.

The idea of the "enable" operation is that userspace would pass in all
the features that it wants in one call, and then find out back what was
successfully enabled. So unlike the other arch_prctl()s, it wants to
pass something in AND get a result in one arch_prctl() call. It doesn't
need to check what is supported ahead of time. Since these enabling
operations can fail (OOM, etc), userspace has to handle unexpected per-
feature failure anyway. So it just blindly asks for what it wants.

Any objections to having it write back the result in the same
structure?

Otherwise, the option that used to be used here was a "status"
arch_prctl(), which was called separately to find out what actually got
enabled after an "enable" call. That fit with the GET/SET semantics
already in place.

I guess we could also get rid of the batch enabling idea, and just have
one "enable" call per feature too. But then it is syscall heavy.

Any chance you could weigh in on this?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ