Re: [PATCH v18 24/25] x86/cet/shstk: Add arch_prctl functions for shadow stack

[Yu, Yu-cheng]

On 2/3/2021 2:11 PM, Dave Hansen wrote:
> On 2/3/21 1:54 PM, Yu, Yu-cheng wrote:
> > On 1/29/2021 10:56 AM, Yu, Yu-cheng wrote:
> > > On 1/29/2021 9:07 AM, Dave Hansen wrote:
> > > > On 1/27/21 1:25 PM, Yu-cheng Yu wrote:
> > > > > +    if (!IS_ENABLED(CONFIG_X86_CET))
> > > > > +        return -EOPNOTSUPP;
> > > >
> > > > Let's ignore glibc for a moment.  What error code *should* the kernel be
> > > > returning here?  errno(3) says:
> > > >
> > > >          EOPNOTSUPP      Operation not supported on socket (POSIX.1)
> > > > ...
> > > >          ENOTSUP         Operation not supported (POSIX.1)
> > >
> > > Yeah, other places in kernel use ENOTSUPP.  This seems to be out of
> > > line.  And since the issue is long-existing, applications already know
> > > how to deal with it.  I should have made that argument.  Change it to
> > > ENOTSUPP.
> >
> > When I make the change, checkpatch says...
> >
> > WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP
> > #128: FILE: arch/x86/kernel/cet_prctl.c:33:
> > +        return -ENOTSUPP;
> >
> > Do we want to reconsider?
>
> I'm not sure I trust checkpatch over manpages.  I had to google "SUSV4".
>   I'm not sure it matters at *all* for a 100% Linux-specific interface.
>
> ENOTSUPP does seem less popular lately:
>
> > $ git diff v5.0.. kernel/ arch/ drivers/ | grep ^+.*return.*E.*NO.*SUP.*\; | grep -o -- -E.*\; | sort | uniq -c | sort -n
> > ... noise
> >       61 -EOPNOTSUPP);
> >      260 -ENOTSUPP;
> >     1577 -EOPNOTSUPP;
>
> but far from unused.  That might be due to checkpatch spew more than
> anything.

Maybe I will keep it ENOTSUPP for now.  If any logical reason should 
come up, I will be happy to change it again.  Thanks!

--
Yu-cheng