Re: Time Namespaces: CLONE_NEWTIME and clone3()?

Hello Christian,

On Mon, 17 Feb 2020 at 16:15, Christian Brauner
[off-list ref] wrote:

On Mon, Feb 17, 2020 at 03:20:55PM +0100, Michael Kerrisk wrote:

quoted

Hello Dmitry, Andrei,

Is the CLONE_NEWTIME flag intended to be usable with clone3()? The
mail quoted below implies (in my reading) that this should be possible
once clone3() is available, which it is by now. (See also [1].)

If the answer is yes, CLONE_NEWTIME  should be usable with clone3(),
then I have a bug report and a question.

I successfully used CLONE_NEWTIME with unshare(). But if I try to use
CLONE_NEWSIGNAL with clone3(), it errors out with EINVAL, because of

s/CLONE_NEWSIGNAL/CLONE_NEWTIME/

quoted

the following check in clone3_args_valid():

        /*
         * - make the CLONE_DETACHED bit reuseable for clone3
         * - make the CSIGNAL bits reuseable for clone3
         */
        if (kargs->flags & (CLONE_DETACHED | CSIGNAL))
                return false;

The problem is that CLONE_NEWTIME matches one of the bits in the
CSIGNAL mask. If the intention is to allow CLONE_NEWTIME with
clone3(), then either the bit needs to be redefined, or the error
checking in clone3_args_valid() needs to be reworked.

If this is intended to be useable with clone3() the check should be
adapted to allow for CLONE_NEWTIME. (I asked about this a while ago I
think.)
But below rather sounds like it should simply be an unshare() flag. The
code seems to set frozen_offsets to true right after copy_namespaces()
in timens_on_fork(new_ns, tsk) and so the offsets can't be changed
anymore unless I'm reading this wrong.
Alternatives seem to either make timens_offsets writable once after fork
and before exec, I guess - though that's probably not going to work
with the vdso judging from timens_on_fork().

The other alternative is that Andrei and Dmitry send me a patch to
enable CLONE_NEWTIME with clone3() by exposing struct timens_offsets (or
a version of it) in the uapi and extend struct clone_args to include a
pointer to a struct timens_offset that is _only_ set when CLONE_NEWTIME
is set.
Though the unshare() way sounds way less invasive simpler.

Actually, I think the alternative you propose just here is better. I
imagine there are times when one will want to create multiple
namespaces with a single call to clone3(), including a time namespace.
I think this should be allowed by the API. And, otherwise, clone3()
becomes something of a second-class citizen for creating namespaces.
(I don't really get the "less invasive" argument. Implementing this is
just a piece of kernel to code to make user-space's life a bit simpler
and more consistent.)

Thanks,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/