Received-SPF: pass (google.com: domain of linux-kernel+bounces-80991-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Mon, 26 Feb 2024 09:57:47 +0100
From: Christian Brauner <brauner@kernel.org>
To: Tycho Andersen <tycho@tycho.pizza>
Cc: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>, 
	stgraber@stgraber.org, cyphar@cyphar.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 2/2] tests/pid_namespace: add pid_max tests
Message-ID: <20240226-gestrafft-pastinaken-94ff0e993a51@brauner>
References: <20240222160915.315255-1-aleksandr.mikhalitsyn@canonical.com>
 <20240222160915.315255-3-aleksandr.mikhalitsyn@canonical.com>
 <Zdd8MAJJD3M11yeR@tycho.pizza>
 <20240223-kantholz-knallen-558beba46c62@brauner>
 <ZdoEavHorDs3IlF5@tycho.pizza>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <ZdoEavHorDs3IlF5@tycho.pizza>

> > > A small quibble, but I wonder about the semantics here. "You can write
> > > whatever you want to this file, but we'll ignore it sometimes" seems
> > > weird to me. What if someone (CRIU) wants to spawn a pid numbered 450
> > > in this case? I suppose they read pid_max first, they'll be able to
> > > tell it's impossible and can exit(1), but returning E2BIG from write()
> > > might be more useful.
> > 
> > That's a good idea. But it's a bit tricky. The straightforward thing is
> > to walk upwards through all ancestor pid namespaces and use the lowest
> > pid_max value as the upper bound for the current pid namespace. This
> > will guarantee that you get an error when you try to write a value that
> > you would't be able to create. The same logic should probably apply to
> > ns_last_pid as well.
> > 
> > However, that still leaves cases where the current pid namespace writes
> > a pid_max limit that is allowed (IOW, all ancestor pid namespaces are
> > above that limit.). But then immediately afterwards an ancestor pid
> > namespace lowers the pid_max limit. So you can always end up in a
> > scenario like this.
> 
> I wonder if we can push edits down too? Or an render .effective file, like

I don't think that works in the current design? The pid_max value is per
struct pid_namespace. And while there is a 1:1 relationship between a
child pid namespace to all of its ancestor pid namespaces there's a 1 to
many relationship between a pid namespace and it's child pid namespaces.
IOW, if you change pid_max in pidns_level_1 then you'd have to go
through each of the child pid namespaces on pidns_level_2 which could be
thousands. So you could only do this lazily. IOW, compare and possibly
update the pid_max value of the child pid namespace everytime it's read
or written. Maybe that .effective is the way to go; not sure right now.