Re: [PATCH v3 2/3] nsproxy: attach to namespaces via pidfds

2 messages, 2 authors, 2020-05-04 · open the first message on its own page

Re: [PATCH v3 2/3] nsproxy: attach to namespaces via pidfds

From: Eric W. Biederman <hidden>
Date: 2020-05-04 17:13:15

Christian Brauner [off-list ref] writes:
On Mon, May 04, 2020 at 11:25:07AM -0500, Eric W. Biederman wrote:
quoted
I am not thrilled about treating nstype as a flags fields when it is not
currently.  It was my hope when I designed the interface that not
treating nstype as a flags field would save us from the problem of bits
running out.
Hm, I researched the setns() syscall history before that and I didn't
see that reasoning anywhere. The "nstype" arg was originally advertised
on the list as "having a flags field is useful in general".
Take a look at the code.  At the end of the day nstype is not treated at
all like a flags field.

It isn't a very important point.  And it was certainly easier to use
the existing bits for essentially their existing meanings.  But it was
certainly something I was thinking at the time.

I think I left it as we can see either way, depending on how things
evolve.

I can imagine a use for a nstype being a single namespace from a pidfd.
Do you have any actual usecases for setting some but not all of the
namespaces from a pidfd?  If we don't have a compelling reason
I would like to kick that can down the road a ways farther.

I am also remembering that that setns freed the low 8 bits.  Which gave
some freedom beyond clone.
quoted
That aside.  It would be very good if the default version of setting
everything from a pidfd would set the root directory from the process it
is copying everything else from.
I'm not sure I follow completely. If you specify CLONE_NEWNS then we do
set the root directory with set_fs_root() in commit_nsset(). Or are you
saying we should always do that independent of whether or not
CLONE_NEWNS is specified? And if so could you explain why we'd want
that? I'm sure I'm missing something!
I am suggesting that when we do:

"setns(pidfd, 0)" or "setns(pidfd, SETNS_PIDFD)"

That the result is not just the namespaces changing but also the root
directory changing to the pids root directory.  Something where the
whole is greater than the parts.

Eric

Re: [PATCH v3 2/3] nsproxy: attach to namespaces via pidfds

From: Christian Brauner <hidden>
Date: 2020-05-04 17:50:22

On Mon, May 04, 2020 at 12:09:46PM -0500, Eric W. Biederman wrote:
Christian Brauner [off-list ref] writes:
quoted
On Mon, May 04, 2020 at 11:25:07AM -0500, Eric W. Biederman wrote:
quoted
I am not thrilled about treating nstype as a flags fields when it is not
currently.  It was my hope when I designed the interface that not
treating nstype as a flags field would save us from the problem of bits
running out.
Hm, I researched the setns() syscall history before that and I didn't
see that reasoning anywhere. The "nstype" arg was originally advertised
on the list as "having a flags field is useful in general".
Take a look at the code.  At the end of the day nstype is not treated at
all like a flags field.
Oh, I wasn't trying to dispute that. I was just pointing at the history
where using it as a flags field in the future wasn't in principle out of
the question.
It isn't a very important point.  And it was certainly easier to use
the existing bits for essentially their existing meanings.  But it was
certainly something I was thinking at the time.

I think I left it as we can see either way, depending on how things
evolve.

I can imagine a use for a nstype being a single namespace from a pidfd.
Do you have any actual usecases for setting some but not all of the
namespaces from a pidfd?  If we don't have a compelling reason
I would like to kick that can down the road a ways farther.
Yeah, I think so. We already have a few use-cases. The syscall
interception stuff selectively attaches to subsets of namespaces
depending on what namespaces are needed to emulate a given syscall. And
the exec logic let's users select what namespaces to attach to. It's
common to setns to a subset of namespaces to perform operations with
privilege and then later attach others (often the userns).
I am also remembering that that setns freed the low 8 bits.  Which gave
some freedom beyond clone.
quoted
quoted
That aside.  It would be very good if the default version of setting
everything from a pidfd would set the root directory from the process it
is copying everything else from.
I'm not sure I follow completely. If you specify CLONE_NEWNS then we do
set the root directory with set_fs_root() in commit_nsset(). Or are you
saying we should always do that independent of whether or not
CLONE_NEWNS is specified? And if so could you explain why we'd want
that? I'm sure I'm missing something!
I am suggesting that when we do:

"setns(pidfd, 0)" or "setns(pidfd, SETNS_PIDFD)"

That the result is not just the namespaces changing but also the root
directory changing to the pids root directory.  Something where the
whole is greater than the parts.
Ok, I can see that being useful. But If we do this, then a new flag
would be quite helpful. (I also think Michael had some reservations
against re-using 0 for something like this.)
But let me suggest moving your phrase from above down to here and
say that we could kick that can down the road for a follow-up extension?

Christian
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help