Re: [PATCH v7 1/3] Add an internal wrapper for clone, clone2 and clone3

["H.J. Lu" <hjl.tools@gmail.com>]

On Mon, May 31, 2021 at 8:57 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 31/05/2021 11:01, H.J. Lu wrote:
> > On Mon, May 31, 2021 at 6:53 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 31/05/2021 10:16, H.J. Lu wrote:
> >>> On Mon, May 31, 2021 at 6:01 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>>>
> >>>> * H. J. Lu:
> >>>>
> >>>>> In the kernel, clone3 and clone go to the same piece of code.  clone won't
> >>>>> fail just because of CET.
> >>>>
> >>>> But clone won't have access to the stack boundaries.  Won't this create
> >>>> issues for setting up the shadow stack?
> >>>>
> >>>
> >>> No.  There are:
> >>>
> >>>         /* Cap shadow stack size to 4 GB */
> >>>         size = min_t(unsigned long long, rlimit(RLIMIT_STACK), SZ_4G);
> >>>         size = min(size, stack_size);
> >>>
> >>> where stack_size is passed in clone3.
> >>
> >> Right, so CET support does not really require clone3 to be used internally
> >> then? Or am I missing something?
> >
> > Shadow stack size shouldn't be more than normal stack size.  The current
> > CET kernel shadow stack size may not be optimal.  My original code did
> >
> > if (stack_size != 0)
> >   size = stack_size;
> > else
> >   size = min_t(unsigned long long, rlimit(RLIMIT_STACK), SZ_4G);
> >
> > But
> >
> > 1. I don't want to disturb it before CET changes are upstreamed.
> > 2. It can be updated AFTER it has been upstreamed.
>
> Right, so I take this is just an optimization assuming that the extra
> size would unused, right? I still failing to see why clone3 is an

Correct.

> requirement for CET enablement (if I understood this correctly).

It isn't a MUST have.  It is an improvement for CET.

> I still think supporting clone3 is a nice thing to have, specially
> for possible newer architectures and to support newer flags and
> functionalities.



-- 
H.J.