On 29/10/2020 02:05, Jann Horn wrote:
On Tue, Oct 27, 2020 at 9:04 PM Mickaël Salaün mic@digikod.net wrote:
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: Jann Horn jannh@google.com Cc: Kees Cook keescook@chromium.org Cc: Serge E. Hallyn serge@hallyn.com Signed-off-by: Mickaël Salaün mic@linux.microsoft.com
Reviewed-by: Jann Horn jannh@google.com
Thanks for the review.
except for some minor nits:
[...]
diff --git a/security/landlock/object.c b/security/landlock/object.c
[...]
+void landlock_put_object(struct landlock_object *const object) +{
/** The call to @object->underops->release(object) might sleep e.g.,s/ e.g.,/, e.g./
I indeed prefer the comma preceding the "e.g.", but it seems that there is a difference between UK english and US english: https://english.stackexchange.com/questions/16172/should-i-always-use-a-comm... Looking at the kernel documentation makes it clear: $ git grep -F 'e.g. ' | wc -l 1179 $ git grep -F 'e.g., ' | wc -l 160
I'll apply your fix in the whole patch series.
* because of iput().*/might_sleep();if (!object)return;[...]
+} diff --git a/security/landlock/object.h b/security/landlock/object.h
[...]
+struct landlock_object {
/*** @usage: This counter is used to tie an object to the rules matching* it or to keep it alive while adding a new rule. If this counter* reaches zero, this struct must not be modified, but this counter can* still be read from within an RCU read-side critical section. When* adding a new rule to an object with a usage counter of zero, we must* wait until the pointer to this object is set to NULL (or recycled).*/refcount_t usage;/*** @lock: Guards against concurrent modifications. This lock must bes/must be/must be held/ ?
Right.
* 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;[...]
+};
+struct landlock_object *landlock_create_object(
const struct landlock_object_underops *const underops,void *const underojb);nit: "underobj"
Good catch!