On 21/11/2020 08:00, Jann Horn wrote:

On Thu, Nov 12, 2020 at 9:51 PM Mickaël Salaün [off-list ref] wrote:

quoted

A Landlock object enables to identify a kernel object (e.g. an inode).
A Landlock rule is a set of access rights allowed on an object.  Rules
are grouped in rulesets that may be tied to a set of processes (i.e.
subjects) to enforce a scoped access-control (i.e. a domain).

Because Landlock's goal is to empower any process (especially
unprivileged ones) to sandbox themselves, we cannot rely on a
system-wide object identification such as file extended attributes.
Indeed, we need innocuous, composable and modular access-controls.

The main challenge with these constraints is to identify kernel objects
while this identification is useful (i.e. when a security policy makes
use of this object).  But this identification data should be freed once
no policy is using it.  This ephemeral tagging should not and may not be
written in the filesystem.  We then need to manage the lifetime of a
rule according to the lifetime of its objects.  To avoid a global lock,
this implementation make use of RCU and counters to safely reference
objects.

A following commit uses this generic object management for inodes.

Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <redacted>
Reviewed-by: Jann Horn <jannh@google.com>

Still looks good, except for one comment:

[...]

quoted

+       /**
+        * @lock: Guards against concurrent modifications.  This lock might be
+        * held from the time @usage drops to zero until any weak references
+        * from @underobj to this object have been cleaned up.
+        *
+        * Lock ordering: inode->i_lock nests inside this.
+        */
+       spinlock_t lock;

Why did you change this to "might be held" (v22 had "must")? Is the
"might" a typo?

Good catch, a typo indeed.