Re: [PATCH -mm 1/9] unshare system call: system call handlerfunction

[Eric W. Biederman]

JANAK DESAI <janak@xxxxxxxxxx> writes:

> Eric W. Biederman wrote:
>
>>If it isn't legal how about we deny the unshare call.
>>Then we can share this code with clone.
>>
>>Eric
>>
>>
>>
>>
> unshare is doing the reverse of what clone does. So if you are unsharing
> namespace
> you want to make sure that you are not sharing fs. That's why the CLONE_FS flag
> is forced (meaning you are also going to unshare fs). Since namespace is shared
> by default and fs is not, if clone is called with CLONE_NEWNS, the intent is to
> create a new namespace, which means you cannot share fs. clone checks to
> makes sure that CLONE_NEWNS and CLONE_FS are not specified together, while
> unshare will force CLONE_FS if CLONE_NEWNS is spefcified. Basically you
> can have a shared namespace and non shared fs, but not the other way around and
> since clone and unshare are doing opposite things, sharing code between them
> that
> checks constraints on the flags can get convoluted.

I follow but I am very disturbed.

You are leaving CLONE_NEWNS to mean you want a new namespace.

For clone CLONE_FS unset means generate an unshared fs_struct
          CLONE_FS set   means share the fs_struct with the parent

But for unshare CLONE_FS unset means share the fs_struct with others
            and CLONE_FS set   means generate an unshared fs_struct

The meaning of CLONE_FS between the two calls in now flipped,
but CLONE_NEWNS is not.  Please let's not implement it this way.

Part of the problem is the double negative in the name, leading
me to suggest that sys_share might almost be a better name.

So please code don't invert the meaning of the bits.  This will
allow sharing of the sanity checks with clone.

In addition this leaves open the possibility that routines like
copy_fs properly refactored can be shared between clone and unshare.


Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/